Skip to contentSkip to conversation

make: improve check for stale generated files in source dir

PATCH In review Daniel P. Berrangé 3 participants 4 messages Mar 20, 2018 – Mar 21, 2018 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 make: improve check for stale generated files in source dir

When doing a build with builddir != srcdir, if any generated files are accidentally present in srcdir from a previous build, these can cause unexpected failures.

Currently there is a rule that checks for existance of config-host.mak, but there have been cases where config-host.mak is absent, while other generated files still exist.

Update the check to look at every file listed in $(GENERATED_FILES). To do this we must move the check further down after $(GENERATED_FILES) has been populated.

---
 Makefile | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
Makefile +19 -10
index c8116694a0..73caabc019 100644
@@ -52,16 +52,6 @@ endif
 
 .git-submodule-status: git-submodule-update config-host.mak
 
-# Check that we're not trying to do an out-of-tree build from
-# a tree that's been used for an in-tree build.
-ifneq ($(realpath $(SRC_PATH)),$(realpath .))
-ifneq ($(wildcard $(SRC_PATH)/config-host.mak),)
-$(error This is an out of tree build but your source tree ($(SRC_PATH)) \
-seems to have been used for an in-tree build. You can fix this by running \
-"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
-endif
-endif
-
 CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
 CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
 CONFIG_XEN := $(CONFIG_XEN_BACKEND)
@@ -1027,6 +1017,25 @@ ifdef SIGNCODE
 endif # SIGNCODE
 endif # CONFIG_WIN
 
+define nl
+
+
+endef
+
+CHECK_FILES = config-host.mak $(filter-out .git-submodule-status, $(GENERATED_FILES))
+UNEXPECTED_FILES = $(wildcard $(CHECK_FILES:%=$(SRC_PATH)/%))
+
+# Check that we're not trying to do an out-of-tree build from
+# a tree that's been used for an in-tree build.
+ifneq ($(realpath $(SRC_PATH)),$(realpath .))
+ifneq ($(UNEXPECTED_FILES),)
+$(error Stale files in source tree:${nl}${nl} $(UNEXPECTED_FILES:%=  %${nl}) ${nl}\
+This is an out of tree build but your source tree ($(SRC_PATH)) \
+seems to have been used for an in-tree build. You can fix this by running \
+"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
+endif
+endif
+
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
 ifneq ($(wildcard config-host.mak),)
Signature · 1 line
2.14.3
#1 lore raw
Quoted 55 lines · Daniel P. Berrangé

When doing a build with builddir != srcdir, if any generated files are accidentally present in srcdir from a previous build, these can cause unexpected failures.

Currently there is a rule that checks for existance of config-host.mak, but there have been cases where config-host.mak is absent, while other generated files still exist.

Update the check to look at every file listed in $(GENERATED_FILES). To do this we must move the check further down after $(GENERATED_FILES) has been populated.

---
 Makefile | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
Makefile +13 -10
index c8116694a0..73caabc019 100644
@@ -52,16 +52,6 @@ endif
 
 .git-submodule-status: git-submodule-update config-host.mak
 
-# Check that we're not trying to do an out-of-tree build from
-# a tree that's been used for an in-tree build.
-ifneq ($(realpath $(SRC_PATH)),$(realpath .))
-ifneq ($(wildcard $(SRC_PATH)/config-host.mak),)
-$(error This is an out of tree build but your source tree ($(SRC_PATH)) \
-seems to have been used for an in-tree build. You can fix this by running \
-"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
-endif
-endif
-
 CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
 CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
 CONFIG_XEN := $(CONFIG_XEN_BACKEND)
@@ -1027,6 +1017,25 @@ ifdef SIGNCODE
 endif # SIGNCODE
 endif # CONFIG_WIN
 
+define nl
+
+
+endef
+
+CHECK_FILES = config-host.mak $(filter-out .git-submodule-status, $(GENERATED_FILES))
+UNEXPECTED_FILES = $(wildcard $(CHECK_FILES:%=$(SRC_PATH)/%))
+
+# Check that we're not trying to do an out-of-tree build from
+# a tree that's been used for an in-tree build.
+ifneq ($(realpath $(SRC_PATH)),$(realpath .))
+ifneq ($(UNEXPECTED_FILES),)
+$(error Stale files in source tree:${nl}${nl} $(UNEXPECTED_FILES:%=  %${nl}) ${nl}\

Makefile ifdef SIGNCODE

Quoted 10 lines
Makefile +6 -1
+This is an out of tree build but your source tree ($(SRC_PATH)) \
+seems to have been used for an in-tree build. You can fix this by running \
+"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
+endif
+endif
+
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
 ifneq ($(wildcard config-host.mak),)
--

Does this still mean the check is made sufficiently early, before we start trying to build config-all-devices.mak and other things? This change has moved it down after 'include' directives which I think will cause make to try to build the file that it must include.

It's also moved out of the "only run this if we've actually run configure" block, but I think that part is OK.

thanks
-- PMM

#2 lore raw
Quoted 64 lines · Peter Maydell
Quoted 63 lines · Daniel P. Berrangé

When doing a build with builddir != srcdir, if any generated files are accidentally present in srcdir from a previous build, these can cause unexpected failures.

Currently there is a rule that checks for existance of config-host.mak, but there have been cases where config-host.mak is absent, while other generated files still exist.

Update the check to look at every file listed in $(GENERATED_FILES). To do this we must move the check further down after $(GENERATED_FILES) has been populated.

---
 Makefile | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
Makefile +19 -10
index c8116694a0..73caabc019 100644
@@ -52,16 +52,6 @@ endif
 
 .git-submodule-status: git-submodule-update config-host.mak
 
-# Check that we're not trying to do an out-of-tree build from
-# a tree that's been used for an in-tree build.
-ifneq ($(realpath $(SRC_PATH)),$(realpath .))
-ifneq ($(wildcard $(SRC_PATH)/config-host.mak),)
-$(error This is an out of tree build but your source tree ($(SRC_PATH)) \
-seems to have been used for an in-tree build. You can fix this by running \
-"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
-endif
-endif
-
 CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
 CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
 CONFIG_XEN := $(CONFIG_XEN_BACKEND)
@@ -1027,6 +1017,25 @@ ifdef SIGNCODE
 endif # SIGNCODE
 endif # CONFIG_WIN
 
+define nl
+
+
+endef
+
+CHECK_FILES = config-host.mak $(filter-out .git-submodule-status, $(GENERATED_FILES))
+UNEXPECTED_FILES = $(wildcard $(CHECK_FILES:%=$(SRC_PATH)/%))
+
+# Check that we're not trying to do an out-of-tree build from
+# a tree that's been used for an in-tree build.
+ifneq ($(realpath $(SRC_PATH)),$(realpath .))
+ifneq ($(UNEXPECTED_FILES),)
+$(error Stale files in source tree:${nl}${nl} $(UNEXPECTED_FILES:%=  %${nl}) ${nl}\
+This is an out of tree build but your source tree ($(SRC_PATH)) \
+seems to have been used for an in-tree build. You can fix this by running \
+"$(MAKE) distclean && rm -rf *-linux-user *-softmmu" in your source tree)
+endif
+endif
+
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
Quoted 8 lines

Makefile ifdef SIGNCODE

Quoted 2 lines
Makefile +0 -1
 ifneq ($(wildcard config-host.mak),)
--

Does this still mean the check is made sufficiently early, before we start trying to build config-all-devices.mak and other things? This change has moved it down after 'include' directives which I think will cause make to try to build the file that it must include.

I'm not 100% sure, but what I did to test was as follows, starting from git srcdir

   ./configure --target-list=x86_64-softmmu
   make -j 8
   mkdir build
   cd build
   ../configure --target-list=x86_64-softmmu
   find -type f > /tmp/before.txt
   make -j 8
   find -type f > /tmp/after.txt

and confirmed that before.txt & after.txt have same list of files after make complains about dirty srcdir. So it does not appear to be triggering any rebuild rules.

I also did another tests where instad of running 'make' in the src dir, I just arbitrarily touchd the single qemu-version.h file

I think that's important here is that this error checking logic is not attached to any make target - it is something that is run immediately as make parses the makefile, before executing any rules. So as long as we don't have other code that is target independant, we should be ok.

Quoted 2 lines

It's also moved out of the "only run this if we've actually run configure" block, but I think that part is OK.

Yeah I think that is safe for this usage scenario.

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Tested-by on v1 1/1
#3 lore raw
In reply to Daniel P. Berrangé

Le 20/03/2018 à 15:23, Daniel P. Berrangé a écrit :

Quoted 8 lines

When doing a build with builddir != srcdir, if any generated files are accidentally present in srcdir from a previous build, these can cause unexpected failures.

Currently there is a rule that checks for existance of config-host.mak, but there have been cases where config-host.mak is absent, while other generated files still exist.

Quoted 8 lines

Update the check to look at every file listed in $(GENERATED_FILES). To do this we must move the check further down after $(GENERATED_FILES) has been populated.

---
 Makefile | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

I had a stale qemu-version.h and this change has correctly detected it.

The problem was:

$ make -C build/all
...
./qemu-nbd.c: In function ‘version’:
./qemu-nbd.c:133:7: error: expected ‘)’ before ‘QEMU_FULL_VERSION’
 "%s " QEMU_FULL_VERSION "\n"
       ^~~~~~~~~~~~~~~~~
./qemu-nbd.c:133:3: error: format ‘%s’ expects a matching ‘char *’
argument [-Werror=format=]
 "%s " QEMU_FULL_VERSION "\n"
  ~^
cc1: all warnings being treated as errors
make: *** [./rules.mak:66: qemu-nbd.o] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory './build/all'

once the file removed, the build can be done.

Thanks,
Laurent