linux/uboot: add support for custom Mercurial repositories

The existing comments in Config.in.legacy are not entirely in-line with current practice. The comments implies that BR2_LEGACY should not be set when the conversion from old-to-new symbol can be done automatically using the appropriate 'select' statements. However, none of the existing legacy options does it this way. Moreover, I think it's intentional that the user is notified of the change, so that the removal of the legacy options in later buildroot versions no longer poses a problem.

Additionally, I have added an example of how to handle the renaming of a string config option. Because a string option cannot 'select' something else, a wrapper symbol is needed of type bool.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
(v2): new patch in this series

 Config.in.legacy |  22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

Config.in.legacy +17 -5

@@ -3,11 +3,10 @@
 #
 # When an existing Config.in symbol is removed, it should be added again in this
 # file, and take appropriate action to approximate backward compatibility. If
-# there is an equivalent (set of) new symbols, these can just be select'ed by
-# the old symbol. This makes sure that running 'make oldconfig' will make things
-# "just work" when upgrading to a new buildroot version. If the change is too
-# fundamental and cannot be fixed by a simple select, then the old symbol should
-# select BR2_LEGACY.  If that symbol is set, the build will issue an error.
+# there is an equivalent (set of) new symbols, these should be select'ed by
+# the old symbol. This will make the transition for the user more convenient.
+# The old symbol should select BR2_LEGACY, so that the user is informed at
+# build-time about selected legacy options.
 #
 # When adding legacy symbols to this file, add them to the front. The oldest
 # symbols will be removed again after about two years.

@@ -15,6 +14,19 @@
 # The symbol should be copied as-is from the place where it was previously
 # defined, but the help text should be removed or replaced with something that
 # explains how to fix it.
+# If the old symbol is of type string, and the symbol has been renamed, you
+# should add it as a bool with another name (BR2_<foo>_WRAP), that defaults
+# to y if the old string is not set at its default value. For example:
+#     config BR2_FOO_STRING
+#             string "Some foo string"
+# would become:
+#     config BR2_FOO_STRING_WRAP
+#             bool "Foo string option has been renamed."
+#             default y if BR2_FOO_STRING != ""
+#             depends on BR2_LEGACY
+#             help
+#               <suitable help text>
+#
 
 config BR2_LEGACY
 	bool

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

The existing comments in Config.in.legacy are not entirely in-line with current practice. The comments implies that BR2_LEGACY should not be set when the conversion from old-to-new symbol can be done automatically using the appropriate 'select' statements. However, none of the existing legacy options does it this way. Moreover, I think it's intentional that the user is notified of the change, so that the removal of the legacy options in later buildroot versions no longer poses a problem.

Additionally, I have added an example of how to handle the renaming of a string config option. Because a string option cannot 'select' something else, a wrapper symbol is needed of type bool.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
(v2): new patch in this series

  Config.in.legacy |  22 +++++++++++++++++-----
  1 files changed, 17 insertions(+), 5 deletions(-)

Config.in.legacy +17 -5

@@ -3,11 +3,10 @@
  #
  # When an existing Config.in symbol is removed, it should be added again in this
  # file, and take appropriate action to approximate backward compatibility. If
-# there is an equivalent (set of) new symbols, these can just be select'ed by
-# the old symbol. This makes sure that running 'make oldconfig' will make things
-# "just work" when upgrading to a new buildroot version. If the change is too
-# fundamental and cannot be fixed by a simple select, then the old symbol should
-# select BR2_LEGACY.  If that symbol is set, the build will issue an error.
+# there is an equivalent (set of) new symbols, these should be select'ed by
+# the old symbol. This will make the transition for the user more convenient.
+# The old symbol should select BR2_LEGACY, so that the user is informed at
+# build-time about selected legacy options.
  #
  # When adding legacy symbols to this file, add them to the front. The oldest
  # symbols will be removed again after about two years.

@@ -15,6 +14,19 @@
  # The symbol should be copied as-is from the place where it was previously
  # defined, but the help text should be removed or replaced with something that
  # explains how to fix it.
+# If the old symbol is of type string, and the symbol has been renamed, you
+# should add it as a bool with another name (BR2_<foo>_WRAP), that defaults
+# to y if the old string is not set at its default value. For example:
+#     config BR2_FOO_STRING
+#             string "Some foo string"
+# would become:
+#     config BR2_FOO_STRING_WRAP
+#             bool "Foo string option has been renamed."
+#             default y if BR2_FOO_STRING != ""
+#             depends on BR2_LEGACY
+#             help
+#               <suitable help text>
+#
 
  config BR2_LEGACY
  	bool

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Config.in.legacy +2 -0

+#             help
+#               <suitable help text>

On 24/07/13 09:22, Thomas De Schampheleire wrote:
[snip]

Config.in.legacy line 26

Quoted 12 lines

+# If the old symbol is of type string, and the symbol has been renamed, you
+# should add it as a bool with another name (BR2_<foo>_WRAP), that defaults

Config.in.legacy +8 -0

+# to y if the old string is not set at its default value. For example:
+#     config BR2_FOO_STRING
+#             string "Some foo string"
+# would become:
+#     config BR2_FOO_STRING_WRAP
+#             bool "Foo string option has been renamed."
+#             default y if BR2_FOO_STRING != ""
+#             depends on BR2_LEGACY

My Ack was too hasty, this should be
select BR2_LEGACY

Specifying a floating tag like HEAD for a repository version is bad practice, as it results in non-reproducible builds. This patch removes the default assignment of HEAD as version when a custom git repository is used for the Linux kernel.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
(v2): new patch in series after comment from Peter

  linux/Config.in |  1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

linux/Config.in +0 -1

@@ -68,7 +68,6 @@ config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_
 
  config BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION
  	string "Custom Git version"
-	default "HEAD"
  	depends on BR2_LINUX_KERNEL_CUSTOM_GIT
  	help
  	  Git revision to use in the format used by git rev-parse,

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Specifying a floating tag like HEAD for a repository version is bad practice, as it results in non-reproducible builds. This patch removes the default assignment of HEAD as version when a custom git repository is used for the Linux kernel.

Signed-off-by Thomas De Schampheleire <[email protected]>

linux/Config.in +6 -0

@@ -52,6 +52,12 @@ config BR2_LINUX_KERNEL_CUSTOM_GIT
  	  This option allows Buildroot to get the Linux kernel source
  	  code from a Git repository.
 
+config BR2_LINUX_KERNEL_CUSTOM_HG
+	bool "Custom Mercurial repository"
+	help
+	  This option allows Buildroot to get the Linux kernel source
+	  code from a Mercurial repository.
+
  endchoice
 
  config BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE

@@ -62,24 +68,26 @@ config BR2_LINUX_KERNEL_CUSTOM_TARBALL_L
  	string "URL of custom kernel tarball"
  	depends on BR2_LINUX_KERNEL_CUSTOM_TARBALL
 

linux/Config.in +7 -2

+
+config BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION
+	string "Custom repository version"
  	help
-	  Git revision to use in the format used by git rev-parse,
+	  Revision to use in the typical format used by Git/Mercurial
  	  E.G. a sha id, a tag, branch, ..
 
+endif
+
  config BR2_LINUX_KERNEL_VERSION
  	string
  	default "3.10.1" if BR2_LINUX_KERNEL_LATEST_VERSION
  	default BR2_DEFAULT_KERNEL_HEADERS if BR2_LINUX_KERNEL_SAME_AS_HEADERS
  	default BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE if BR2_LINUX_KERNEL_CUSTOM_VERSION
  	default "custom" if BR2_LINUX_KERNEL_CUSTOM_TARBALL
-	default $BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION if BR2_LINUX_KERNEL_CUSTOM_GIT
+	default $BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION if BR2_LINUX_KERNEL_CUSTOM_HG || BR2_LINUX_KERNEL_CUSTOM_GIT
 
  #
  # Patch selection

linux/linux.mk +4 -1

@@ -14,8 +14,11 @@ LINUX_TARBALL = $(call qstrip,$(BR2_LINU
  LINUX_SITE = $(patsubst %/,%,$(dir $(LINUX_TARBALL)))
  LINUX_SOURCE = $(notdir $(LINUX_TARBALL))
  else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y)
-LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL))
+LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
  LINUX_SITE_METHOD = git
+else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HG),y)
+LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
+LINUX_SITE_METHOD = hg
  else
  LINUX_SOURCE = linux-$(LINUX_VERSION).tar.xz
  # In X.Y.Z, get X and Y. We replace dots and dashes by spaces in order

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

On 24/07/13 09:23, Thomas De Schampheleire wrote:
[snip]

Quoted 20 lines

linux/Config.in +6 -0

@@ -52,6 +52,12 @@ config BR2_LINUX_KERNEL_CUSTOM_GIT
        This option allows Buildroot to get the Linux kernel source
        code from a Git repository.
 
+config BR2_LINUX_KERNEL_CUSTOM_HG
+     bool "Custom Mercurial repository"
+     help
+       This option allows Buildroot to get the Linux kernel source
+       code from a Mercurial repository.
+
  endchoice
 
  config BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE

@@ -62,24 +68,26 @@ config BR2_LINUX_KERNEL_CUSTOM_TARBALL_L
      string "URL of custom kernel tarball"
      depends on BR2_LINUX_KERNEL_CUSTOM_TARBALL
 

linux/Config.in config BR2_LINUX_KERNEL_CUSTOM_TARBALL_L · line 74

Quoted 10 lines

linux/Config.in +3 -6

-config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
-     string "URL of custom Git repository"
-     depends on BR2_LINUX_KERNEL_CUSTOM_GIT
+if BR2_LINUX_KERNEL_CUSTOM_GIT || BR2_LINUX_KERNEL_CUSTOM_HG
 
-config BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION
-     string "Custom Git version"
-     depends on BR2_LINUX_KERNEL_CUSTOM_GIT
+config BR2_LINUX_KERNEL_CUSTOM_REPO_URL
+     string "URL of custom repository"

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

If we do rename the option symbol names, then I would make sure that it
is propagated:

        default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Also, you have to change the symbol names in the defconfigs as well
(there are already 10 uses of this symbol).

Hi Arnout,

On Mon, Aug 12, 2013 at 8:12 AM, Arnout Vandecappelle <[email protected]> wrote:

Quoted 7 lines

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

If we do rename the option symbol names, then I would make sure that it
is propagated:

Quoted 7 lines

         default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Also, you have to change the symbol names in the defconfigs as well
(there are already 10 uses of this symbol).

Thanks for your feedback on this series.

Regarding the choice of keeping the _GIT_ option names as-is, or changing them with the appropriate legacy handling, I'd like to hear the opinion of others.

I understand that the _GIT_ names make the legacy handling easier, but I also feel it's awkward to have _GIT_ in your config file when you're really using Mercurial. It seems like a hack to me. So my preference would be to stick to the renaming as proposed in this patch series, but with the additional default in linux/Config.in (and likewise for uboot) as you suggested, deprecating this default later. In this case I'll also change the defconfigs, as you correctly noted (thanks).

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

If we do rename the option symbol names, then I would make sure that it
is propagated:

	default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Dear Arnout Vandecappelle,

Quoted 11 lines · Arnout Vandecappelle

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

If we do rename the option symbol names, then I would make sure that it
is propagated:

	default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Hum, I'm not sure how this articulates with the _WRAP mechanism that patch 1/6 is proposing. If we do this:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

then why would we need the _WRAP thing?

Quoted 25 lines · Thomas Petazzoni

Dear Arnout Vandecappelle,

Quoted 4 lines · Arnout Vandecappelle

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

Quoted 8 lines

If we do rename the option symbol names, then I would make sure that it
is propagated:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Hum, I'm not sure how this articulates with the _WRAP mechanism that patch 1/6 is proposing. If we do this:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

Quoted 1 line

then why would we need the _WRAP thing?

The _WRAP thing is needed to make sure there is a user-visible boolean
option in the legacy menu, and to make sure you only select BR2_LEGACY when the old option is not empty.

However, come to think of it, wouldn't it be enough to keep it as a string
option and add

        select BR2_LEGACY if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

? Although I guess Thomas has tested this already.

Looking again at the patch, though, I wonder if it works at all. Aren't the
old options that have no symbol definition removed? I think there should still be a

config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
string

in Config.in.legacy.

But then, it will not be possible to modify the string anymore... so there
is no escape...

Argh, Kconfig...

Hi,

Quoted 26 lines · Arnout Vandecappelle

Quoted 25 lines · Thomas Petazzoni

Dear Arnout Vandecappelle,

Quoted 4 lines · Arnout Vandecappelle

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

Quoted 8 lines

If we do rename the option symbol names, then I would make sure that it
is propagated:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Hum, I'm not sure how this articulates with the _WRAP mechanism that patch 1/6 is proposing. If we do this:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

Quoted 6 lines

Quoted 1 line

then why would we need the _WRAP thing?

The _WRAP thing is needed to make sure there is a user-visible boolean
option in the legacy menu, and to make sure you only select BR2_LEGACY when the old option is not empty.

