[Buildroot] [PATCH 0/4] core/pkg-kconfig: fix saving back the configuration (branch yem/pr8156)

[Buildroot] [PATCH 0/4] core/pkg-kconfig: fix saving back the configuration (branch yem/pr8156)

[Yann E. MORIN]

Hello All!

This series is an attempt at fixing the pkg-kconfig infrastructure as
reported in PR8156:
    https://bugs.busybox.net/show_bug.cgi?id=8156

The basic underlying idea of PR8156 is to kick-start the configuration
of a kconfig-based package (say, Linux), using a (def)config bundled in
that package, then tweak that configuration to suit ones needs, and
eventually save it back to a custom (def)config for later re-use.

That is currently broken. It has in fact never worked, but is a
completely valid and useful use-case.

This 4-patch series is an attempt at making that possible.

Patch 1:
First, the initial (def)config file is inexistent since we want to
create it, so make believe it has to re-do all the targets that depend
on that file. Since we really do need that rule, we have to manually
check the files do exist.

Patches 2-3:
Second, because configurators remove the kconfig-fixup-done stamp file,
we'd save the .config file as directly configured by the user, which
might be missing our requirements. So, as soon as we exit the
configurators, we re-run the fixups, so the .config file is always
clean with our fixups.

Patch 4:
Third, the dependency chain for foo-update-config goes way back to that
base kconfig file, so saving the configuration will have make believe it
has to rebuild all targets in that dependency chain. We clearly do not
want that, as the file is really non-exitent and we want to create it.
So, we break the dependency chain and add a terminal rule which purpose
is to check the package is properly configured first.

Pfeww, that one was not trivial... ;-)

Regards,
Yann E. MORIN.


The following changes since commit e1cf34a39538e58ecf50d091482b2304c2f7e6d8:

  gcc/gcc-final: install libatomic via HOST_GCC_FINAL_GCC_LIB_DIR (2015-06-06 03:15:23 +0200)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/pr8156

for you to fetch changes up to a195692f03dab1f3074825feab277c905379dfa2:

  core/pkg-kconfig: allow saving config to a non-existing custom config file (2015-06-06 13:45:06 +0200)

----------------------------------------------------------------
Yann E. MORIN (4):
      core/pkg-kconfig: ensure kconfig file and fragments exist
      core/pkg-kconfig: move the kconfig fixups to a macro
      core/pkg-kconfig: run the kconfig fixups after exiting configurators
      core/pkg-kconfig: allow saving config to a non-existing custom config file

 package/pkg-kconfig.mk | 64 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 7 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 1/4] core/pkg-kconfig: ensure kconfig file and fragments exist

[Yann E. MORIN]

Because the base kconfig file has a dependency but no rule, make will
always try to rebuild targets that depend on it:

    https://www.gnu.org/software/make/manual/make.html#Force-Targets

To complexify things yet a little bit more, missing kconfig fragments
are properly caught, but since they could be bundled in the package,
they should depend on it being extracted. And then we'd have the same
issue as with the base kconfig file, above.

Furthermore, merge-config.sh does not check for the existence of the
fragments, not even the existence of the base file.

So, this patch does more than one single thing, but they are clearly all
pretty much inter-woven each with the others, and thus are done together:

  - make kconfig fragments depend on the package being patched, like the
    base kconfig file,

  - manualy check that the base and fragments do exist.

Ideally, merge-config.sh should be fixed, but it's easier to have
code in our makefiles rather than patch the linux-inherited kconfig,
which we are trying to keep as close as possible to upstream.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Floris Bos <bos@je-eigen-domein.nl>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-kconfig.mk | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index dcaed53..1d69722 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -37,16 +37,26 @@ $(2)_KCONFIG_OPTS ?=
 $(2)_KCONFIG_FIXUP_CMDS ?=
 $(2)_KCONFIG_FRAGMENT_FILES ?=
 
-# The config file could be in-tree, so before depending on it the package should
-# be extracted (and patched) first
-$$($(2)_KCONFIG_FILE): | $(1)-patch
+# The config file as well as the fragments could be in-tree, so before
+# depending on them the package should be extracted (and patched) first
+$$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
 
 # 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.
+#
+# merge-config.sh does not check for the existence of the fragments, not even
+# the existence of the base file, so we do it manually.
+#
 $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
+	for f in $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES); do \
+		if [ ! -f "$$$${f}" ]; then \
+			printf "Kconfig fragment '%s' for '%s' does not exist\n" "$$$${f}" "$(1)"; \
+			exit 1; \
+		fi; \
+	done
 	support/kconfig/merge_config.sh -m -O $$(@D) \
 		$$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
 	@yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
-- 
1.9.1

[Buildroot] [PATCH 2/4] core/pkg-kconfig: move the kconfig fixups to a macro

[Yann E. MORIN]

The same fixups will have to be done after leaving the configurators,
so we want to commonalise that code.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-kconfig.mk | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 1d69722..d6986c3 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -64,11 +64,15 @@ $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
 
 # In order to get a usable, consistent configuration, some fixup may be needed.
 # The exact rules are specified by the package .mk file.
-$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
+define $(1)_FIXUP_KCONFIG
 	$$($(2)_KCONFIG_FIXUP_CMDS)
 	@yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
 		$$($(2)_KCONFIG_OPTS) oldconfig
-	$$(Q)touch $$@
+	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done
+endef
+
+$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
+	$$(call $(1)_FIXUP_KCONFIG)
 
 # Before running configure, the configuration file should be present and fixed
 $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
-- 
1.9.1

[Buildroot] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Yann E. MORIN]

After we exit the configurators, we need to re-run the kconfig fixups to
ensure the user is not able to override them in the configurators.

Currently, we schedule that "for later", by removing the corresponding
stamp file, so make will run the fixups "later".

This means the user has access to the un-fixed .config file, which he
might decide to copy and use as a reference (not too bad, since we'd run
the fixups anyway; but not clean either).

Furthermore, we'll need to have the .config file properly fixed-up
without requiring the full dependency chain up to the original
user-supplied (def)config, so we won't be able to rely on the stamp file
in the dependency list, so we'll need to run the fixups when exiting the
configurators anyway.

Note that we still remove the stamp file before running the fixups, in
case any one of those fixups breaks, so we don't want to believe the
fixups have been applied; the fixup macro will touch that file anyway.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-kconfig.mk | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index d6986c3..6bb2559 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -89,11 +89,24 @@ $$(error Internal error: no value specified for $(2)_KCONFIG_FILE)
 endif
 
 # Configuration editors (menuconfig, ...)
+#
+# Apply the kconfig fixups right after exiting the configurators, so
+# that the user always sees a .config file that is clean wrt. our
+# requirements.
+#
+# Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
+# fake it for the configurators (otherwise it is set to just '.', i.e.
+# the current directory where make is run, which happens to be in
+# $(TOPDIR), because the target of the rule is not an actual file, so
+# does not have any path component).
+#
+$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
 $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
 	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
 		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
 	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
 	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
+	$$(call $(1)_FIXUP_KCONFIG)
 
 $(1)-savedefconfig: $$($(2)_DIR)/.stamp_kconfig_fixup_done
 	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
-- 
1.9.1

[Buildroot] [PATCH 4/4] core/pkg-kconfig: allow saving config to a non-existing custom config file

[Yann E. MORIN]

A very interesting use-case for a kconfig-based package is to create a
custom (def)config file based on one bundled with the package itself,
like described in PR-8156:

    make menuconfig
     -> enable kernel, use an in-tree defconfig, save and exit
    make linux-menuconfig
     -> enable/disable whatever option, save and exit
    make menuconfig
     -> change to use a custom defconfig file, set a path, save and exit
    make linux-update-config
     -> should save to the new custom defconfig file

However, that is currently not possible, because the dependency chain
when saving the configuration goes back up to the (newly-set!) custom
(def)config file, which does not exist.

So, we break the dependency chain so that saving the configuration does
not depend on that file. Instead, we use a terminal rule that checks
that the configuration has indeed been done, and fails if not.

Closes #8156.

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 package/pkg-kconfig.mk | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 6bb2559..453a59d 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -90,9 +90,10 @@ endif
 
 # Configuration editors (menuconfig, ...)
 #
-# Apply the kconfig fixups right after exiting the configurators, so
-# that the user always sees a .config file that is clean wrt. our
-# requirements.
+# We need to apply the configuration fixups right after a configuration
+# editor exits, so that it is possible to save the configuration right
+# after exiting an editor, and so the user always sees a .config file
+# that is clean wrt. our requirements.
 #
 # Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
 # fake it for the configurators (otherwise it is set to just '.', i.e.
@@ -108,14 +109,35 @@ $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_
 	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
 	$$(call $(1)_FIXUP_KCONFIG)
 
-$(1)-savedefconfig: $$($(2)_DIR)/.stamp_kconfig_fixup_done
+# Saving back the configuration
+#
+# Ideally, that should directly depend on $$($(2)_DIR)/.stamp_kconfig_fixup_done,
+# but that breaks the use-case in PR-8156 (from a clean tree):
+#   make menuconfig           <- enable kernel, use an in-tree defconfig, save and exit
+#   make linux-menuconfig     <- enable/disable whatever option, save and exit
+#   make menuconfig           <- change to use a custom defconfig file, set a path, save and exit
+#   make linux-update-config  <- should save to the new custom defconfig file
+#
+# Because of that use-case, saving the configuration can not directly depend
+# on the stamp file, because it itself depends on the .config, which in turn
+# depends on the (newly-set) custom defconfig file.
+#
+# Instead, we use an intermediate rule that will catch that situation.
+#
+$(1)-check-configuration-done:
+	@if [ ! -f $$($(2)_DIR)/.stamp_kconfig_fixup_done ]; then \
+		echo "$(1) is not yet configured"; \
+		exit 1; \
+	fi
+
+$(1)-savedefconfig: $(1)-check-configuration-done
 	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
 		$$($(2)_KCONFIG_OPTS) savedefconfig
 
 # Target to copy back the configuration to the source configuration file
 # Even though we could use 'cp --preserve-timestamps' here, the separate
 # cp and 'touch --reference' is used for symmetry with $(1)-update-defconfig.
-$(1)-update-config: $$($(2)_DIR)/.stamp_kconfig_fixup_done
+$(1)-update-config: $(1)-check-configuration-done
 	@$$(if $$($(2)_KCONFIG_FRAGMENT_FILES), \
 		echo "Unable to perform $(1)-update-config when fragment files are set"; exit 1)
 	cp -f $$($(2)_DIR)/.config $$($(2)_KCONFIG_FILE)
@@ -137,6 +159,7 @@ endif # package enabled
 	$(1)-update-config \
 	$(1)-update-defconfig \
 	$(1)-savedefconfig \
+	$(1)-check-configuration-done \
 	$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS))
 
 endef # inner-kconfig-package
-- 
1.9.1

[Buildroot] [PATCH 1/4] core/pkg-kconfig: ensure kconfig file and fragments exist

[Thomas Petazzoni]

Yann,

On Sat,  6 Jun 2015 13:54:23 +0200, Yann E. MORIN wrote:
> Because the base kconfig file has a dependency but no rule, make will
> always try to rebuild targets that depend on it:
> 
>     https://www.gnu.org/software/make/manual/make.html#Force-Targets
> 
> To complexify things yet a little bit more, missing kconfig fragments
> are properly caught,

What do you mean by "are properly caught" ? Also, I don't see the
interaction between this and the previous paragraph, and hence the "To
complexity things yet a little bit more"

> but since they could be bundled in the package,
> they should depend on it being extracted. And then we'd have the same
> issue as with the base kconfig file, above.

"then" what ?

> Furthermore, merge-config.sh does not check for the existence of the
> fragments, not even the existence of the base file.

How is this related to the previous paragraph ?

> So, this patch does more than one single thing, but they are clearly all
> pretty much inter-woven each with the others, and thus are done together:
> 
>   - make kconfig fragments depend on the package being patched, like the
>     base kconfig file,
> 
>   - manualy check that the base and fragments do exist.

manually

> -# The config file could be in-tree, so before depending on it the package should
> -# be extracted (and patched) first
> -$$($(2)_KCONFIG_FILE): | $(1)-patch
> +# The config file as well as the fragments could be in-tree, so before
> +# depending on them the package should be extracted (and patched) first
> +$$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch

This part makes complete sense.

>  # 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.
> +#
> +# merge-config.sh does not check for the existence of the fragments, not even
> +# the existence of the base file, so we do it manually.
> +#
>  $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
> +	for f in $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES); do \
> +		if [ ! -f "$$$${f}" ]; then \
> +			printf "Kconfig fragment '%s' for '%s' does not exist\n" "$$$${f}" "$(1)"; \
> +			exit 1; \
> +		fi; \
> +	done

And this one as well.

So basically, I understand the code, but absolutely not the commit
log :-)

And also, I don't understand why those two things are so much
"inter-woven" that they cannot be done in two separate patches. It's
actually two very separate things: 1/ support kconfig fragments bundled
in the package source code (chunk 1) and 2/ check that they exist
(chunk 2).

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/4] core/pkg-kconfig: move the kconfig fixups to a macro

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sat,  6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote:

>  # In order to get a usable, consistent configuration, some fixup may be needed.
>  # The exact rules are specified by the package .mk file.
> -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
> +define $(1)_FIXUP_KCONFIG
>  	$$($(2)_KCONFIG_FIXUP_CMDS)
>  	@yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \

We could use $(@D) instead of $$($(2)_DIR) here.

>  		$$($(2)_KCONFIG_OPTS) oldconfig
> -	$$(Q)touch $$@
> +	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done

and here. But that's indeed not very important, since they are the same.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sat,  6 Jun 2015 13:54:25 +0200, Yann E. MORIN wrote:
> After we exit the configurators, we need to re-run the kconfig fixups to
> ensure the user is not able to override them in the configurators.
> 
> Currently, we schedule that "for later", by removing the corresponding
> stamp file, so make will run the fixups "later".
> 
> This means the user has access to the un-fixed .config file, which he
> might decide to copy and use as a reference (not too bad, since we'd run
> the fixups anyway; but not clean either).

Until now, OK.

> Furthermore, we'll need to have the .config file properly fixed-up
> without requiring the full dependency chain up to the original
> user-supplied (def)config, so we won't be able to rely on the stamp file
> in the dependency list, so we'll need to run the fixups when exiting the
> configurators anyway.

But here, I'm lost.

>  # Configuration editors (menuconfig, ...)
> +#
> +# Apply the kconfig fixups right after exiting the configurators, so
> +# that the user always sees a .config file that is clean wrt. our
> +# requirements.
> +#
> +# Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
> +# fake it for the configurators (otherwise it is set to just '.', i.e.
> +# the current directory where make is run, which happens to be in
> +# $(TOPDIR), because the target of the rule is not an actual file, so
> +# does not have any path component).
> +#
> +$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
>  $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
>  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
>  		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
>  	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
>  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
> +	$$(call $(1)_FIXUP_KCONFIG)

I find a bit sad that we don't manage to use make dependencies to
schedule the kconfig-fixup -> configurator -> kconfig-fixup thing
logic, and that we have to manually recall $(1)_FIXUP_CONFIG. I've put
a bit of thought into it, but I indeed don't see a simple way of
achieving that in a different way that the one you're proposing. The
hack on @D is particularly tricky.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] core/pkg-kconfig: allow saving config to a non-existing custom config file

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sat,  6 Jun 2015 13:54:26 +0200, Yann E. MORIN wrote:

> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 6bb2559..453a59d 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -90,9 +90,10 @@ endif
>  
>  # Configuration editors (menuconfig, ...)
>  #
> -# Apply the kconfig fixups right after exiting the configurators, so
> -# that the user always sees a .config file that is clean wrt. our
> -# requirements.
> +# We need to apply the configuration fixups right after a configuration
> +# editor exits, so that it is possible to save the configuration right
> +# after exiting an editor, and so the user always sees a .config file
> +# that is clean wrt. our requirements.

Shouldn't this chunk be part of the previous patch?

>  #
>  # Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
>  # fake it for the configurators (otherwise it is set to just '.', i.e.
> @@ -108,14 +109,35 @@ $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_
>  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
>  	$$(call $(1)_FIXUP_KCONFIG)
>  
> -$(1)-savedefconfig: $$($(2)_DIR)/.stamp_kconfig_fixup_done
> +# Saving back the configuration
> +#
> +# Ideally, that should directly depend on $$($(2)_DIR)/.stamp_kconfig_fixup_done,
> +# but that breaks the use-case in PR-8156 (from a clean tree):
> +#   make menuconfig           <- enable kernel, use an in-tree defconfig, save and exit
> +#   make linux-menuconfig     <- enable/disable whatever option, save and exit
> +#   make menuconfig           <- change to use a custom defconfig file, set a path, save and exit
> +#   make linux-update-config  <- should save to the new custom defconfig file
> +#
> +# Because of that use-case, saving the configuration can not directly depend

can not -> cannot

Other than that, looks good to me.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 0/4] core/pkg-kconfig: fix saving back the configuration (branch yem/pr8156)

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sat,  6 Jun 2015 13:54:22 +0200, Yann E. MORIN wrote:

> Yann E. MORIN (4):
>       core/pkg-kconfig: ensure kconfig file and fragments exist
>       core/pkg-kconfig: move the kconfig fixups to a macro
>       core/pkg-kconfig: run the kconfig fixups after exiting configurators
>       core/pkg-kconfig: allow saving config to a non-existing custom config file

I've tested the series, and I confirm it fixes the problem for me.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/4] core/pkg-kconfig: ensure kconfig file and fragments exist

[Yann E. MORIN]

Thomas, All,

On 2015-06-12 23:24 +0200, Thomas Petazzoni spake thusly:
> On Sat,  6 Jun 2015 13:54:23 +0200, Yann E. MORIN wrote:
> > Because the base kconfig file has a dependency but no rule, make will
> > always try to rebuild targets that depend on it:
> > 
> >     https://www.gnu.org/software/make/manual/make.html#Force-Targets
> > 
> > To complexify things yet a little bit more, missing kconfig fragments
> > are properly caught,
> 
> What do you mean by "are properly caught" ?

Just that missing fragments cause a build failure, becasue of that rule:

    $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)

Contrary to $$($(2)_KCONFIG_FILE), which has a rule to generate it,
there is no such rule for the fragments. So make itself whines there is
a missing file for which it has no rule to generate it.

> Also, I don't see the
> interaction between this and the previous paragraph, and hence the "To
> complexity things yet a little bit more"

Well, complexity comes from the fact that a missing base config file or
a missing fragment do not cause the same behaviour:

  - a missing fragment cause a build error that make reports as a missing
    file (see above)

  - a missing base file causes no build failure, because merge-config.sh
    does not check its inputs (and thus produces a garbled .config).

> > but since they could be bundled in the package,
> > they should depend on it being extracted. And then we'd have the same
> > issue as with the base kconfig file, above.
> 
> "then" what ?

Then make would not complain, and feed them to merge-config, wich would
not care at all (but produce a garbled .config).

> > Furthermore, merge-config.sh does not check for the existence of the
> > fragments, not even the existence of the base file.
> 
> How is this related to the previous paragraph ?

Well, it is related because a missing file is not an error for merge-config
and we'd get a broken .config.

But Ok, that is far from being clear, so I'll rework and split the
commit in two, as you explained below.

> >   - manualy check that the base and fragments do exist.
> manually

OK.

> > -# The config file could be in-tree, so before depending on it the package should
> > -# be extracted (and patched) first
> > -$$($(2)_KCONFIG_FILE): | $(1)-patch
> > +# The config file as well as the fragments could be in-tree, so before
> > +# depending on them the package should be extracted (and patched) first
> > +$$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
> 
> This part makes complete sense.

Ah! :-)

> >  # 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.
> > +#
> > +# merge-config.sh does not check for the existence of the fragments, not even
> > +# the existence of the base file, so we do it manually.
> > +#
> >  $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
> > +	for f in $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES); do \
> > +		if [ ! -f "$$$${f}" ]; then \
> > +			printf "Kconfig fragment '%s' for '%s' does not exist\n" "$$$${f}" "$(1)"; \
> > +			exit 1; \
> > +		fi; \
> > +	done
> 
> And this one as well.

Yeah!

> So basically, I understand the code, but absolutely not the commit
> log :-)

OK. It means the problem is badly explained. After the patch is plit, I
hope it will be easier to explain each part independently.

However, by further splitting those, I'm afraid we'd loose the big
picture. I'll try to catch that in the cover letter.

> And also, I don't understand why those two things are so much
> "inter-woven" that they cannot be done in two separate patches. It's
> actually two very separate things: 1/ support kconfig fragments bundled
> in the package source code (chunk 1) and 2/ check that they exist
> (chunk 2).

Right, will do.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 2/4] core/pkg-kconfig: move the kconfig fixups to a macro

[Yann E. MORIN]

Thomas, All,

On 2015-06-12 23:33 +0200, Thomas Petazzoni spake thusly:
> On Sat,  6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote:
> >  # In order to get a usable, consistent configuration, some fixup may be needed.
> >  # The exact rules are specified by the package .mk file.
> > -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
> > +define $(1)_FIXUP_KCONFIG
> >  	$$($(2)_KCONFIG_FIXUP_CMDS)
> >  	@yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> 
> We could use $(@D) instead of $$($(2)_DIR) here.

Yes, and Arnout suggested the same in another series I sent.

My reasoning for using $($(2)_DIR) is that we are in the infra, and we
do have this variable, while $$(@D) only works when there is an actual
file being generated, which is not always the case.

See for example the followup patch, for which we have to fake @D because
of those package-provided macros that may need it.

So, I prefer using $($(2)_DIR) because I /know/ what it is, while $(@D)
I don;t know beforehand if it would be set.

> >  		$$($(2)_KCONFIG_OPTS) oldconfig
> > -	$$(Q)touch $$@
> > +	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done
> 
> and here. But that's indeed not very important, since they are the same.

Yes, it works with this patch because there is an actual file (and in
the correct location) as the target of the current rule.

It wiull also work with the next patch, because we are faking @D .

So, I'd prefer we stick with $($(2)_DIR) here, to avoid any issue in the
future.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Yann E. MORIN]

Thomas, All,

On 2015-06-12 23:36 +0200, Thomas Petazzoni spake thusly:
> On Sat,  6 Jun 2015 13:54:25 +0200, Yann E. MORIN wrote:
> > After we exit the configurators, we need to re-run the kconfig fixups to
> > ensure the user is not able to override them in the configurators.
> > 
> > Currently, we schedule that "for later", by removing the corresponding
> > stamp file, so make will run the fixups "later".
> > 
> > This means the user has access to the un-fixed .config file, which he
> > might decide to copy and use as a reference (not too bad, since we'd run
> > the fixups anyway; but not clean either).
> 
> Until now, OK.
> 
> > Furthermore, we'll need to have the .config file properly fixed-up
> > without requiring the full dependency chain up to the original
> > user-supplied (def)config, so we won't be able to rely on the stamp file
> > in the dependency list, so we'll need to run the fixups when exiting the
> > configurators anyway.
> 
> But here, I'm lost.

Right, this has nothing to do in that commit log.

> >  # Configuration editors (menuconfig, ...)
> > +#
> > +# Apply the kconfig fixups right after exiting the configurators, so
> > +# that the user always sees a .config file that is clean wrt. our
> > +# requirements.
> > +#
> > +# Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
> > +# fake it for the configurators (otherwise it is set to just '.', i.e.
> > +# the current directory where make is run, which happens to be in
> > +# $(TOPDIR), because the target of the rule is not an actual file, so
> > +# does not have any path component).
> > +#
> > +$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
> >  $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
> >  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> >  		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
> >  	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
> >  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
> > +	$$(call $(1)_FIXUP_KCONFIG)
> 
> I find a bit sad that we don't manage to use make dependencies to
> schedule the kconfig-fixup -> configurator -> kconfig-fixup thing
> logic, and that we have to manually recall $(1)_FIXUP_CONFIG.

And I just noticed that it should be named $(2)_FIXUP_CONFIG, not
$(1)_FIXUP_CONFIG. Will fix before I respin.

> I've put
> a bit of thought into it, but I indeed don't see a simple way of
> achieving that in a different way that the one you're proposing.

Well, I had it working at one point with tricky makefile dependencies,
that I did not fully understand myself, so I prefered to ditch them and
use simpler code that is easily understandable.

> The
> hack on @D is particularly tricky.

s/tricky/ugly/  ;-)

Yes you're right, hence the reason I put a big comment about it.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 4/4] core/pkg-kconfig: allow saving config to a non-existing custom config file

[Yann E. MORIN]

Thomas, All,

On 2015-06-12 23:39 +0200, Thomas Petazzoni spake thusly:
> On Sat,  6 Jun 2015 13:54:26 +0200, Yann E. MORIN wrote:
> 
> > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> > index 6bb2559..453a59d 100644
> > --- a/package/pkg-kconfig.mk
> > +++ b/package/pkg-kconfig.mk
> > @@ -90,9 +90,10 @@ endif
> >  
> >  # Configuration editors (menuconfig, ...)
> >  #
> > -# Apply the kconfig fixups right after exiting the configurators, so
> > -# that the user always sees a .config file that is clean wrt. our
> > -# requirements.
> > +# We need to apply the configuration fixups right after a configuration
> > +# editor exits, so that it is possible to save the configuration right
> > +# after exiting an editor, and so the user always sees a .config file
> > +# that is clean wrt. our requirements.
> 
> Shouldn't this chunk be part of the previous patch?

Well, this chunk is _updating_ the comment introduced in the previous
patch.

In the previous patch, we were not yet able to save back the
configuration to a non-existing, so I did not talk about that in the
previous patch.

With this new patch, we are now able to save the configuration back to
a non-existing file, so I ammend the comment to take this new
possibility into account.

> >  # Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
> >  # fake it for the configurators (otherwise it is set to just '.', i.e.
> > @@ -108,14 +109,35 @@ $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_
> >  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
> >  	$$(call $(1)_FIXUP_KCONFIG)
> >  
> > -$(1)-savedefconfig: $$($(2)_DIR)/.stamp_kconfig_fixup_done
> > +# Saving back the configuration
> > +#
> > +# Ideally, that should directly depend on $$($(2)_DIR)/.stamp_kconfig_fixup_done,
> > +# but that breaks the use-case in PR-8156 (from a clean tree):
> > +#   make menuconfig           <- enable kernel, use an in-tree defconfig, save and exit
> > +#   make linux-menuconfig     <- enable/disable whatever option, save and exit
> > +#   make menuconfig           <- change to use a custom defconfig file, set a path, save and exit
> > +#   make linux-update-config  <- should save to the new custom defconfig file
> > +#
> > +# Because of that use-case, saving the configuration can not directly depend
> 
> can not -> cannot

Well, the Oxford dictionary believes both are acceptable (but cannot is
much more usual, granted):
    https://www.oxforddictionaries.com/words/cannot-or-can-not
    https://www.oxforddictionaries.com/definition/english/cannot

Also, the "can not" construct is more acceptable when one wants to
emphasize the negative part, which is exactly what I wwanted to convey
here. Think of it like if I said: "it can *not* depend on..."
That's the position of the Washington State University language site:
    http://public.wsu.edu/~brians/errors/cannot.html

But Oh well... ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 0/4] core/pkg-kconfig: fix saving back the configuration (branch yem/pr8156)

[Yann E. MORIN]

Thomas, All,

On 2015-06-12 23:46 +0200, Thomas Petazzoni spake thusly:
> On Sat,  6 Jun 2015 13:54:22 +0200, Yann E. MORIN wrote:
> > Yann E. MORIN (4):
> >       core/pkg-kconfig: ensure kconfig file and fragments exist
> >       core/pkg-kconfig: move the kconfig fixups to a macro
> >       core/pkg-kconfig: run the kconfig fixups after exiting configurators
> >       core/pkg-kconfig: allow saving config to a non-existing custom config file
> 
> I've tested the series, and I confirm it fixes the problem for me.

Great!

I'll apply your comment later this WE and will respin then,

Thanks for the review! :-)


-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Arnout Vandecappelle]

On 06/12/15 23:36, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
> 
> On Sat,  6 Jun 2015 13:54:25 +0200, Yann E. MORIN wrote:

[snip]

>> +# Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
>> +# fake it for the configurators (otherwise it is set to just '.', i.e.
>> +# the current directory where make is run, which happens to be in
>> +# $(TOPDIR), because the target of the rule is not an actual file, so
>> +# does not have any path component).
>> +#
>> +$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
>>  $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
>>  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
>>  		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
>>  	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
>>  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
>> +	$$(call $(1)_FIXUP_KCONFIG)
> 
> I find a bit sad that we don't manage to use make dependencies to
> schedule the kconfig-fixup -> configurator -> kconfig-fixup thing
> logic, and that we have to manually recall $(1)_FIXUP_CONFIG. I've put
> a bit of thought into it, but I indeed don't see a simple way of
> achieving that in a different way that the one you're proposing. The
> hack on @D is particularly tricky.

How about:

$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $(1)-%: $$($(2)_DIR)/%

$$($(2)_DIR)/%: $$($(2)_DIR)/.stamp_kconfig_fixup_done
        $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
                $$($(2)_KCONFIG_OPTS) $$*
	...

Note that there's no need to actually create $$($(2)_DIR)/%, so it should
probably be declared PHONY too:

.PHONY: $$(addprefix $$($(2)_DIR)/,$$($(2)_KCONFIG_EDITORS))


 Of course, you will still need to call FIXUP_CONFIG twice, but really in make
there is no way to do things twice except by repeating them - make tries very
hard to do things only once.

 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

[Buildroot] [PATCH 2/4] core/pkg-kconfig: move the kconfig fixups to a macro

[Arnout Vandecappelle]

