* Re: [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
@ 2017-12-22 22:44 Guenter Roeck
0 siblings, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2017-12-22 22:44 UTC (permalink / raw)
To: Douglas Anderson
Cc: Masahiro Yamada, malat, dave.hansen, yang.s, Matthias Kaehlcke,
Cao jin, Arnd Bergmann, Marcin Nowakowski, Mark Charlebois,
linux-kernel, Ingo Molnar, Nick Desaulniers
On Fri, Dec 22, 2017 at 02:14:53PM -0800, Douglas Anderson wrote:
> Several people reported that the commit 3298b690b21c ("kbuild: Add a
> cache for generated variables") caused them problems when they updated
> gcc versions. Specifically the reports all looked something similar
> to this:
>
> > In file included from ./include/uapi/linux/uuid.h:21:0,
> > from ./include/linux/uuid.h:19,
> > from ./include/linux/mod_devicetable.h:12,
> > from scripts/mod/devicetable-offsets.c:2:
> > ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
> > directory
> > #include <stdarg.h>
>
> Masahiro Yamada determined that the problem was with:
>
> NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
> -print-file-name=include)
>
> Specifically that the stale result of -print-file-name is stored in
> the cache file. It was determined that a "make clean" fixed the
> problems in all cases.
>
> In this particular case we could certainly try to clean just the cache
> when we detect a gcc update, but it seems like overall it's a bad idea
> to do an incremental build when gcc changes. We should warn the user
> and tell them that they need a 'make clean'.
>
> Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
> Reported-by: Yang Shi <yang.s@alibaba-inc.com>
> Reported-by: Dave Hansen <dave.hansen@intel.com>
> Reported-by: Mathieu Malaterre <malat@debian.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
No idea if it is correct, but this version works for me.
Tested-by: Guenter Roeck <linux@roeck-us.net>
> ---
>
> Changes in v2:
> - Don't error if MAKECMDGOALS is blank.
>
> scripts/Kbuild.include | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..f7efb59d85d1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
> cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
> $(srctree)/scripts/gcc-version.sh -p $(CC))
>
> +# cc-fullversion-uncached
> +cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
> + $(srctree)/scripts/gcc-version.sh -p $(CC))
> +
> # cc-ifversion
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
> @@ -475,3 +479,16 @@ endif
> endef
> #
> ###############################################################################
> +
> +# Require a 'make clean' if the compiler changed; not only does the .cache.mk
> +# need to be thrown out but we should also start with fresh object files.
> +#
> +# NOTE: it's important that we don't error out when the goal is actually to
> +# try to make clean, distclean or mrproper.
> +ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
> + ifneq ($(MAKECMDGOALS),)
> + ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
> + $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
> + endif
> + endif
> +endif
> --
> 2.15.1.620.gb9897f4670-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk
@ 2017-12-22 22:14 Douglas Anderson
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Anderson @ 2017-12-22 22:14 UTC (permalink / raw)
To: Masahiro Yamada
Cc: malat, dave.hansen, yang.s, linux, Douglas Anderson,
Matthias Kaehlcke, Cao jin, Arnd Bergmann, Marcin Nowakowski,
Mark Charlebois, linux-kernel, Ingo Molnar, Nick Desaulniers
This two-patches fixes two corner cases with .cache.mk that have been
reported recently. Neither problem was catastrophic, but certainly
several people ran into the problem solved by the first patch (can't
build after gcc upgrade) and wasted time debugging, so it's really a
good idea to fix.
For both patches, please make sure to give them an extra thorough
review. I _think_ I understand enough about $(MAKECMDGOALS), but I'd
never explored that feature of make before writing these patches so
the patches certainly need someone more experienced to give them a
careful look.
I've only got one more day of work before I'm on Christmas vacation
for 2 weeks, so if there are problems with these patches please give
me a bit of time to fix. ...or, if someone is in a hurry, I wouldn't
object to someone else hijacking them and posting fixes.
Changes in v2:
- Don't error if MAKECMDGOALS is blank.
Douglas Anderson (2):
kbuild: Require a 'make clean' if we detect gcc changed underneath us
kbuild: Don't mess with the .cache.mk when installing
scripts/Kbuild.include | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
--
2.15.1.620.gb9897f4670-goog
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
2017-12-22 22:14 [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk Douglas Anderson
@ 2017-12-22 22:14 ` Douglas Anderson
2017-12-31 7:46 ` Masahiro Yamada
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Anderson @ 2017-12-22 22:14 UTC (permalink / raw)
To: Masahiro Yamada
Cc: malat, dave.hansen, yang.s, linux, Douglas Anderson,
Matthias Kaehlcke, Cao jin, Arnd Bergmann, Marcin Nowakowski,
Mark Charlebois, linux-kernel, Ingo Molnar, Nick Desaulniers
Several people reported that the commit 3298b690b21c ("kbuild: Add a
cache for generated variables") caused them problems when they updated
gcc versions. Specifically the reports all looked something similar
to this:
> In file included from ./include/uapi/linux/uuid.h:21:0,
> from ./include/linux/uuid.h:19,
> from ./include/linux/mod_devicetable.h:12,
> from scripts/mod/devicetable-offsets.c:2:
> ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
> directory
> #include <stdarg.h>
Masahiro Yamada determined that the problem was with:
NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
-print-file-name=include)
Specifically that the stale result of -print-file-name is stored in
the cache file. It was determined that a "make clean" fixed the
problems in all cases.
In this particular case we could certainly try to clean just the cache
when we detect a gcc update, but it seems like overall it's a bad idea
to do an incremental build when gcc changes. We should warn the user
and tell them that they need a 'make clean'.
Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
Reported-by: Yang Shi <yang.s@alibaba-inc.com>
Reported-by: Dave Hansen <dave.hansen@intel.com>
Reported-by: Mathieu Malaterre <malat@debian.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- Don't error if MAKECMDGOALS is blank.
scripts/Kbuild.include | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..f7efb59d85d1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
$(srctree)/scripts/gcc-version.sh -p $(CC))
+# cc-fullversion-uncached
+cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
+ $(srctree)/scripts/gcc-version.sh -p $(CC))
+
# cc-ifversion
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
@@ -475,3 +479,16 @@ endif
endef
#
###############################################################################
+
+# Require a 'make clean' if the compiler changed; not only does the .cache.mk
+# need to be thrown out but we should also start with fresh object files.
+#
+# NOTE: it's important that we don't error out when the goal is actually to
+# try to make clean, distclean or mrproper.
+ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
+ ifneq ($(MAKECMDGOALS),)
+ ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
+ $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
+ endif
+ endif
+endif
--
2.15.1.620.gb9897f4670-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
@ 2017-12-31 7:46 ` Masahiro Yamada
0 siblings, 0 replies; 3+ messages in thread
From: Masahiro Yamada @ 2017-12-31 7:46 UTC (permalink / raw)
To: Douglas Anderson
Cc: malat, dave.hansen, Yang Shi, Guenter Roeck, Matthias Kaehlcke,
Cao jin, Arnd Bergmann, Marcin Nowakowski, Mark Charlebois,
Linux Kernel Mailing List, Ingo Molnar, Nick Desaulniers
Hi Douglas,
2017-12-23 7:14 GMT+09:00 Douglas Anderson <dianders@chromium.org>:
> Several people reported that the commit 3298b690b21c ("kbuild: Add a
> cache for generated variables") caused them problems when they updated
> gcc versions. Specifically the reports all looked something similar
> to this:
>
>> In file included from ./include/uapi/linux/uuid.h:21:0,
>> from ./include/linux/uuid.h:19,
>> from ./include/linux/mod_devicetable.h:12,
>> from scripts/mod/devicetable-offsets.c:2:
>> ./include/linux/string.h:8:20: fatal error: stdarg.h: No such file or
>> directory
>> #include <stdarg.h>
>
> Masahiro Yamada determined that the problem was with:
>
> NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC)
> -print-file-name=include)
>
> Specifically that the stale result of -print-file-name is stored in
> the cache file. It was determined that a "make clean" fixed the
> problems in all cases.
>
> In this particular case we could certainly try to clean just the cache
> when we detect a gcc update, but it seems like overall it's a bad idea
> to do an incremental build when gcc changes. We should warn the user
> and tell them that they need a 'make clean'.
>
> Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
> Reported-by: Yang Shi <yang.s@alibaba-inc.com>
> Reported-by: Dave Hansen <dave.hansen@intel.com>
> Reported-by: Mathieu Malaterre <malat@debian.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Don't error if MAKECMDGOALS is blank.
>
> scripts/Kbuild.include | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..f7efb59d85d1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -222,6 +222,10 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
> cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
> $(srctree)/scripts/gcc-version.sh -p $(CC))
>
> +# cc-fullversion-uncached
> +cc-fullversion-uncached := $(shell $(CONFIG_SHELL) \
> + $(srctree)/scripts/gcc-version.sh -p $(CC))
> +
No.
You used ':=' flavor assignment, so the gcc-version.sh
script is evaluated here.
The top-level Makefile includes scripts/Kbuild.include at line 278,
then defines CC at line 284.
Since $(CC) is not defined here,
the resulted cc-fullversion-uncached is
"Error: No compiler specified. Usage: ./scripts/gcc-version.sh <gcc-command>"
cc-fullversion-uncached works as expected
only after Kbuild descends into sub-directories.
> # cc-ifversion
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
> @@ -475,3 +479,16 @@ endif
> endef
> #
> ###############################################################################
> +
> +# Require a 'make clean' if the compiler changed; not only does the .cache.mk
> +# need to be thrown out but we should also start with fresh object files.
> +#
> +# NOTE: it's important that we don't error out when the goal is actually to
> +# try to make clean, distclean or mrproper.
> +ifeq ($(filter %clean,$(MAKECMDGOALS))$(filter mrproper,$(MAKECMDGOALS)),)
> + ifneq ($(MAKECMDGOALS),)
> + ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
As I noted above, CC is not defined yet when parsing the top-level Makefile.
Evaluating "cc-fullversion" here
adds a strange cache line, like follows:
__cached__./scripts/gcc-version.sh_-p_ := Error: No compiler
specified. Usage: ./scripts/gcc-version.sh <gcc-command>
This check only works only in the second inclusion or later.
At the first inclusion,
both 'cc-fullversion-uncached' and 'cc-fullversion'
contain "Error: No compiler specified. ..."
scripts/Kbuild.include is included every time Kbuild
descends into a sub-directory.
It is pointless to check gcc version multiple times.
> + $(error Detected new CC version ($(cc-fullversion-uncached) vs $(cc-fullversion)). Please 'make clean')
> + endif
> + endif
> +endif
> --
> 2.15.1.620.gb9897f4670-goog
>
After all, I recommend to move this code into the top-level Makefile,
around line 585.
--- a/Makefile
+++ b/Makefile
@@ -585,6 +585,14 @@ virt-y := virt/
endif # KBUILD_EXTMOD
ifeq ($(dot-config),1)
+# Require a 'make clean' if the compiler changed; not only does the .cache.mk
+# need to be thrown out but we should also start with fresh object files.
+cc-fullversion-uncached := $(shell $(CONFIG_SHELL)
$(srctree)/scripts/gcc-version.sh -p $(CC))
+
+ifneq ($(cc-fullversion-uncached),$(cc-fullversion))
+ $(error Detected new CC version ($(cc-fullversion-uncached) vs
$(cc-fullversion)). Please 'make clean')
+endif
+
# Read in config
-include include/config/auto.conf
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-31 7:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-22 22:44 [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Guenter Roeck
-- strict thread matches above, loose matches on Subject: below --
2017-12-22 22:14 [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk Douglas Anderson
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
2017-12-31 7:46 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox