| Message ID | 20170206112953.16993-1-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon 06 Feb 2017 12:29:53 PM CET, "Daniel P. Berrange" wrote: > Rather than trying to workaround this, just revert the code that > attempted to mkdir the missing include directories. Instead just turn > off the compiler warning in question as the missing dir is expected & > harmless in general. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Tested-by: Alberto Garcia <berto@igalia.com> Berto
On 6 February 2017 at 11:29, Daniel P. Berrange <berrange@redhat.com> wrote: > In > > commit ba78db44f6532d66a1e704bd44613e841baa2fc5 > Author: Daniel P. Berrange <berrange@redhat.com> > Date: Wed Jan 25 16:14:10 2017 +0000 > > make: move top level dir to end of include search path > > The dir $(BUILD_DIR)/$(@D) was added to the include > path. This would sometimes point to a non-existant > directory, if the sub-dir in question did not contain > any target-independant files (eg tcg/). To deal with > this the rules.mak attempted to create the directory. > > While this was succesful, it also caused accidental > creation of files in the parent of the build dir. > e.g. when building common source files into target > specific output files. Aha, that's where those directories came from! > Rather than trying to workaround this, just revert > the code that attempted to mkdir the missing include > directories. Instead just turn off the compiler warning > in question as the missing dir is expected & harmless > in general. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > configure | 2 +- > rules.mak | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/configure b/configure > index 86fd833..6325339 100755 > --- a/configure > +++ b/configure > @@ -1474,7 +1474,7 @@ fi > > gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" > gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" > -gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" > +gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" > gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" > gcc_flags="-Wno-initializer-overrides $gcc_flags" > gcc_flags="-Wno-string-plus-int $gcc_flags" > diff --git a/rules.mak b/rules.mak > index 575a3af..83d6dd1 100644 > --- a/rules.mak > +++ b/rules.mak > @@ -374,7 +374,6 @@ define unnest-vars > $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)), > $(error $o added in $v but $o-objs is not set))) > $(shell mkdir -p ./ $(sort $(dir $($v)))) > - $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) I know this is the same syntax as the existing line above and we're deleting it anyway, but what does it actually do? When does telling mkdir to create "./" make sense? thanks -- PMM
On Mon, Feb 06, 2017 at 11:50:09AM +0000, Peter Maydell wrote: > On 6 February 2017 at 11:29, Daniel P. Berrange <berrange@redhat.com> wrote: > > In > > > > commit ba78db44f6532d66a1e704bd44613e841baa2fc5 > > Author: Daniel P. Berrange <berrange@redhat.com> > > Date: Wed Jan 25 16:14:10 2017 +0000 > > > > make: move top level dir to end of include search path > > > > The dir $(BUILD_DIR)/$(@D) was added to the include > > path. This would sometimes point to a non-existant > > directory, if the sub-dir in question did not contain > > any target-independant files (eg tcg/). To deal with > > this the rules.mak attempted to create the directory. > > > > While this was succesful, it also caused accidental > > creation of files in the parent of the build dir. > > e.g. when building common source files into target > > specific output files. > > Aha, that's where those directories came from! > > > Rather than trying to workaround this, just revert > > the code that attempted to mkdir the missing include > > directories. Instead just turn off the compiler warning > > in question as the missing dir is expected & harmless > > in general. > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > configure | 2 +- > > rules.mak | 1 - > > 2 files changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/configure b/configure > > index 86fd833..6325339 100755 > > --- a/configure > > +++ b/configure > > @@ -1474,7 +1474,7 @@ fi > > > > gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" > > gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" > > -gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" > > +gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" > > gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" > > gcc_flags="-Wno-initializer-overrides $gcc_flags" > > gcc_flags="-Wno-string-plus-int $gcc_flags" > > diff --git a/rules.mak b/rules.mak > > index 575a3af..83d6dd1 100644 > > --- a/rules.mak > > +++ b/rules.mak > > @@ -374,7 +374,6 @@ define unnest-vars > > $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)), > > $(error $o added in $v but $o-objs is not set))) > > $(shell mkdir -p ./ $(sort $(dir $($v)))) > > - $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) > > I know this is the same syntax as the existing line above > and we're deleting it anyway, but what does it actually do? > When does telling mkdir to create "./" make sense? No idea why the ./ was there originally - it appears to serve no purpose. The useful bit is the stuff afterwards - the $($v) bit. It gets populated based on the variable being unnested. For example block-obj-y = block.o blockjob.o block/ nbd/ will make $v contain "block nbd", hence cause creation of those dirs in the the build dir. Regards, Daniel
On 06/02/2017 13:05, Daniel P. Berrange wrote: >>> $(shell mkdir -p ./ $(sort $(dir $($v)))) >>> - $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) >> I know this is the same syntax as the existing line above >> and we're deleting it anyway, but what does it actually do? >> When does telling mkdir to create "./" make sense? > No idea why the ./ was there originally - it appears to serve no > purpose. The useful bit is the stuff afterwards - the $($v) bit. > It gets populated based on the variable being unnested. For example > > block-obj-y = block.o blockjob.o block/ nbd/ > > will make $v contain "block nbd", hence cause creation of those > dirs in the the build dir. "mkdir -p" with no arguments gives an error, the "./" shuts it up. Paolo
On Mon, Feb 06, 2017 at 01:22:08PM +0100, Paolo Bonzini wrote: > > > On 06/02/2017 13:05, Daniel P. Berrange wrote: > >>> $(shell mkdir -p ./ $(sort $(dir $($v)))) > >>> - $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) > >> I know this is the same syntax as the existing line above > >> and we're deleting it anyway, but what does it actually do? > >> When does telling mkdir to create "./" make sense? > > No idea why the ./ was there originally - it appears to serve no > > purpose. The useful bit is the stuff afterwards - the $($v) bit. > > It gets populated based on the variable being unnested. For example > > > > block-obj-y = block.o blockjob.o block/ nbd/ > > > > will make $v contain "block nbd", hence cause creation of those > > dirs in the the build dir. > > "mkdir -p" with no arguments gives an error, the "./" shuts it up. Ah yes, clever. So that deals with case of unnesting a variable which does not contain any nested dirs. Regards, Daniel
On 6 February 2017 at 11:29, Daniel P. Berrange <berrange@redhat.com> wrote: > In > > commit ba78db44f6532d66a1e704bd44613e841baa2fc5 > Author: Daniel P. Berrange <berrange@redhat.com> > Date: Wed Jan 25 16:14:10 2017 +0000 > > make: move top level dir to end of include search path > > The dir $(BUILD_DIR)/$(@D) was added to the include > path. This would sometimes point to a non-existant > directory, if the sub-dir in question did not contain > any target-independant files (eg tcg/). To deal with > this the rules.mak attempted to create the directory. > > While this was succesful, it also caused accidental > creation of files in the parent of the build dir. > e.g. when building common source files into target > specific output files. > > Rather than trying to workaround this, just revert > the code that attempted to mkdir the missing include > directories. Instead just turn off the compiler warning > in question as the missing dir is expected & harmless > in general. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- Thanks; planning to apply this to master as a buildfix with the following added to the commit message: NB: you can clean up a build directory parent that has been filled with empty directories by commit ba78db44f653 using this GNU find command in that parent directory: find audio backends block chardev crypto disas fsdev hw io linux-user \ migration nbd net qapi qom replay slirp target ui util \ -type d -empty -delete thanks -- PMM
On Mon, Feb 06, 2017 at 11:29:53AM +0000, Daniel P. Berrange wrote: > In > > commit ba78db44f6532d66a1e704bd44613e841baa2fc5 > Author: Daniel P. Berrange <berrange@redhat.com> > Date: Wed Jan 25 16:14:10 2017 +0000 > > make: move top level dir to end of include search path > > The dir $(BUILD_DIR)/$(@D) was added to the include > path. This would sometimes point to a non-existant > directory, if the sub-dir in question did not contain > any target-independant files (eg tcg/). To deal with > this the rules.mak attempted to create the directory. > > While this was succesful, it also caused accidental > creation of files in the parent of the build dir. > e.g. when building common source files into target > specific output files. > > Rather than trying to workaround this, just revert > the code that attempted to mkdir the missing include > directories. Instead just turn off the compiler warning > in question as the missing dir is expected & harmless > in general. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > configure | 2 +- > rules.mak | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/configure b/configure index 86fd833..6325339 100755 --- a/configure +++ b/configure @@ -1474,7 +1474,7 @@ fi gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" -gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" +gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" gcc_flags="-Wno-initializer-overrides $gcc_flags" gcc_flags="-Wno-string-plus-int $gcc_flags" diff --git a/rules.mak b/rules.mak index 575a3af..83d6dd1 100644 --- a/rules.mak +++ b/rules.mak @@ -374,7 +374,6 @@ define unnest-vars $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)), $(error $o added in $v but $o-objs is not set))) $(shell mkdir -p ./ $(sort $(dir $($v)))) - $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) # Include all the .d files $(eval -include $(patsubst %.o,%.d,$(patsubst %.mo,%.d,$($v)))) $(eval $v := $(filter-out %/,$($v))))
In commit ba78db44f6532d66a1e704bd44613e841baa2fc5 Author: Daniel P. Berrange <berrange@redhat.com> Date: Wed Jan 25 16:14:10 2017 +0000 make: move top level dir to end of include search path The dir $(BUILD_DIR)/$(@D) was added to the include path. This would sometimes point to a non-existant directory, if the sub-dir in question did not contain any target-independant files (eg tcg/). To deal with this the rules.mak attempted to create the directory. While this was succesful, it also caused accidental creation of files in the parent of the build dir. e.g. when building common source files into target specific output files. Rather than trying to workaround this, just revert the code that attempted to mkdir the missing include directories. Instead just turn off the compiler warning in question as the missing dir is expected & harmless in general. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure | 2 +- rules.mak | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)