| Message ID | 1460971383-10595-4-git-send-email-romain.izard.pro@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Romain, All, On 2016-04-18 11:23 +0200, Romain Izard spake thusly: > Using 'yes "" | make oldconfig' to regenerate a .config file from an > existing defconfig does not work reliably in all cases. Specifically, it > does not work well with tristate choice entries. > > The correct way to do it is to use 'make olddefconfig', but this target > is not supported in projects that use an old version of kconfig. As > most projects do not use tristate entries, there is no need for them to > upgrade. > > 'oldnoconfig' is an alias for 'olddefconfig', but it also is the only > name for this target between Linux versions 2.6.36 and 3.6. > > Try to use 'oldnoconfig' by default, using 'oldconfig' as a fallback > > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> Usually, reviewd-by tags are staged after the signed-off-by tag, as in a temporal mark: first signed-off-by, then reviewed. > --- > Changelog: > v1: New patch > v2: Use oldnoconfig instead of olddefconfig > Fix silent mode > Change the pattern in both places where it is used > Use a macro to avoid redundancy > v3: Do not use $(call ...) > Use the $(2)_KCONFIG_MAKE variable > v4: No change > > package/pkg-kconfig.mk | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk > index 4033915..e04cfd5 100644 > --- a/package/pkg-kconfig.mk > +++ b/package/pkg-kconfig.mk > @@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch > $(2)_KCONFIG_MAKE = \ > $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS) > > +# The correct way to regenerate a .config file is to use 'make olddefconfig'. > +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel > +# versions 2.6.36 and 3.6, and remains as an alias in later versions. > +# In older versions, and in some other projects that use kconfig, the target is > +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback > +# only, as this can fail in complex cases. > +define $(2)_REGEN_DOT_CONFIG > + $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \ > + (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) What I don't much like here, is that we conflate two things: - a non-zero return code because of a missing oldnoconfig, - a non-zero return code because of an error found by olnoconfig. What I would have loved however, would be a way to detect whether oldnoconfig (better, olddefconfig) was supported before calling it. Maybe something along the lines of: if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ else (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ fi Yes, this is more code, less compact, but it is more systematic. At the very least, it would not fallback to the yes|oldconfig trick in case old{def,no}config do exist but fail, which we do want to catch. BTW, the make trick above is highly (as in: almost identical) from the bash-autocompletion package. Regards, Yann E. MORIN. > +endef > + > # The specified source configuration file and any additional configuration file > # fragments are merged together to .config, after the package has been patched. > # Since the file could be a defconfig file it needs to be expanded to a > -# full .config first. We use 'make oldconfig' because this can be safely > -# done even when the package does not support defconfigs. > +# full .config first. > $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES) > $$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \ > $$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \ > cp $$($(2)_KCONFIG_FILE) $$(@)) > $$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \ > $$(@) $$($(2)_KCONFIG_FRAGMENT_FILES) > - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig > + $$($(2)_REGEN_DOT_CONFIG) > > # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is > # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add > @@ -82,7 +92,7 @@ $$($(2)_DIR)/.config: | $(1)-patch > # The exact rules are specified by the package .mk file. > define $(2)_FIXUP_DOT_CONFIG > $$($(2)_KCONFIG_FIXUP_CMDS) > - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig > + $$($(2)_REGEN_DOT_CONFIG) > $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done > endef > > -- > 2.5.0 >
Romain, All, On 2016-04-18 23:08 +0200, Yann E. MORIN spake thusly: > On 2016-04-18 11:23 +0200, Romain Izard spake thusly: > > Using 'yes "" | make oldconfig' to regenerate a .config file from an > > existing defconfig does not work reliably in all cases. Specifically, it > > does not work well with tristate choice entries. > > > > The correct way to do it is to use 'make olddefconfig', but this target > > is not supported in projects that use an old version of kconfig. As > > most projects do not use tristate entries, there is no need for them to > > upgrade. > > > > 'oldnoconfig' is an alias for 'olddefconfig', but it also is the only > > name for this target between Linux versions 2.6.36 and 3.6. > > > > Try to use 'oldnoconfig' by default, using 'oldconfig' as a fallback > > > > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> > > Usually, reviewd-by tags are staged after the signed-off-by tag, as in a > temporal mark: first signed-off-by, then reviewed. > > > --- > > Changelog: > > v1: New patch > > v2: Use oldnoconfig instead of olddefconfig > > Fix silent mode > > Change the pattern in both places where it is used > > Use a macro to avoid redundancy > > v3: Do not use $(call ...) > > Use the $(2)_KCONFIG_MAKE variable > > v4: No change > > > > package/pkg-kconfig.mk | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk > > index 4033915..e04cfd5 100644 > > --- a/package/pkg-kconfig.mk > > +++ b/package/pkg-kconfig.mk > > @@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch > > $(2)_KCONFIG_MAKE = \ > > $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS) > > > > +# The correct way to regenerate a .config file is to use 'make olddefconfig'. > > +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel > > +# versions 2.6.36 and 3.6, and remains as an alias in later versions. > > +# In older versions, and in some other projects that use kconfig, the target is > > +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback > > +# only, as this can fail in complex cases. > > +define $(2)_REGEN_DOT_CONFIG > > + $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \ > > + (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) > > What I don't much like here, is that we conflate two things: > - a non-zero return code because of a missing oldnoconfig, > - a non-zero return code because of an error found by olnoconfig. > > What I would have loved however, would be a way to detect whether > oldnoconfig (better, olddefconfig) was supported before calling it. > Maybe something along the lines of: > > if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ Hmmm, incomplete (missing colon). Should have been: ... |grep -E '^olddefconfig:' >/dev/null > $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ > elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ Ditto, with oldnoconfig... Regards, Yann E. MORIN. > $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ > else > (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ > fi > > Yes, this is more code, less compact, but it is more systematic. At the > very least, it would not fallback to the yes|oldconfig trick in case > old{def,no}config do exist but fail, which we do want to catch. > > BTW, the make trick above is highly (as in: almost identical) from the > bash-autocompletion package. > > Regards, > Yann E. MORIN. > > > +endef > > + > > # The specified source configuration file and any additional configuration file > > # fragments are merged together to .config, after the package has been patched. > > # Since the file could be a defconfig file it needs to be expanded to a > > -# full .config first. We use 'make oldconfig' because this can be safely > > -# done even when the package does not support defconfigs. > > +# full .config first. > > $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES) > > $$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \ > > $$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \ > > cp $$($(2)_KCONFIG_FILE) $$(@)) > > $$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \ > > $$(@) $$($(2)_KCONFIG_FRAGMENT_FILES) > > - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig > > + $$($(2)_REGEN_DOT_CONFIG) > > > > # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is > > # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add > > @@ -82,7 +92,7 @@ $$($(2)_DIR)/.config: | $(1)-patch > > # The exact rules are specified by the package .mk file. > > define $(2)_FIXUP_DOT_CONFIG > > $$($(2)_KCONFIG_FIXUP_CMDS) > > - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig > > + $$($(2)_REGEN_DOT_CONFIG) > > $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done > > endef > > > > -- > > 2.5.0 > > > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------' > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi Yann, 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>: > Romain, All, > > On 2016-04-18 11:23 +0200, Romain Izard spake thusly: > >> +# The correct way to regenerate a .config file is to use 'make olddefconfig'. >> +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel >> +# versions 2.6.36 and 3.6, and remains as an alias in later versions. >> +# In older versions, and in some other projects that use kconfig, the target is >> +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback >> +# only, as this can fail in complex cases. >> +define $(2)_REGEN_DOT_CONFIG >> + $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \ >> + (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) > > What I don't much like here, is that we conflate two things: > - a non-zero return code because of a missing oldnoconfig, > - a non-zero return code because of an error found by olnoconfig. > > What I would have loved however, would be a way to detect whether > oldnoconfig (better, olddefconfig) was supported before calling it. > Maybe something along the lines of: > > if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ > $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ > elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ > $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ > else > (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ > fi > > Yes, this is more code, less compact, but it is more systematic. At > the very least, it would not fallback to the yes|oldconfig trick in > case old{def,no}config do exist but fail, which we do want to catch. > > BTW, the make trick above is highly (as in: almost identical) from the > bash-autocompletion package. I see the idea, but the specific implementation is not working: the kernel's Makefile targets are dynamic, and the targets we want to check are only present when the target we try to build matches %config. I will try to find a syntax that works for all projects that use the kconfig-package infrastructure. Best regards,
Romain, All, On 2016-04-19 11:11 +0200, Romain Izard spake thusly: > 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>: > > Romain, All, > > > > On 2016-04-18 11:23 +0200, Romain Izard spake thusly: > > > >> +# The correct way to regenerate a .config file is to use 'make olddefconfig'. > >> +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel > >> +# versions 2.6.36 and 3.6, and remains as an alias in later versions. > >> +# In older versions, and in some other projects that use kconfig, the target is > >> +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback > >> +# only, as this can fail in complex cases. > >> +define $(2)_REGEN_DOT_CONFIG > >> + $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \ > >> + (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) > > > > What I don't much like here, is that we conflate two things: > > - a non-zero return code because of a missing oldnoconfig, > > - a non-zero return code because of an error found by olnoconfig. > > > > What I would have loved however, would be a way to detect whether > > oldnoconfig (better, olddefconfig) was supported before calling it. > > Maybe something along the lines of: > > > > if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ > > $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ > > elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ > > $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ > > else > > (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ > > fi > > > > Yes, this is more code, less compact, but it is more systematic. At > > the very least, it would not fallback to the yes|oldconfig trick in > > case old{def,no}config do exist but fail, which we do want to catch. > > > > BTW, the make trick above is highly (as in: almost identical) from the > > bash-autocompletion package. > > I see the idea, but the specific implementation is not working: the > kernel's Makefile targets are dynamic, and the targets we want to check > are only present when the target we try to build matches %config. I just checked, and it works on the current master on the kernel tree: $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:' olddefconfig: scripts/kconfig/conf $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:' oldnoconfig: olddefconfig Regards, Yann E. MORIN. > I will try to find a syntax that works for all projects that use the > kconfig-package infrastructure. > > Best regards, > -- > Romain Izard
Romain, All, On 2016-04-19 19:40 +0200, Yann E. MORIN spake thusly: > On 2016-04-19 11:11 +0200, Romain Izard spake thusly: > > 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>: [--SNIP--] > > > What I would have loved however, would be a way to detect whether > > > oldnoconfig (better, olddefconfig) was supported before calling it. > > > Maybe something along the lines of: > > > > > > if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ > > > $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ > > > elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ > > > $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ > > > else > > > (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ > > > fi > > > > > > Yes, this is more code, less compact, but it is more systematic. At > > > the very least, it would not fallback to the yes|oldconfig trick in > > > case old{def,no}config do exist but fail, which we do want to catch. > > > > > > BTW, the make trick above is highly (as in: almost identical) from the > > > bash-autocompletion package. > > > > I see the idea, but the specific implementation is not working: the > > kernel's Makefile targets are dynamic, and the targets we want to check > > are only present when the target we try to build matches %config. > > I just checked, and it works on the current master on the kernel tree: > > $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:' > olddefconfig: scripts/kconfig/conf > > $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:' > oldnoconfig: olddefconfig I also checked on v3.6 (which is missing olddefconfig) and v3.7 (which introduced olddefconfig), and the tests above still work: oldnconfig is found, olddefconfig is not. Regards, Yann E. MORIN.
2016-04-19 19:58 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>: > Romain, All, > > On 2016-04-19 19:40 +0200, Yann E. MORIN spake thusly: >> On 2016-04-19 11:11 +0200, Romain Izard spake thusly: >> > 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>: > [--SNIP--] >> > > What I would have loved however, would be a way to detect whether >> > > oldnoconfig (better, olddefconfig) was supported before calling it. >> > > Maybe something along the lines of: >> > > >> > > if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \ >> > > $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \ >> > > elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \ >> > > $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \ >> > > else >> > > (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \ >> > > fi >> > > >> > > Yes, this is more code, less compact, but it is more systematic. At >> > > the very least, it would not fallback to the yes|oldconfig trick in >> > > case old{def,no}config do exist but fail, which we do want to catch. >> > > >> > > BTW, the make trick above is highly (as in: almost identical) from the >> > > bash-autocompletion package. >> > >> > I see the idea, but the specific implementation is not working: the >> > kernel's Makefile targets are dynamic, and the targets we want to check >> > are only present when the target we try to build matches %config. >> >> I just checked, and it works on the current master on the kernel tree: >> >> $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:' >> olddefconfig: scripts/kconfig/conf >> >> $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:' >> oldnoconfig: olddefconfig > > I also checked on v3.6 (which is missing olddefconfig) and v3.7 (which > introduced olddefconfig), and the tests above still work: oldnconfig is > found, olddefconfig is not. > When I tried it yesterday it did not work, but it works today. I must have done something wrong when testing it. Best regards,
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index 4033915..e04cfd5 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch $(2)_KCONFIG_MAKE = \ $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS) +# The correct way to regenerate a .config file is to use 'make olddefconfig'. +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel +# versions 2.6.36 and 3.6, and remains as an alias in later versions. +# In older versions, and in some other projects that use kconfig, the target is +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback +# only, as this can fail in complex cases. +define $(2)_REGEN_DOT_CONFIG + $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \ + (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) +endef + # The specified source configuration file and any additional configuration file # fragments are merged together to .config, after the package has been patched. # Since the file could be a defconfig file it needs to be expanded to a -# full .config first. We use 'make oldconfig' because this can be safely -# done even when the package does not support defconfigs. +# full .config first. $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES) $$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \ $$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \ cp $$($(2)_KCONFIG_FILE) $$(@)) $$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \ $$(@) $$($(2)_KCONFIG_FRAGMENT_FILES) - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig + $$($(2)_REGEN_DOT_CONFIG) # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add @@ -82,7 +92,7 @@ $$($(2)_DIR)/.config: | $(1)-patch # The exact rules are specified by the package .mk file. define $(2)_FIXUP_DOT_CONFIG $$($(2)_KCONFIG_FIXUP_CMDS) - $$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig + $$($(2)_REGEN_DOT_CONFIG) $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done endef