[Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Daniel P. Berrange]

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

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

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Alberto Garcia]

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

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Daniel P. Berrange]

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Daniel P. Berrange]

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] rules: don't try to create missing include dirs

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

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>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]