Message ID | 20230416151310.2698912-1-marcus.folkesson@gmail.com |
---|---|
State | Rejected |
Headers | show |
Series | [RESEND,v2,1/2] package/pkg-kconfig.mk new <pkg>-show-fragment target | expand |
Marcus, All, On 2023-04-16 17:13 +0200, Marcus Folkesson spake thusly: > This patch introduce the new target to simplify the generation of > configuration fragments as the output could be directly copied into a > fragment file as-is. > > It is *heavily* based on the <pkg>-diff-config target, but serves a > different purpose. > > Output from linux-diff-config: > -CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK y > -CONFIG_GCC_PLUGIN_CYC_COMPLEXITY n > -CONFIG_GCC_PLUGIN_LATENT_ENTROPY n > -CONFIG_GCC_PLUGIN_RANDSTRUCT n > -CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF n > -CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL n > -CONFIG_GCC_PLUGIN_STRUCTLEAK_USER n > -CONFIG_STACKPROTECTOR_PER_TASK y > CONFIG_CRYPTO_DH n -> y > CONFIG_CRYPTO_KPP m -> y > CONFIG_GCC_PLUGINS y -> n > CONFIG_KEY_DH_OPERATIONS n -> y > CONFIG_PKCS8_PRIVATE_KEY_PARSER n -> y > CONFIG_VIDEO_IMX274 n -> m > > Output from linux-show-fragment: > CONFIG_CRYPTO_DH=y > CONFIG_CRYPTO_KPP=y > CONFIG_KEY_DH_OPERATIONS=y > CONFIG_PKCS8_PRIVATE_KEY_PARSER=y > CONFIG_VIDEO_IMX274=m Notice how "# CONFIG_GCC_PLUGINS is not set" is not generated in that -show-fragment output. But CONFIG_GCC_PLUGINS already has special handling in Buildrot, so maybe that's just an artifact of that... So, it looks like it only reports symbols which values has "increased" (i.e. from 'n' to 'm' or 'y', or from 'm' to 'y'), and completely ignores symbols which values has "decreased" (i.e. from 'y' to 'm' or 'n', or from 'm' to 'n'). This does not look like it is generating a proper fragment, then, because if one reuses that, it's gonna miss some (un)settings. So, even though I do understand the rationale (make it easier to generate a fragment), it is not going to help much if one can't trust it, and has to manually compare the result with what has actually changed, as if -show-fragment did not exist in the first place... Could check with another symbol, to validate that the fragmetn does indeed contain un-setting entries? Still, I am a bit conflicted about that new target. There have been a lot of requests (not recently, though, granted) from people that used multiple fragmetns, who requested a way to automatically update each from the current state of the configuration. This is not possible at all, of course; -show-fragment won't help in that case. -show-fragment can not even help to update an existing fragment; it can only help to *create* a single *additional* fragment. Indeed, -show-fragment only reports the delta from the current configuration, with the expansion of the configuration as expresed from the settings (base + fragment config) in Buildroot's .config. How that delta can be used will be very dependent on the situation: - the delta is re-used as-is to form a new fragment, that stacks on-top on the existing fragment list, in which case this risk acculating some history (with a latter fragment changing a previous one); it may also cause quite some confusiotn, as the user will have to also edit Buildroot's .config to add it to the list of fragments to apply; - the delta is appended as-is to an existing fragment already listed in Buildroot's .config; it is like stacking, above, whith all the cons; it has the advantage that it does not requires changing Buildroot's .config, because the existing fragment is already listed. - the existing fragments (or config file) have to be modified to account for the delta; in this case, -diff-config is probably better suited. So yes, I am a bit unsure... Plus, see below... > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > package/pkg-kconfig.mk | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk > index f4f35bf96a..ee96525c47 100644 > --- a/package/pkg-kconfig.mk > +++ b/package/pkg-kconfig.mk > @@ -309,12 +309,26 @@ $(1)-diff-config: $(1)-check-configuration-done > $$(Q)cp -a $$($(2)_DIR)/.config.dc.bak $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) > $$(Q)rm -f $$($(2)_DIR)/.config.dc.bak > > +# Target to output differences between the configuration obtained via the > +# defconfig + fragments (if any) and the current configuration. > +# Output format is suitable to be used as-is in fragment files. > +# Note: it preserves the timestamp of the current configuration when moving it > +# around. > +$(1)-show-fragment: $(1)-check-configuration-done > + $$(Q)cp -a $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_DIR)/.config.dc.bak > + $$(call kconfig-package-merge-config,$(2),$$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG),\ > + $$($(2)_KCONFIG_FRAGMENT_FILES)) > + $$(Q)utils/diffconfig -m $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) \ > + $$($(2)_DIR)/.config.dc.bak > + $$(Q)cp -a $$($(2)_DIR)/.config.dc.bak $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) > + $$(Q)rm -f $$($(2)_DIR)/.config.dc.bak So, -show-fragment only differs from -diff-config by the -m option passed to utils/diffconfig, right? So, maybe we can simplify things a bit, to really re-use the same code: diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index f4f35bf96a..1ba39f0ee8 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -304,11 +304,13 @@ $(1)-diff-config: $(1)-check-configuration-done $$(Q)cp -a $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_DIR)/.config.dc.bak $$(call kconfig-package-merge-config,$(2),$$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG),\ $$($(2)_KCONFIG_FRAGMENT_FILES)) - $$(Q)utils/diffconfig $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) \ + $$(Q)utils/diffconfig $$(DIFFCONFIG_OPTS) $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) \ $$($(2)_DIR)/.config.dc.bak $$(Q)cp -a $$($(2)_DIR)/.config.dc.bak $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$(Q)rm -f $$($(2)_DIR)/.config.dc.bak +$(1)-show-fragment: DIFFCONFIG_OPTS=-m +$(1)-show-fragment: $(1)-diff-config endif # package enabled Regards, Yann E. MORIN. > endif # package enabled > > .PHONY: \ > $(1)-diff-config \ > $(1)-check-configuration-done \ > + $(1)-show-fragment \ > $$($(2)_DIR)/.kconfig_editor_% \ > $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)) > > -- > 2.39.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index f4f35bf96a..ee96525c47 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -309,12 +309,26 @@ $(1)-diff-config: $(1)-check-configuration-done $$(Q)cp -a $$($(2)_DIR)/.config.dc.bak $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$(Q)rm -f $$($(2)_DIR)/.config.dc.bak +# Target to output differences between the configuration obtained via the +# defconfig + fragments (if any) and the current configuration. +# Output format is suitable to be used as-is in fragment files. +# Note: it preserves the timestamp of the current configuration when moving it +# around. +$(1)-show-fragment: $(1)-check-configuration-done + $$(Q)cp -a $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_DIR)/.config.dc.bak + $$(call kconfig-package-merge-config,$(2),$$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG),\ + $$($(2)_KCONFIG_FRAGMENT_FILES)) + $$(Q)utils/diffconfig -m $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) \ + $$($(2)_DIR)/.config.dc.bak + $$(Q)cp -a $$($(2)_DIR)/.config.dc.bak $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) + $$(Q)rm -f $$($(2)_DIR)/.config.dc.bak endif # package enabled .PHONY: \ $(1)-diff-config \ $(1)-check-configuration-done \ + $(1)-show-fragment \ $$($(2)_DIR)/.kconfig_editor_% \ $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS))
This patch introduce the new target to simplify the generation of configuration fragments as the output could be directly copied into a fragment file as-is. It is *heavily* based on the <pkg>-diff-config target, but serves a different purpose. Output from linux-diff-config: -CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK y -CONFIG_GCC_PLUGIN_CYC_COMPLEXITY n -CONFIG_GCC_PLUGIN_LATENT_ENTROPY n -CONFIG_GCC_PLUGIN_RANDSTRUCT n -CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF n -CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL n -CONFIG_GCC_PLUGIN_STRUCTLEAK_USER n -CONFIG_STACKPROTECTOR_PER_TASK y CONFIG_CRYPTO_DH n -> y CONFIG_CRYPTO_KPP m -> y CONFIG_GCC_PLUGINS y -> n CONFIG_KEY_DH_OPERATIONS n -> y CONFIG_PKCS8_PRIVATE_KEY_PARSER n -> y CONFIG_VIDEO_IMX274 n -> m Output from linux-show-fragment: CONFIG_CRYPTO_DH=y CONFIG_CRYPTO_KPP=y CONFIG_KEY_DH_OPERATIONS=y CONFIG_PKCS8_PRIVATE_KEY_PARSER=y CONFIG_VIDEO_IMX274=m Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- package/pkg-kconfig.mk | 14 ++++++++++++++ 1 file changed, 14 insertions(+)