public inbox for backports@vger.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: "Luis R. Rodriguez" <mcgrof@suse.com>
Cc: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>,
	backports@vger.kernel.org, linux-kernel@vger.kernel.org,
	yann.morin.1998@free.fr, mmarek@suse.cz, sassmann@kpanic.de
Subject: Re: [PATCH v2 03/13] backports: allow for different backport prefix
Date: Wed, 05 Nov 2014 22:17:38 +0100	[thread overview]
Message-ID: <1415222258.7485.10.camel@sipsolutions.net> (raw)
In-Reply-To: <20141105194215.GN12953@wotan.suse.de>

On Wed, 2014-11-05 at 20:42 +0100, Luis R. Rodriguez wrote:

> > No, I mean if bp_prefix were to contain some special character like [.
> > This can't actually happen though.
> 
> OK if that can't happen then I don't see the point.

Where by "can't happen" I mean that Kconfig probably wouldn't be happy
about it :-)

> How about:
> 
> if integrate:
> 	kconfig_prefix = "CONFIG_"
> 	project_prefix = "BACKPORT_"
> else:
> 	kconfig_prefix = "CPTCFG_"
> 	# implied by kconfig_prefix
> 	project_prefix = ""
> full_prefix = kconfig_prefix + project_prefix

Yeah, this seems like decent naming.

> But note that we special case things all over for when the bp_prefix is 'CPTCFG'
> and the reason is that it is used when the kconfig getenv trick is used. I suppose
> we can infer that the kconfig trick is used when kconfig_prefix != 'CONFIG_' though.
> But the question still stands: why do we want to make these configurable? As it
> stands I actually hard code things mostly, I can clean this up but would prefer
> to deal with just the above.

I'm not sure where that's really the case? The one thing you did was use
CPTCFG_ in some places like the version, but that was independent of
this and could just be hardcoded there?

> Also note that in some places we use BACKPORT vs BACKPORT_ so a sub or another
> variable would be needed.

I also didn't see that, why would that ever be needed?

> As it stands in the v2 series packaging gets no BACKPORT_ prefix as the
> kconfig getenv() CTPCFG_ trick is used, however for integration we
> do add the prefix everywhere as we are carrying code into the kernel,
> we then use this to easily make this symbol depend on the non backported
> respective symbol making them mutually exclusive. Because of these two
> things I think a BACKPORT_ prefix for things we carry in is proper. This
> applies to the BACKPORT_BPAUTO and versioning entries, BACKPORT_KERNEL_3_18
> and so on.

Sure.

> I am not sure what other prefix we could possibly use here for integration.

Well, in theory you could imagine having two people backport different
subsystems and trying to integrate them both into a backport kernel ...
seems unlikely and will conflict on other levels (e.g. compat.ko) but
still.

Anyway, I don't really care if you don't make this configurable, but I
don't like the way you're doing string manipulation to figure out what
you want :-) That was really the reason for suggesting to split the
existing prefix variable into two pieces. You have it today, after all,
in theory you could have gotten rid of it completely and just pass the
"integrate" flag around.

johannes


  reply	other threads:[~2014-11-05 21:17 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-05  3:18 [PATCH v2 00/13] backports: add kernel integration support Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 01/13] backports: move legacy and SmPL patch application into helper Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 02/13] backports: ifdef around module_init() module_exit() for modules Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 03/13] backports: allow for different backport prefix Luis R. Rodriguez
2014-11-05  7:46   ` Johannes Berg
2014-11-05  9:16     ` Luis R. Rodriguez
2014-11-05  9:22       ` Johannes Berg
2014-11-05 19:42         ` Luis R. Rodriguez
2014-11-05 21:17           ` Johannes Berg [this message]
2014-11-05 22:21             ` Luis R. Rodriguez
2014-11-05 23:09               ` Andi Kleen
2014-11-05 23:15                 ` Luis R. Rodriguez
2014-11-05 23:31                   ` Andi Kleen
2014-11-05  3:18 ` [PATCH v2 04/13] backports: replace BACKPORT_PWD with BACKPORT_DIR Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 05/13] backports: use BACKPORT_DIR prefix on kconfig sources Luis R. Rodriguez
2014-11-05  7:51   ` Johannes Berg
2014-11-05 20:11     ` Luis R. Rodriguez
2014-11-05 21:19       ` Johannes Berg
2014-11-05 22:22         ` Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 06/13] backports: update dependencies map file Luis R. Rodriguez
2014-11-05  7:54   ` Johannes Berg
2014-11-05 20:13     ` Luis R. Rodriguez
2014-11-05 21:20       ` Johannes Berg
2014-11-05 22:23         ` Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 07/13] backports: split Kconfig into Kconfig.package and Kconfig.sources Luis R. Rodriguez
2014-11-05  7:55   ` Johannes Berg
2014-11-05 20:14     ` Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 08/13] backports: move version file generation to run earlier Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 09/13] backports: define C code backport version info using CPTCFG_ Luis R. Rodriguez
2014-11-05  7:57   ` Johannes Berg
2014-11-05 20:29     ` Luis R. Rodriguez
2014-11-05 21:20       ` Johannes Berg
2014-11-05 22:27         ` Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 10/13] backports: add backport version parsing for kernel integration Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 11/13] backports: prefix c-file / h-file auto backport with BPAUTO Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 12/13] backports: remove extra BACKPORT_ prefix from kernel versioning Luis R. Rodriguez
2014-11-05  3:18 ` [PATCH v2 13/13] backports: add full kernel integration support Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1415222258.7485.10.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=backports@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@do-not-panic.com \
    --cc=mcgrof@suse.com \
    --cc=mmarek@suse.cz \
    --cc=sassmann@kpanic.de \
    --cc=yann.morin.1998@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox