* [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories
@ 2013-07-24 7:22 Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:22 UTC (permalink / raw)
To: buildroot
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
v2:
- update based on comments from Peter
- added a few new patches, loosely related to the series.
Config.in.legacy | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
boot/uboot/Config.in | 29 ++++++++++++++++-------------
boot/uboot/uboot.mk | 5 ++++-
linux/Config.in | 31 +++++++++++++++++++------------
linux/linux.mk | 5 ++++-
5 files changed, 96 insertions(+), 32 deletions(-)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
@ 2013-07-24 7:22 ` Thomas De Schampheleire
2013-07-27 13:50 ` Thomas Petazzoni
` (2 more replies)
2013-07-24 7:22 ` [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories Thomas De Schampheleire
` (4 subsequent siblings)
5 siblings, 3 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:22 UTC (permalink / raw)
To: 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 <thomas.de.schampheleire@gmail.com>
---
(v2): new patch in this series
Config.in.legacy | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/Config.in.legacy b/Config.in.legacy
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
@ 2013-07-24 7:22 ` Thomas De Schampheleire
2013-08-12 6:02 ` Arnout Vandecappelle
2013-08-13 10:00 ` Thomas Petazzoni
2013-07-24 7:23 ` [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository Thomas De Schampheleire
` (3 subsequent siblings)
5 siblings, 2 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:22 UTC (permalink / raw)
To: 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 <thomas.de.schampheleire@gmail.com>
---
(v2): new patch in series after comment from Peter
linux/Config.in | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/linux/Config.in b/linux/Config.in
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -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,
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories Thomas De Schampheleire
@ 2013-07-24 7:23 ` Thomas De Schampheleire
2013-08-12 6:12 ` Arnout Vandecappelle
2013-07-24 7:23 ` [Buildroot] [PATCH 4 of 6 v2] u-boot: " Thomas De Schampheleire
` (2 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:23 UTC (permalink / raw)
To: buildroot
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
v2: add Config.in.legacy entries (comment Peter)
Config.in.legacy | 18 ++++++++++++++++++
linux/Config.in | 24 ++++++++++++++++--------
linux/linux.mk | 5 ++++-
3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/Config.in.legacy b/Config.in.legacy
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -59,6 +59,24 @@ endif
###############################################################################
comment "Legacy options removed in 2013.08"
+config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL_WRAP
+ bool "linux: the git repository URL option has been renamed"
+ default y if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""
+ select BR2_LEGACY
+ help
+ The option BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL has
+ been renamed to
+ BR2_LINUX_KERNEL_CUSTOM_REPO_URL.
+
+config BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION_WRAP
+ bool "linux: the git repository version option has been renamed"
+ default y if BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION != ""
+ select BR2_LEGACY
+ help
+ The option BR2_LINUX_KERNEL_CUSTOM_GIT_VERSION has
+ been renamed to
+ BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION.
+
config BR2_PACKAGE_DOSFSTOOLS_DOSFSCK
bool "dosfstools dosfsck renamed to fsck.fat"
select BR2_LEGACY
diff --git a/linux/Config.in b/linux/Config.in
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -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
-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"
+
+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
diff --git a/linux/linux.mk b/linux/linux.mk
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 4 of 6 v2] u-boot: add support for custom Mercurial repository
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
` (2 preceding siblings ...)
2013-07-24 7:23 ` [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository Thomas De Schampheleire
@ 2013-07-24 7:23 ` Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 5 of 6 v2] linux/uboot: line-up repository-related configuration options Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 6 of 6 v2] linux: mention 3.x.y kernels in 'custom version' help Thomas De Schampheleire
5 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:23 UTC (permalink / raw)
To: buildroot
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
v2: add Config.in.legacy entries (comment Peter)
Config.in.legacy | 18 ++++++++++++++++++
boot/uboot/Config.in | 15 +++++++++------
boot/uboot/uboot.mk | 5 ++++-
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Config.in.legacy b/Config.in.legacy
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -59,6 +59,24 @@ endif
###############################################################################
comment "Legacy options removed in 2013.08"
+config BR2_TARGET_UBOOT_CUSTOM_GIT_REPO_URL_WRAP
+ bool "u-boot: the git repository URL option has been renamed"
+ default y if BR2_TARGET_UBOOT_CUSTOM_GIT_REPO_URL != ""
+ select BR2_LEGACY
+ help
+ The option BR2_TARGET_UBOOT_CUSTOM_GIT_REPO_URL has
+ been renamed to
+ BR2_TARGET_UBOOT_CUSTOM_REPO_URL.
+
+config BR2_TARGET_UBOOT_CUSTOM_GIT_VERSION_WRAP
+ bool "u-boot: the git repository version option has been renamed"
+ default y if BR2_TARGET_UBOOT_CUSTOM_GIT_VERSION != ""
+ select BR2_LEGACY
+ help
+ The option BR2_TARGET_UBOOT_CUSTOM_GIT_VERSION has
+ been renamed to
+ BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION.
+
config BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL_WRAP
bool "linux: the git repository URL option has been renamed"
default y if BR2_LINUX_KERNEL_CUSTOM_GIT_REPO_URL != ""
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -31,6 +31,9 @@ config BR2_TARGET_UBOOT_CUSTOM_TARBALL
config BR2_TARGET_UBOOT_CUSTOM_GIT
bool "Custom Git repository"
+config BR2_TARGET_UBOOT_CUSTOM_HG
+ bool "Custom Mercurial repository"
+
endchoice
config BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE
@@ -49,7 +52,7 @@ config BR2_TARGET_UBOOT_VERSION
default "2013.04" if BR2_TARGET_UBOOT_LATEST_VERSION
default $BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE if BR2_TARGET_UBOOT_CUSTOM_VERSION
default "custom" if BR2_TARGET_UBOOT_CUSTOM_TARBALL
- default $BR2_TARGET_UBOOT_CUSTOM_GIT_VERSION if BR2_TARGET_UBOOT_CUSTOM_GIT
+ default $BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION if BR2_TARGET_UBOOT_CUSTOM_GIT || BR2_TARGET_UBOOT_CUSTOM_HG
config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
string "custom patch dir"
@@ -60,13 +63,13 @@ config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
Most users may leave this empty
-if BR2_TARGET_UBOOT_CUSTOM_GIT
+if BR2_TARGET_UBOOT_CUSTOM_GIT || BR2_TARGET_UBOOT_CUSTOM_HG
-config BR2_TARGET_UBOOT_CUSTOM_GIT_REPO_URL
- string "URL of custom Git repository"
+config BR2_TARGET_UBOOT_CUSTOM_REPO_URL
+ string "URL of custom repository"
-config BR2_TARGET_UBOOT_CUSTOM_GIT_VERSION
- string "Custom Git version"
+config BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION
+ string "Custom repository version"
endif
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -18,8 +18,11 @@ UBOOT_TARBALL = $(call qstrip,$(BR2_TARG
UBOOT_SITE = $(patsubst %/,%,$(dir $(UBOOT_TARBALL)))
UBOOT_SOURCE = $(notdir $(UBOOT_TARBALL))
else ifeq ($(BR2_TARGET_UBOOT_CUSTOM_GIT),y)
-UBOOT_SITE = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_GIT_REPO_URL))
+UBOOT_SITE = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_REPO_URL))
UBOOT_SITE_METHOD = git
+else ifeq ($(BR2_TARGET_UBOOT_CUSTOM_HG),y)
+UBOOT_SITE = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_REPO_URL))
+UBOOT_SITE_METHOD = hg
else
# Handle stable official U-Boot versions
UBOOT_SITE = ftp://ftp.denx.de/pub/u-boot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 5 of 6 v2] linux/uboot: line-up repository-related configuration options
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
` (3 preceding siblings ...)
2013-07-24 7:23 ` [Buildroot] [PATCH 4 of 6 v2] u-boot: " Thomas De Schampheleire
@ 2013-07-24 7:23 ` Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 6 of 6 v2] linux: mention 3.x.y kernels in 'custom version' help Thomas De Schampheleire
5 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:23 UTC (permalink / raw)
To: buildroot
Although the configuration options for custom repository locations and
versions are very similar between the linux and uboot packages, there are
some minor differences. This patch lines up both packages.
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
v2:
- remove 'HEAD/tip' defaults for repository version (comment Peter)
- replace if/config/endif with config/depends for uboot tarball
- move repo url/version settings before unrelated 'custom patch dir'
Note: one remaining difference is that there are help texts in linux for
options BR2_LINUX_KERNEL_CUSTOM_GIT and BR2_LINUX_KERNEL_CUSTOM_HG. I
didn't add them to uboot because it seems a bit redundant, but if you
prefer we can line this up as well (either add help texts to uboot or
remove them from linux).
boot/uboot/Config.in | 24 ++++++++++++------------
linux/Config.in | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -40,10 +40,20 @@ config BR2_TARGET_UBOOT_CUSTOM_VERSION_V
string "U-Boot version"
depends on BR2_TARGET_UBOOT_CUSTOM_VERSION
-if BR2_TARGET_UBOOT_CUSTOM_TARBALL
-
config BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION
string "URL of custom U-Boot tarball"
+ depends on BR2_TARGET_UBOOT_CUSTOM_TARBALL
+
+if BR2_TARGET_UBOOT_CUSTOM_GIT || BR2_TARGET_UBOOT_CUSTOM_HG
+
+config BR2_TARGET_UBOOT_CUSTOM_REPO_URL
+ string "URL of custom repository"
+
+config BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION
+ string "Custom repository version"
+ help
+ Revision to use in the typical format used by Git/Mercurial
+ E.G. a sha id, a tag, branch, ..
endif
@@ -63,16 +73,6 @@ config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
Most users may leave this empty
-if BR2_TARGET_UBOOT_CUSTOM_GIT || BR2_TARGET_UBOOT_CUSTOM_HG
-
-config BR2_TARGET_UBOOT_CUSTOM_REPO_URL
- string "URL of custom repository"
-
-config BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION
- string "Custom repository version"
-
-endif
-
choice
prompt "U-Boot binary format"
default BR2_TARGET_UBOOT_FORMAT_BIN
diff --git a/linux/Config.in b/linux/Config.in
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -47,7 +47,7 @@ config BR2_LINUX_KERNEL_CUSTOM_TARBALL
specific kernel source tarball
config BR2_LINUX_KERNEL_CUSTOM_GIT
- bool "Custom Git tree"
+ bool "Custom Git repository"
help
This option allows Buildroot to get the Linux kernel source
code from a Git repository.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 6 of 6 v2] linux: mention 3.x.y kernels in 'custom version' help
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
` (4 preceding siblings ...)
2013-07-24 7:23 ` [Buildroot] [PATCH 5 of 6 v2] linux/uboot: line-up repository-related configuration options Thomas De Schampheleire
@ 2013-07-24 7:23 ` Thomas De Schampheleire
5 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-07-24 7:23 UTC (permalink / raw)
To: buildroot
(also fix grammatical error versions -> version)
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
(v2): new patch in series, although only loosely related.
linux/Config.in | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/linux/Config.in b/linux/Config.in
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -37,8 +37,8 @@ config BR2_LINUX_KERNEL_SAME_AS_HEADERS
config BR2_LINUX_KERNEL_CUSTOM_VERSION
bool "Custom version"
help
- This option allows to use a specific 2.6.x or 2.6.x.y
- official versions, as available on kernel.org
+ This option allows to use a specific official version from
+ kernel.org, like 2.6.x, 2.6.x.y, 3.x.y, ...
config BR2_LINUX_KERNEL_CUSTOM_TARBALL
bool "Custom tarball"
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
@ 2013-07-27 13:50 ` Thomas Petazzoni
2013-08-12 5:59 ` Arnout Vandecappelle
2013-08-13 16:31 ` Arnout Vandecappelle
2 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni @ 2013-07-27 13:50 UTC (permalink / raw)
To: buildroot
Hello Arnout,
As the author of the Config.in.legacy mechanism, it would be great if
you could review/ack the below patch.
Thanks,
Thomas
On Wed, 24 Jul 2013 09:22:58 +0200, Thomas De Schampheleire wrote:
> 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 <thomas.de.schampheleire@gmail.com>
>
> ---
> (v2): new patch in this series
>
> Config.in.legacy | 22 +++++++++++++++++-----
> 1 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/Config.in.legacy b/Config.in.legacy
> --- a/Config.in.legacy
> +++ b/Config.in.legacy
> @@ -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
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
2013-07-27 13:50 ` Thomas Petazzoni
@ 2013-08-12 5:59 ` Arnout Vandecappelle
2013-08-13 16:31 ` Arnout Vandecappelle
2 siblings, 0 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-12 5:59 UTC (permalink / raw)
To: buildroot
On 24/07/13 09:22, Thomas De Schampheleire wrote:
> 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 <thomas.de.schampheleire@gmail.com>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Note, however, that when you change a string option, it is quite
difficult to propagate the old value to the new symbol automatically
(you'd have to add a default to the new option). So renaming string
options should be avoided.
Regards,
Arnout
>
> ---
> (v2): new patch in this series
>
> Config.in.legacy | 22 +++++++++++++++++-----
> 1 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/Config.in.legacy b/Config.in.legacy
> --- a/Config.in.legacy
> +++ b/Config.in.legacy
> @@ -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
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories
2013-07-24 7:22 ` [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories Thomas De Schampheleire
@ 2013-08-12 6:02 ` Arnout Vandecappelle
2013-08-13 10:00 ` Thomas Petazzoni
1 sibling, 0 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-12 6:02 UTC (permalink / raw)
To: buildroot
On 24/07/13 09:22, Thomas De Schampheleire wrote:
> 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 <thomas.de.schampheleire@gmail.com>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
I would still push this one in 2013.08.
Regards,
Arnout
>
> ---
> (v2): new patch in series after comment from Peter
>
> linux/Config.in | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/linux/Config.in b/linux/Config.in
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -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
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-07-24 7:23 ` [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository Thomas De Schampheleire
@ 2013-08-12 6:12 ` Arnout Vandecappelle
2013-08-12 10:54 ` Thomas De Schampheleire
2013-08-13 9:09 ` Thomas Petazzoni
0 siblings, 2 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-12 6:12 UTC (permalink / raw)
To: buildroot
On 24/07/13 09:23, Thomas De Schampheleire wrote:
[snip]
> diff --git a/linux/Config.in b/linux/Config.in
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -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
>
> -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).
Regards,
Arnout
> +
> +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
> diff --git a/linux/linux.mk b/linux/linux.mk
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -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
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-12 6:12 ` Arnout Vandecappelle
@ 2013-08-12 10:54 ` Thomas De Schampheleire
2013-08-12 11:01 ` Arnout Vandecappelle
2013-08-13 9:09 ` Thomas Petazzoni
1 sibling, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-12 10:54 UTC (permalink / raw)
To: buildroot
Hi Arnout,
On Mon, Aug 12, 2013 at 8:12 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 24/07/13 09:23, Thomas De Schampheleire wrote:
> [snip]
>> diff --git a/linux/Config.in b/linux/Config.in
>> --- a/linux/Config.in
>> +++ b/linux/Config.in
>> @@ -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
>>
>> -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).
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).
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-12 10:54 ` Thomas De Schampheleire
@ 2013-08-12 11:01 ` Arnout Vandecappelle
0 siblings, 0 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-12 11:01 UTC (permalink / raw)
To: buildroot
On 12/08/13 12:54, Thomas De Schampheleire wrote:
> Hi Arnout,
>
> On Mon, Aug 12, 2013 at 8:12 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
[snip]
>> 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).
>
> 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).
Sounds OK to me.
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-12 6:12 ` Arnout Vandecappelle
2013-08-12 10:54 ` Thomas De Schampheleire
@ 2013-08-13 9:09 ` Thomas Petazzoni
2013-08-13 16:30 ` Arnout Vandecappelle
1 sibling, 1 reply; 25+ messages in thread
From: Thomas Petazzoni @ 2013-08-13 9:09 UTC (permalink / raw)
To: buildroot
Dear Arnout Vandecappelle,
On Mon, 12 Aug 2013 08:12:13 +0200, Arnout Vandecappelle wrote:
> 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?
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories
2013-07-24 7:22 ` [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories Thomas De Schampheleire
2013-08-12 6:02 ` Arnout Vandecappelle
@ 2013-08-13 10:00 ` Thomas Petazzoni
1 sibling, 0 replies; 25+ messages in thread
From: Thomas Petazzoni @ 2013-08-13 10:00 UTC (permalink / raw)
To: buildroot
Dear Thomas De Schampheleire,
On Wed, 24 Jul 2013 09:22:59 +0200, Thomas De Schampheleire wrote:
> 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 <thomas.de.schampheleire@gmail.com>
Applied, thanks.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-13 9:09 ` Thomas Petazzoni
@ 2013-08-13 16:30 ` Arnout Vandecappelle
2013-08-14 17:06 ` Thomas De Schampheleire
0 siblings, 1 reply; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-13 16:30 UTC (permalink / raw)
To: buildroot
On 13/08/13 11:09, Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
>
> On Mon, 12 Aug 2013 08:12:13 +0200, Arnout Vandecappelle wrote:
>
>> 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?
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...
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
2013-07-27 13:50 ` Thomas Petazzoni
2013-08-12 5:59 ` Arnout Vandecappelle
@ 2013-08-13 16:31 ` Arnout Vandecappelle
2013-08-14 16:27 ` Thomas De Schampheleire
2 siblings, 1 reply; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-13 16:31 UTC (permalink / raw)
To: buildroot
On 24/07/13 09:22, Thomas De Schampheleire wrote:
[snip]
> +# 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
My Ack was too hasty, this should be
select BR2_LEGACY
Regards,
Arnout
> +# help
> +# <suitable help text>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description
2013-08-13 16:31 ` Arnout Vandecappelle
@ 2013-08-14 16:27 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-14 16:27 UTC (permalink / raw)
To: buildroot
On Tue, Aug 13, 2013 at 6:31 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 24/07/13 09:22, Thomas De Schampheleire wrote:
> [snip]
>
>> +# 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
>
>
> My Ack was too hasty, this should be
> select BR2_LEGACY
>
(ashamed) You're absolutely right...
Thanks for noticing,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-13 16:30 ` Arnout Vandecappelle
@ 2013-08-14 17:06 ` Thomas De Schampheleire
2013-08-17 8:13 ` Thomas De Schampheleire
0 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-14 17:06 UTC (permalink / raw)
To: buildroot
Hi,
On Tue, Aug 13, 2013 at 6:30 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 13/08/13 11:09, Thomas Petazzoni wrote:
>>
>> Dear Arnout Vandecappelle,
>>
>> On Mon, 12 Aug 2013 08:12:13 +0200, Arnout Vandecappelle wrote:
>>
>>> 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?
>
>
> 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.
>
> 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
>
> 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...
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.
The above behavior is the most you can achieve from the
Config.in.legacy file. If you want to automatically propagate the
string values, you _have_ to do it from linux/linux.mk. As this is the
behavior a user would expect, I will update my patch with that change.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-14 17:06 ` Thomas De Schampheleire
@ 2013-08-17 8:13 ` Thomas De Schampheleire
2013-08-19 16:05 ` Arnout Vandecappelle
0 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-17 8:13 UTC (permalink / raw)
To: buildroot
On Wed, Aug 14, 2013 at 7:06 PM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
> Hi,
>
> On Tue, Aug 13, 2013 at 6:30 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 13/08/13 11:09, Thomas Petazzoni wrote:
>>>
>>> Dear Arnout Vandecappelle,
>>>
>>> On Mon, 12 Aug 2013 08:12:13 +0200, Arnout Vandecappelle wrote:
>>>
>>>> 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?
>>
>>
>> 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.
>
>>
>> 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
>
>>
>> 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...
>
> 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.
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-17 8:13 ` Thomas De Schampheleire
@ 2013-08-19 16:05 ` Arnout Vandecappelle
2013-08-20 8:36 ` Thomas De Schampheleire
0 siblings, 1 reply; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-19 16:05 UTC (permalink / raw)
To: buildroot
On 17/08/13 10:13, Thomas De Schampheleire wrote:
> On Wed, Aug 14, 2013 at 7:06 PM, Thomas De Schampheleire
> <patrickdepinguin+buildroot@gmail.com> wrote:
>> Hi,
>>
>> On Tue, Aug 13, 2013 at 6:30 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 13/08/13 11:09, Thomas Petazzoni wrote:
>>>>
>>>> Dear Arnout Vandecappelle,
>>>>
>>>> On Mon, 12 Aug 2013 08:12:13 +0200, Arnout Vandecappelle wrote:
>>>>
>>>>> 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?
>>>
>>>
>>> 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.
>>
>>>
>>> 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
>>
>>>
>>> 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...
>>
>> 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.
>
> 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?
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.
Regards,
Arnout
> 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
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-19 16:05 ` Arnout Vandecappelle
@ 2013-08-20 8:36 ` Thomas De Schampheleire
2013-08-21 5:40 ` Arnout Vandecappelle
0 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-20 8:36 UTC (permalink / raw)
To: buildroot
On Mon, Aug 19, 2013 at 6:05 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 17/08/13 10:13, Thomas De Schampheleire wrote:
>>
>>>
>>>
>>> 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.
>>
>>
>> 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?
>
>
> 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
>
>
Thanks a lot for this suggestion. It indeed seems to work, and I like
it: the original string values are still visible somewhere, and there
is just one line in the legacy menu.
>
>
> 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.
It doesn't, I made a mistake there. You still need the original option
somewhere.
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.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-20 8:36 ` Thomas De Schampheleire
@ 2013-08-21 5:40 ` Arnout Vandecappelle
2013-08-27 12:29 ` Thomas De Schampheleire
2013-08-27 14:14 ` Thomas De Schampheleire
0 siblings, 2 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2013-08-21 5:40 UTC (permalink / raw)
To: buildroot
On 08/20/13 10:36, Thomas De Schampheleire wrote:
> 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.
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.
> 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.
> 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.
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-21 5:40 ` Arnout Vandecappelle
@ 2013-08-27 12:29 ` Thomas De Schampheleire
2013-08-27 14:14 ` Thomas De Schampheleire
1 sibling, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-27 12:29 UTC (permalink / raw)
To: buildroot
All,
On Wed, Aug 21, 2013 at 7:40 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/20/13 10:36, Thomas De Schampheleire wrote:
>>
>> 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.
>
>
> 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.
>
>
>
>
>> 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.
>
>
>
>> 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.
>
What is the input of the buildroot community on this thread?
Thanks,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository
2013-08-21 5:40 ` Arnout Vandecappelle
2013-08-27 12:29 ` Thomas De Schampheleire
@ 2013-08-27 14:14 ` Thomas De Schampheleire
1 sibling, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2013-08-27 14:14 UTC (permalink / raw)
To: buildroot
Hi Arnout,
On Wed, Aug 21, 2013 at 7:40 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/20/13 10:36, Thomas De Schampheleire wrote:
>>
>> 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.
>
>
> 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.
>
I wasn't aware that this behavior was also there for bool options. I
agree that it doesn't make sense to make strings behave 'normal' if it
isn't true for bool options (which are in the majority).
> 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.
I can't make it work in this case. If you just remove 'select
BR2_LEGACY' from a legacy bool option, it is still visible in the
legacy menu, and users will tend to remove it if they already know how
the legacy menu works. Here, you still have to save the config in
between to avoid losing an option.
If you remove the visibility of the legacy option, I don't get
automatic propagation (I may be doing something wrong).
>
> Another option is warn the user in the comments at the top of
> Config.in.legacy that they should save and re-run menuconfig.
This is certainly acceptable to me.
>
>
>
>
>> 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.
I will certainly not block this if others prefer the automatic
propagation, it's indeed not a showstopper. But I'd really like to
hear some other opinions on this.
>
>
>
>> 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.
Unnecessary renames should indeed be avoided, but in this case of
GIT/HG options I don't feel it's unnecessary.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-08-27 14:14 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-24 7:22 [Buildroot] [PATCH 0 of 6 v2] linux/uboot: add support for custom Mercurial repositories Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 1 of 6 v2] Config.in.legacy: update description Thomas De Schampheleire
2013-07-27 13:50 ` Thomas Petazzoni
2013-08-12 5:59 ` Arnout Vandecappelle
2013-08-13 16:31 ` Arnout Vandecappelle
2013-08-14 16:27 ` Thomas De Schampheleire
2013-07-24 7:22 ` [Buildroot] [PATCH 2 of 6 v2] linux: don't take HEAD as default for git repositories Thomas De Schampheleire
2013-08-12 6:02 ` Arnout Vandecappelle
2013-08-13 10:00 ` Thomas Petazzoni
2013-07-24 7:23 ` [Buildroot] [PATCH 3 of 6 v2] linux: add support for custom Mercurial repository Thomas De Schampheleire
2013-08-12 6:12 ` Arnout Vandecappelle
2013-08-12 10:54 ` Thomas De Schampheleire
2013-08-12 11:01 ` Arnout Vandecappelle
2013-08-13 9:09 ` Thomas Petazzoni
2013-08-13 16:30 ` Arnout Vandecappelle
2013-08-14 17:06 ` Thomas De Schampheleire
2013-08-17 8:13 ` Thomas De Schampheleire
2013-08-19 16:05 ` Arnout Vandecappelle
2013-08-20 8:36 ` Thomas De Schampheleire
2013-08-21 5:40 ` Arnout Vandecappelle
2013-08-27 12:29 ` Thomas De Schampheleire
2013-08-27 14:14 ` Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 4 of 6 v2] u-boot: " Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 5 of 6 v2] linux/uboot: line-up repository-related configuration options Thomas De Schampheleire
2013-07-24 7:23 ` [Buildroot] [PATCH 6 of 6 v2] linux: mention 3.x.y kernels in 'custom version' help Thomas De Schampheleire
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox