[U-Boot] [PATCH 1/2] Makefile: use if_change_dep for u-boot.cfg

[U-Boot] [PATCH 1/2] Makefile: use if_change_dep for u-boot.cfg

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

cmd_cpp_cfg generates a dependency output, but because it's invoked using
if_changed rather than if_changed_dep, that dependency file is ignored.
This results in Kbuild not knowing about which files u-boot.cfg depends
on, so it may not be rebuilt when required.

A practical result of this is that u-boot.cfg may continue to reference
CONFIG_ options that no longer exist in the source tree, and this can
cause the adhoc config options check to fail.

This change modifies Makefile to use if_changed_dep, which in turn causes
all dependencies to be known to the next make invocation.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 Makefile             | 2 +-
 scripts/Makefile.spl | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index fffc188f9ac0..949b264b8fee 100644
--- a/Makefile
+++ b/Makefile
@@ -937,7 +937,7 @@ u-boot.dis:	u-boot
 		$(OBJDUMP) -d $< > $@
 
 u-boot.cfg:	include/config.h FORCE
-	$(call if_changed,cpp_cfg)
+	$(call if_changed_dep,cpp_cfg)
 
 # Check that this build does not use CONFIG options that we don't know about
 # unless they are in Kconfig. All the existing CONFIG options are whitelisted,
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 4994fa887ba3..d0d73d3b0a4c 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -217,7 +217,7 @@ cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 	-DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $<
 
 $(obj)/$(SPL_BIN).cfg:	include/config.h FORCE
-	$(call if_changed,cpp_cfg)
+	$(call if_changed_dep,cpp_cfg)
 
 pythonpath = PYTHONPATH=tools
 
-- 
2.9.3

[U-Boot] [PATCH 2/2] Makefile: rm u-boot.cfg dependencies are missing

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Prior to the previous patch, a freshly created .u-boot.cfg.cmd may not
correctly represent all dependencies for u-boot.cfg. The previous change
only solved this issue for fresh builds; when performing an incremental
build, the deficient .u-boot.cfg.cmd is already present, so u-boot.cfg
is not rebuilt, and hence .u-boot.cfg.cmd is not rebuilt with the correct
content.

Solve this by explicitly detecting when the dependency file .u-boot.cfg.d
has not been integrated into .u-boot.cfg.cmd, and force u-boot.cfg to be
rebuilt in this case by deleting it first. This is possible since
if_changed_dep will always delete .u-boot.cfg.d when it executes
successfully, so its presence means either that the previous build was
made by a source tree that contained a Makefile that didn't include the
previous patch, or that the build failed part way through executing
if_changed_dep for u-boot.cfg. Forcing a rebuild of u-boot.cfg is required
in the former case, and will cause no additional work in the latter case,
since the file would be rebuilt anyway for the same reason it was being
rebuilt by the previous build.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 Makefile             | 11 +++++++++++
 scripts/Makefile.spl | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/Makefile b/Makefile
index 949b264b8fee..c30f90af8cff 100644
--- a/Makefile
+++ b/Makefile
@@ -936,6 +936,17 @@ u-boot.sha1:	u-boot.bin
 u-boot.dis:	u-boot
 		$(OBJDUMP) -d $< > $@
 
+# If .u-boot.cfg.d is still present, then either:
+# a) The previous build used a Makefile that used if_changed rather than
+#    if_changed_dep when building u-boot.cfg, and hence any later builds will
+#    be unaware of the dependencies for u-boot.cfg. In this case, we must
+#    delete u-boot.cfg to force it and .u-boot.cfg.cmd to be rebuilt the
+#    correct way.
+# b) The previous build failed or was interrupted while building u-boot.cfg,
+#    so deleting u-boot.cfg isn't going to cause any additional work.
+ifneq ($(wildcard $(obj)/.u-boot.cfg.d),)
+  unused := $(shell rm -f $(obj)/u-boot.cfg)
+endif
 u-boot.cfg:	include/config.h FORCE
 	$(call if_changed_dep,cpp_cfg)
 
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index d0d73d3b0a4c..5a7f79c25a76 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -216,6 +216,17 @@ quiet_cmd_cpp_cfg = CFG     $@
 cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 	-DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $<
 