On 06/13/15 00:17, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2015-06-12 23:33 +0200, Thomas Petazzoni spake thusly:
>> On Sat,  6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote:
>>>  # In order to get a usable, consistent configuration, some fixup may be needed.
>>>  # The exact rules are specified by the package .mk file.
>>> -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
>>> +define $(1)_FIXUP_KCONFIG
>>>  	$$($(2)_KCONFIG_FIXUP_CMDS)
>>>  	@yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
>>
>> We could use $(@D) instead of $$($(2)_DIR) here.
> 
> Yes, and Arnout suggested the same in another series I sent.
> 
> My reasoning for using $($(2)_DIR) is that we are in the infra, and we
> do have this variable, while $$(@D) only works when there is an actual
> file being generated, which is not always the case.
> 
> See for example the followup patch, for which we have to fake @D because
> of those package-provided macros that may need it.
> 
> So, I prefer using $($(2)_DIR) because I /know/ what it is, while $(@D)
> I don;t know beforehand if it would be set.

 I was going to stick to my guns and say it has to be @D (cfr. the workaround I
proposed on the following patch). However, what you say makes complete sense, so
let's keep it at $(2)_DIR. It's also more readable anyway.

 Regards,
 Arnout

[snip]


-- 
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

[Buildroot] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Yann E. MORIN]

Arnout, All,

On 2015-06-13 18:25 +0200, Arnout Vandecappelle spake thusly:
> On 06/12/15 23:36, Thomas Petazzoni wrote:
> > On Sat,  6 Jun 2015 13:54:25 +0200, Yann E. MORIN wrote:
[--SNIP--]
> >> +$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
[--SNIP--]
> > The
> > hack on @D is particularly tricky.
> 
> How about:
> 
> $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $(1)-%: $$($(2)_DIR)/%
> 
> $$($(2)_DIR)/%: $$($(2)_DIR)/.stamp_kconfig_fixup_done
>         $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
>                 $$($(2)_KCONFIG_OPTS) $$*
> 	...

Yup, that would work. I was tempted to do so, but this is starting to
get pretty advanced Makefile syntax.

I'm not too comfortable with that syntax. Although I fully understand
what it means, I had to double-check the manual to be sure.

I'd have a slight preference for twiddling with @D...

> Note that there's no need to actually create $$($(2)_DIR)/%, so it should
> probably be declared PHONY too:
> 
> .PHONY: $$(addprefix $$($(2)_DIR)/,$$($(2)_KCONFIG_EDITORS))

Yes, it would need to be PHONY.

>  Of course, you will still need to call FIXUP_CONFIG twice, but really in make
> there is no way to do things twice except by repeating them - make tries very
> hard to do things only once.

Yup...

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 3/4] core/pkg-kconfig: run the kconfig fixups after exiting configurators

[Yann E. MORIN]

Arnout, All,

On 2015-06-13 18:25 +0200, Arnout Vandecappelle spake thusly:
> On 06/12/15 23:36, Thomas Petazzoni wrote:
> > On Sat,  6 Jun 2015 13:54:25 +0200, Yann E. MORIN wrote:
> >> +# Because commands in $(1)_FIXUP_KCONFIG are probably using $(@D), we
> >> +# fake it for the configurators (otherwise it is set to just '.', i.e.
> >> +# the current directory where make is run, which happens to be in
> >> +# $(TOPDIR), because the target of the rule is not an actual file, so
> >> +# does not have any path component).
> >> +#
> >> +$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): @D=$$($(2)_DIR)
> >>  $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
> >>  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> >>  		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
> >>  	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
> >>  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
> >> +	$$(call $(1)_FIXUP_KCONFIG)
[--SNIP--]
> How about:
> 
> $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $(1)-%: $$($(2)_DIR)/%
> 
> $$($(2)_DIR)/%: $$($(2)_DIR)/.stamp_kconfig_fixup_done
>         $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
>                 $$($(2)_KCONFIG_OPTS) $$*
> 	...
> 
> Note that there's no need to actually create $$($(2)_DIR)/%, so it should
> probably be declared PHONY too:
> 
> .PHONY: $$(addprefix $$($(2)_DIR)/,$$($(2)_KCONFIG_EDITORS))

In the end, I stumbled upon a hard-to-debug issue caused by overriding
@D, so I'll switch the code to using your proposal, since that would fix
my issue (mor eon that one later! ;-) ), the alternate solution being
even uglier...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'