Correct.
Unlike for bool options that can be manipulated from other options, you cannot manipulate a string option from another option, only from the string option itself by adding a default statement, as Arnout proposed in his earlier mail.

Quoted 7 lines

However, come to think of it, wouldn't it be enough to keep it as a string
option and add

        select BR2_LEGACY if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

? Although I guess Thomas has tested this already.

You mean this, right?
config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
string "linux: the git repository URL option has been renamed" select BR2_LEGACY if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

I did indeed try this but it doesn't work. Kconfig gives the following message:

Config.in.legacy:75:warning: config symbol 'BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL' uses select, but is not boolean or tristate

Quoted 8 lines

Looking again at the patch, though, I wonder if it works at all. Aren't the
old options that have no symbol definition removed? I think there should still be a

config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
string

Quoted 6 lines

in Config.in.legacy.

But then, it will not be possible to modify the string anymore... so there
is no escape...

Argh, Kconfig...

If you have the following config snippet before applying the patch:
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL="my_repo_url" BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION="my_repo_version" BR2_LINUX_KERNEL_VERSION="my_repo_version"

and then apply the patch and run 'make menuconfig' and save without changes, you get:

BR2_LINUX_KERNEL_CUSTOM_GIT=y
BR2_LINUX_KERNEL_CUSTOM_REPO_URL=""
BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION=""
BR2_LINUX_KERNEL_VERSION=""
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL_WRAP=y
BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION_WRAP=y

i.e. the wrap thing does work. It's true that the original option BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL is no longer present _after_ saving the config, but the original value _has_ been detected and used to set the _WRAP option.

Quoted 92 lines · Thomas De Schampheleire

Hi,

Quoted 26 lines · Arnout Vandecappelle

Quoted 25 lines · Thomas Petazzoni

Dear Arnout Vandecappelle,

Quoted 4 lines · Arnout Vandecappelle

Given that string options can't be propagated from their legacy values,
and since the name of an option isn't really that important, I'd keep the _GIT_ names for the mercurial options as well.

Quoted 8 lines

If we do rename the option symbol names, then I would make sure that it
is propagated:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

+ add a comment to the .legacy file so these hacks can be removed eventually.

Hum, I'm not sure how this articulates with the _WRAP mechanism that patch 1/6 is proposing. If we do this:

default BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL if
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

Quoted 6 lines

Quoted 1 line

then why would we need the _WRAP thing?

The _WRAP thing is needed to make sure there is a user-visible boolean
option in the legacy menu, and to make sure you only select BR2_LEGACY when the old option is not empty.

Correct.
Unlike for bool options that can be manipulated from other options, you cannot manipulate a string option from another option, only from the string option itself by adding a default statement, as Arnout proposed in his earlier mail.

Quoted 7 lines

However, come to think of it, wouldn't it be enough to keep it as a string
option and add

         select BR2_LEGACY if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

? Although I guess Thomas has tested this already.

You mean this, right?
config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
string "linux: the git repository URL option has been renamed" select BR2_LEGACY if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""

I did indeed try this but it doesn't work. Kconfig gives the following message:

Config.in.legacy:75:warning: config symbol 'BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL' uses select, but is not boolean or tristate

Quoted 8 lines

Looking again at the patch, though, I wonder if it works at all. Aren't the
old options that have no symbol definition removed? I think there should still be a

config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL
string

Quoted 6 lines

in Config.in.legacy.

But then, it will not be possible to modify the string anymore... so there
is no escape...

Argh, Kconfig...

If you have the following config snippet before applying the patch:
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL="my_repo_url" BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION="my_repo_version" BR2_LINUX_KERNEL_VERSION="my_repo_version"

and then apply the patch and run 'make menuconfig' and save without changes, you get:

BR2_LINUX_KERNEL_CUSTOM_GIT=y
BR2_LINUX_KERNEL_CUSTOM_REPO_URL=""
BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION=""
BR2_LINUX_KERNEL_VERSION=""
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL_WRAP=y
BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION_WRAP=y

Quoted 4 lines

i.e. the wrap thing does work. It's true that the original option BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL is no longer present _after_ saving the config, but the original value _has_ been detected and used to set the _WRAP option.

It seems this is not entirely correct: if you were not using CUSTOM_GIT options at all, the legacy options would incorrectly be set as well. I think we'll indeed have to re-introduce the original symbol as string in Config.in.legacy, as Arnout mentioned, but a plain config FOO_STRING
string

doesn't seem to work. The original symbol value is not stored. I can make it work with a non-hidden symbol:
config FOO_STRING
string "original string"

but this means that the legacy menu would contain the original strings. Is that a problem?

If it is, then some help with setting this up the right way would be greatly appreciated. I tried several things but can't find a better solution (except for keeping the original symbol names, which I'd like to avoid).

Thanks,
Thomas

Quoted 32 lines · Thomas De Schampheleire

Quoted 19 lines

If you have the following config snippet before applying the patch:
BR2_LINUX_KERNEL_CUSTOM_GIT=y BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL="my_repo_url" BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION="my_repo_version" BR2_LINUX_KERNEL_VERSION="my_repo_version"

and then apply the patch and run 'make menuconfig' and save without changes, you get:

BR2_LINUX_KERNEL_CUSTOM_GIT=y
BR2_LINUX_KERNEL_CUSTOM_REPO_URL=""
BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION=""
BR2_LINUX_KERNEL_VERSION=""
BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL_WRAP=y
BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION_WRAP=y

Quoted 4 lines

i.e. the wrap thing does work. It's true that the original option BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL is no longer present _after_ saving the config, but the original value _has_ been detected and used to set the _WRAP option.

It seems this is not entirely correct: if you were not using CUSTOM_GIT options at all, the legacy options would incorrectly be set as well. I think we'll indeed have to re-introduce the original symbol as string in Config.in.legacy, as Arnout mentioned, but a plain config FOO_STRING
string

Quoted 7 lines

doesn't seem to work. The original symbol value is not stored. I can make it work with a non-hidden symbol:
config FOO_STRING
string "original string"

but this means that the legacy menu would contain the original strings. Is that a problem?

For me, that as such is not much of a problem. In fact, I think you can
make the _WRAP option hidden in that case. So to remove the legacy, you'd have to set FOO_STRING to the empty string again.

config FOO_STRING
string "FOO_STRING has been replaced by BAR_STRING" help
...

config FOO_STRING_WRAP

        bool
        default y if FOO_STRING != ""
        select BR2_LEGACY

However, in this case it is a simple symbol rename, then maybe we don't
need to add anything to the legacy menu. Just add the default to BR2_LINUX_KERNEL_CUSTOM_REPO_URL without defining a symbol for BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL. I'm surprised that that works, though.

To come back on the question: should we try and propagate the options from old to new string options or not.
The original patch did not do that, the user was responsible for making the change.
Arnout responded that he'd like to make it 'just work' for the user, and advocated automatic propagation.
I originally agreed to this reasoning, but am now reconsidering. To implement the automatic propagation of string values, we'd have:

config FOO_NEW_STRING (in linux/Config.in)

         default FOO_OLD_STRING if FOO_OLD_STRING != ""

config FOO_OLD_STRING (in Config.in.legacy)

However, the behavior of this combination is odd: if you start from an old config, update to the newer buildroot, and run make menuconfig, you get the legacy warnings (as expected). In the Kernel menu, the new strings are correctly showing the original values (this is the automatic propagation). In the Legacy menu, the original values are shown too. Everything seems fine up to this point, but there are two scenarios now:
1. to clear the legacy messages, you have to empty the original string in the legacy menu. If you do that, however, the new string in the Kernel menu will also be cleared! If you now save your config, the original string is gone everywhere.
2. if you first save the config (legacy warnings still intact), then reopen menuconfig and clear the legacy warnings, the automatic propagation holds, and now the Kernel menu still contains the original values.

This behavior is very confusing and annoying, IMO. A typical user would perform scenario 1, not knowing about the mentioned problem.

An additional advantage of not automatic propagating, is that there is just one place that deals with legacy options: Config.in.legacy. To remove support for all legacy options, we could just delete that one file and be done with it. However, to automatically propagate string options, we also have to change additional files (linux/Config.in in this example), which is less clean.

Both arguments lead me to advocating not automatically propagating legacy string options.

Quoted 32 lines · Thomas De Schampheleire

To come back on the question: should we try and propagate the options from old to new string options or not.
The original patch did not do that, the user was responsible for making the change.
Arnout responded that he'd like to make it 'just work' for the user, and advocated automatic propagation.
I originally agreed to this reasoning, but am now reconsidering. To implement the automatic propagation of string values, we'd have:

config FOO_NEW_STRING (in linux/Config.in)

         default FOO_OLD_STRING if FOO_OLD_STRING != ""

config FOO_OLD_STRING (in Config.in.legacy)

However, the behavior of this combination is odd: if you start from an old config, update to the newer buildroot, and run make menuconfig, you get the legacy warnings (as expected). In the Kernel menu, the new strings are correctly showing the original values (this is the automatic propagation). In the Legacy menu, the original values are shown too. Everything seems fine up to this point, but there are two scenarios now:
1. to clear the legacy messages, you have to empty the original string in the legacy menu. If you do that, however, the new string in the Kernel menu will also be cleared! If you now save your config, the original string is gone everywhere.
2. if you first save the config (legacy warnings still intact), then reopen menuconfig and clear the legacy warnings, the automatic propagation holds, and now the Kernel menu still contains the original values.

Quoted 2 lines

This behavior is very confusing and annoying, IMO. A typical user would perform scenario 1, not knowing about the mentioned problem.

I agree that it is confusing and annoying. However, this is the same
behaviour as for boolean options. So if we accept this argument, then also the automatic propagation for boolean options should be removed.

So maybe we could make the options with automatic propagation not select
BR2_LEGACY. If it is indeed just a symbol rename, then the user doesn't need to be made aware of it, I guess. The disadvantage is that the old options will still appear in saved defconfigs, but that's a minor thing according to me.

Another option is warn the user in the comments at the top of
Config.in.legacy that they should save and re-run menuconfig.

Quoted 6 lines

An additional advantage of not automatic propagating, is that there is just one place that deals with legacy options: Config.in.legacy. To remove support for all legacy options, we could just delete that one file and be done with it. However, to automatically propagate string options, we also have to change additional files (linux/Config.in in this example), which is less clean.

But I don't think that's a big deal. Config.in.legacy can contain some text
to point to the other place where the symbol has to be removed.

Quoted 2 lines

Both arguments lead me to advocating not automatically propagating legacy string options.

If that is the case, then simple renames of config symbols should still be
avoided, I think. We want it to be really easy for people to upgrade buildroot, so they shouldn't be exposed to the whole legacy stuff unless really needed.

Quoted 32 lines · Thomas De Schampheleire

To come back on the question: should we try and propagate the options from old to new string options or not.
The original patch did not do that, the user was responsible for making the change.
Arnout responded that he'd like to make it 'just work' for the user, and advocated automatic propagation.
I originally agreed to this reasoning, but am now reconsidering. To implement the automatic propagation of string values, we'd have:

config FOO_NEW_STRING (in linux/Config.in)

         default FOO_OLD_STRING if FOO_OLD_STRING != ""

config FOO_OLD_STRING (in Config.in.legacy)

However, the behavior of this combination is odd: if you start from an old config, update to the newer buildroot, and run make menuconfig, you get the legacy warnings (as expected). In the Kernel menu, the new strings are correctly showing the original values (this is the automatic propagation). In the Legacy menu, the original values are shown too. Everything seems fine up to this point, but there are two scenarios now:
1. to clear the legacy messages, you have to empty the original string in the legacy menu. If you do that, however, the new string in the Kernel menu will also be cleared! If you now save your config, the original string is gone everywhere.
2. if you first save the config (legacy warnings still intact), then reopen menuconfig and clear the legacy warnings, the automatic propagation holds, and now the Kernel menu still contains the original values.

Quoted 2 lines

This behavior is very confusing and annoying, IMO. A typical user would perform scenario 1, not knowing about the mentioned problem.

I agree that it is confusing and annoying. However, this is the same
behaviour as for boolean options. So if we accept this argument, then also the automatic propagation for boolean options should be removed.

So maybe we could make the options with automatic propagation not select
BR2_LEGACY. If it is indeed just a symbol rename, then the user doesn't need to be made aware of it, I guess. The disadvantage is that the old options will still appear in saved defconfigs, but that's a minor thing according to me.

Another option is warn the user in the comments at the top of
Config.in.legacy that they should save and re-run menuconfig.

Quoted 10 lines

An additional advantage of not automatic propagating, is that there is just one place that deals with legacy options: Config.in.legacy. To remove support for all legacy options, we could just delete that one file and be done with it. However, to automatically propagate string options, we also have to change additional files (linux/Config.in in this example), which is less clean.

But I don't think that's a big deal. Config.in.legacy can contain some text
to point to the other place where the symbol has to be removed.

Quoted 5 lines

Both arguments lead me to advocating not automatically propagating legacy string options.

If that is the case, then simple renames of config symbols should still be
avoided, I think. We want it to be really easy for people to upgrade buildroot, so they shouldn't be exposed to the whole legacy stuff unless really needed.