+# If .u-boot.cfg.d is still present, then either:
+# a) The previous build used a Makefile that used if_changed rather than
+#    if_changed_dep when building u-boot.cfg, and hence any later builds will
+#    be unaware of the dependencies for u-boot.cfg. In this case, we must
+#    delete u-boot.cfg to force it and .u-boot.cfg.cmd to be rebuilt the
+#    correct way.
+# b) The previous build failed or was interrupted while building u-boot.cfg,
+#    so deleting u-boot.cfg isn't going to cause any additional work.
+ifneq ($(wildcard $(obj)/.$(SPL_BIN).d),)
+  unused := $(shell rm -f $(obj)/$(SPL_BIN).cfg)
+endif
 $(obj)/$(SPL_BIN).cfg:	include/config.h FORCE
 	$(call if_changed_dep,cpp_cfg)
 
-- 
2.9.3

[U-Boot] [PATCH 1/2] Makefile: use if_change_dep for u-boot.cfg

[Simon Glass]

On 19 September 2016 at 12:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> cmd_cpp_cfg generates a dependency output, but because it's invoked using
> if_changed rather than if_changed_dep, that dependency file is ignored.
> This results in Kbuild not knowing about which files u-boot.cfg depends
> on, so it may not be rebuilt when required.
>
> A practical result of this is that u-boot.cfg may continue to reference
> CONFIG_ options that no longer exist in the source tree, and this can
> cause the adhoc config options check to fail.
>
> This change modifies Makefile to use if_changed_dep, which in turn causes
> all dependencies to be known to the next make invocation.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  Makefile             | 2 +-
>  scripts/Makefile.spl | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/2] Makefile: rm u-boot.cfg dependencies are missing

[Simon Glass]

On 19 September 2016 at 12:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> Prior to the previous patch, a freshly created .u-boot.cfg.cmd may not
> correctly represent all dependencies for u-boot.cfg. The previous change
> only solved this issue for fresh builds; when performing an incremental
> build, the deficient .u-boot.cfg.cmd is already present, so u-boot.cfg
> is not rebuilt, and hence .u-boot.cfg.cmd is not rebuilt with the correct
> content.
>
> Solve this by explicitly detecting when the dependency file .u-boot.cfg.d
> has not been integrated into .u-boot.cfg.cmd, and force u-boot.cfg to be
> rebuilt in this case by deleting it first. This is possible since
> if_changed_dep will always delete .u-boot.cfg.d when it executes
> successfully, so its presence means either that the previous build was
> made by a source tree that contained a Makefile that didn't include the
> previous patch, or that the build failed part way through executing
> if_changed_dep for u-boot.cfg. Forcing a rebuild of u-boot.cfg is required
> in the former case, and will cause no additional work in the latter case,
> since the file would be rebuilt anyway for the same reason it was being
> rebuilt by the previous build.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  Makefile             | 11 +++++++++++
>  scripts/Makefile.spl | 11 +++++++++++
>  2 files changed, 22 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 1/2] Makefile: use if_change_dep for u-boot.cfg

[Masahiro Yamada]

2016-09-23 13:16 GMT+09:00 Simon Glass <sjg@chromium.org>:
> On 19 September 2016 at 12:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> cmd_cpp_cfg generates a dependency output, but because it's invoked using
>> if_changed rather than if_changed_dep, that dependency file is ignored.
>> This results in Kbuild not knowing about which files u-boot.cfg depends
>> on, so it may not be rebuilt when required.
>>
>> A practical result of this is that u-boot.cfg may continue to reference
>> CONFIG_ options that no longer exist in the source tree, and this can
>> cause the adhoc config options check to fail.
>>
>> This change modifies Makefile to use if_changed_dep, which in turn causes
>> all dependencies to be known to the next make invocation.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>


In the first place, why do we need u-boot.cfg?

CONFIG options from board headers are already collected in include/autoconf.mk.

I think the most correct way is to rework check-config.sh
to parse include/autoconf.mk (I can do this).

Then, remove all of the u-boot.cfg rules.



-- 
Best Regards
Masahiro Yamada

[U-Boot] [U-Boot, 1/2] Makefile: use if_change_dep for u-boot.cfg

[Tom Rini]

On Mon, Sep 19, 2016 at 12:20:25PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> cmd_cpp_cfg generates a dependency output, but because it's invoked using
> if_changed rather than if_changed_dep, that dependency file is ignored.
> This results in Kbuild not knowing about which files u-boot.cfg depends
> on, so it may not be rebuilt when required.
> 
> A practical result of this is that u-boot.cfg may continue to reference
> CONFIG_ options that no longer exist in the source tree, and this can
> cause the adhoc config options check to fail.
> 
> This change modifies Makefile to use if_changed_dep, which in turn causes
> all dependencies to be known to the next make invocation.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

A few days ago now, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/6e183634/attachment.sig>

[U-Boot] [U-Boot, 2/2] Makefile: rm u-boot.cfg dependencies are missing

[Tom Rini]

On Mon, Sep 19, 2016 at 12:20:26PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Prior to the previous patch, a freshly created .u-boot.cfg.cmd may not
> correctly represent all dependencies for u-boot.cfg. The previous change
> only solved this issue for fresh builds; when performing an incremental
> build, the deficient .u-boot.cfg.cmd is already present, so u-boot.cfg
> is not rebuilt, and hence .u-boot.cfg.cmd is not rebuilt with the correct
> content.
> 
> Solve this by explicitly detecting when the dependency file .u-boot.cfg.d
> has not been integrated into .u-boot.cfg.cmd, and force u-boot.cfg to be
> rebuilt in this case by deleting it first. This is possible since
> if_changed_dep will always delete .u-boot.cfg.d when it executes
> successfully, so its presence means either that the previous build was
> made by a source tree that contained a Makefile that didn't include the
> previous patch, or that the build failed part way through executing
> if_changed_dep for u-boot.cfg. Forcing a rebuild of u-boot.cfg is required
> in the former case, and will cause no additional work in the latter case,
> since the file would be rebuilt anyway for the same reason it was being
> rebuilt by the previous build.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

A few days ago now, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/470ab24a/attachment.sig>

[U-Boot] [PATCH 1/2] Makefile: use if_change_dep for u-boot.cfg

[Simon Glass]

Hi Masahiro,

On 23 September 2016 at 03:32, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2016-09-23 13:16 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> On 19 September 2016 at 12:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> cmd_cpp_cfg generates a dependency output, but because it's invoked using
>>> if_changed rather than if_changed_dep, that dependency file is ignored.
>>> This results in Kbuild not knowing about which files u-boot.cfg depends
>>> on, so it may not be rebuilt when required.
>>>
>>> A practical result of this is that u-boot.cfg may continue to reference
>>> CONFIG_ options that no longer exist in the source tree, and this can
>>> cause the adhoc config options check to fail.
>>>
>>> This change modifies Makefile to use if_changed_dep, which in turn causes
>>> all dependencies to be known to the next make invocation.
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
>
> In the first place, why do we need u-boot.cfg?
>
> CONFIG options from board headers are already collected in include/autoconf.mk.
>
> I think the most correct way is to rework check-config.sh
> to parse include/autoconf.mk (I can do this).
>
> Then, remove all of the u-boot.cfg rules.

This file is intended to allow buildman to see all the CONFIG options
that are set, whether in #defines or in Kconfig.

Regards,
Simon