[PATCH] build: fix dependencies for files compiled from their parent directory

[PATCH] build: fix dependencies for files compiled from their parent directory

[Jan Beulich]

The use of $(basename ...) here was wrong (yet I'm sure I tested it).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -105,7 +105,7 @@ include Makefile
 DEPS = .*.d
 define gendep
     ifneq ($(1),$(subst /,:,$(1)))
-        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
+        DEPS += $(dir $(1)).$(notdir $(1)).d
     endif
 endef
 $(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o))))

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Ian Campbell]

On Wed, 2015-11-25 at 09:16 -0700, Jan Beulich wrote:
> The use of $(basename ...) here was wrong (yet I'm sure I tested it).

Is the issue here that xen/arch/x86/x86_64/.compat.o.d ought really to be
xen/arch/x86/.x86_64.compat.o.d?

Otherwise xen/arch/x86/Makefile (which contains obj-y := ...
x86_64/compat.o) does not correctly track the dependencies of that file?

But that being the case I'm then confused, since it doesn't seem that the
currently wrong version is missing the .o or anything like that which is
what I would have expected the basename to be doing.

This is all in a recently built x86_64 tree (the one I used when building
before push earlier today).

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -105,7 +105,7 @@ include Makefile
>  DEPS = .*.d
>  define gendep
>      ifneq ($(1),$(subst /,:,$(1)))
> -        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
> +        DEPS += $(dir $(1)).$(notdir $(1)).d
>      endif
>  endef
>  $(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o))))
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Jan Beulich]

>>> On 25.11.15 at 17:26, <ian.campbell@citrix.com> wrote:
> On Wed, 2015-11-25 at 09:16 -0700, Jan Beulich wrote:
>> The use of $(basename ...) here was wrong (yet I'm sure I tested it).
> 
> Is the issue here that xen/arch/x86/x86_64/.compat.o.d ought really to be
> xen/arch/x86/.x86_64.compat.o.d?

No, xen/arch/x86/x86_64/.compat.o.d is the correct name. Just
that $(dir $(1)).$(basename $(notdir $(1))).d produces
xen/arch/x86/x86_64/.compat.d (i.e. strips the .o, which is not in
line with $(@D)/.$(@F).d used to generate those files), and hence
neither dependency tracking nor cleaning work.

Jan

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Ian Campbell]

On Thu, 2015-11-26 at 00:43 -0700, Jan Beulich wrote:
> > > > On 25.11.15 at 17:26, <ian.campbell@citrix.com> wrote:
> > On Wed, 2015-11-25 at 09:16 -0700, Jan Beulich wrote:
> > > The use of $(basename ...) here was wrong (yet I'm sure I tested it).
> > 
> > Is the issue here that xen/arch/x86/x86_64/.compat.o.d ought really to
> > be
> > xen/arch/x86/.x86_64.compat.o.d?
> 
> No, xen/arch/x86/x86_64/.compat.o.d is the correct name. Just
> that $(dir $(1)).$(basename $(notdir $(1))).d produces
> xen/arch/x86/x86_64/.compat.d (i.e. strips the .o, which is not in
> line with $(@D)/.$(@F).d used to generate those files), and hence
> neither dependency tracking nor cleaning work.

My tree, which was last built with 98b69a010c24 has:

    ianc@cosworth    :committer-amd64.git$ find xen -name .\*compat\*
    xen/arch/x86/.compat.o.d
    xen/arch/x86/x86_64/.compat.o.d
    xen/arch/x86/efi/.compat.o.d
    ianc@cosworth    :committer-amd64.git$ 

Which is very odd, since the prevailing xen/Rules.mk has:

    define gendep
        ifneq ($(1),$(subst /,:,$(1)))
            DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
        endif
    endef

Which I agree looks as if it should have stripped the .o as you describe,
but it seems not to have done so. AIUI makes "basename" differs from the
CLI basename(1) in that the make one will always strip 

Just to double check I did:

    $ make --version
    GNU Make 4.0
    Built for x86_64-pc-linux-gnu
    Copyright (C) 1988-2013 Free Software Foundation, Inc.
    License GPLv3+: GNU GPL version 3 or later <    http://gnu.org/licenses/gpl.html    >
    This is free software: you are free to change and redistribute it.
    There is NO WARRANTY, to the extent permitted by law.
    ianc@cosworth    :committer-amd64.git$ git clean -fdqx xen/
    ianc@cosworth    :committer-amd64.git$ make -j12 xen -s
     __  __            _  _   _____                  _        _     _      
     \ \/ /___ _ __   | || | |___  | _   _ _ __  ___| |_ __ _| |__ | | ___ 
      \  // _ \ '_ \  | || |_   / /_| | | | '_ \/ __| __/ _` | '_ \| |/ _ \
      /  \  __/ | | | |__   _| / /__| |_| | | | \__ \ || (_| | |_) | |  __/
     /_/\_\___|_| |_|    |_|(_)_/    \__,_|_| |_|___/\__\__,_|_.__/|_|\___|
                                                                           
    ianc@cosworth    :committer-amd64.git$ find xen -name .\*compat\*
    xen/arch/x86/.compat.o.d
    xen/arch/x86/x86_64/.compat.o.d
    xen/arch/x86/efi/.compat.o.d
    ianc@cosworth    :committer-amd64.git$ git grep -A4 define.gendep xen/Rules.mk
    xen/Rules.mk:define gendep
    xen/Rules.mk-    ifneq ($(1),$(subst /,:,$(1)))
    xen/Rules.mk-        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
    xen/Rules.mk-    endif
    xen/Rules.mk-endef

Maybe the behaviour of basename varies with make versions?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Jan Beulich]

>>> On 26.11.15 at 10:51, <ian.campbell@citrix.com> wrote:
> On Thu, 2015-11-26 at 00:43 -0700, Jan Beulich wrote:
>> > > > On 25.11.15 at 17:26, <ian.campbell@citrix.com> wrote:
>> > On Wed, 2015-11-25 at 09:16 -0700, Jan Beulich wrote:
>> > > The use of $(basename ...) here was wrong (yet I'm sure I tested it).
>> > 
>> > Is the issue here that xen/arch/x86/x86_64/.compat.o.d ought really to
>> > be
>> > xen/arch/x86/.x86_64.compat.o.d?
>> 
>> No, xen/arch/x86/x86_64/.compat.o.d is the correct name. Just
>> that $(dir $(1)).$(basename $(notdir $(1))).d produces
>> xen/arch/x86/x86_64/.compat.d (i.e. strips the .o, which is not in
>> line with $(@D)/.$(@F).d used to generate those files), and hence
>> neither dependency tracking nor cleaning work.
> 
> My tree, which was last built with 98b69a010c24 has:
> 
>     ianc@cosworth    :committer-amd64.git$ find xen -name .\*compat\*
>     xen/arch/x86/.compat.o.d
>     xen/arch/x86/x86_64/.compat.o.d
>     xen/arch/x86/efi/.compat.o.d
>     ianc@cosworth    :committer-amd64.git$ 
> 
> Which is very odd, since the prevailing xen/Rules.mk has:
> 
>     define gendep
>         ifneq ($(1),$(subst /,:,$(1)))
>             DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
>         endif
>     endef
> 
> Which I agree looks as if it should have stripped the .o as you describe,
> but it seems not to have done so. AIUI makes "basename" differs from the
> CLI basename(1) in that the make one will always strip 

You're looking at the wrong thing I'm afraid: How the files get named
depends on the $(@D)/.$(@F).d thing in CFLAGS. What gets put in
$(DEPS) (and hence gets included) depends on the construct above.
So the correctly named files get generated, but they get neither
included for proper dependency checking, nor would they get
deleted by the "clean" rule. And since the inclusion of $(DEPS) has a
minus sign in front, make wouldn't complain about not finding these
files.

Jan

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Ian Campbell]

On Thu, 2015-11-26 at 03:38 -0700, Jan Beulich wrote:
> > > > On 26.11.15 at 10:51, <ian.campbell@citrix.com> wrote:
> > On Thu, 2015-11-26 at 00:43 -0700, Jan Beulich wrote:
> > > > > > On 25.11.15 at 17:26, <ian.campbell@citrix.com> wrote:
> > > > On Wed, 2015-11-25 at 09:16 -0700, Jan Beulich wrote:
> > > > > The use of $(basename ...) here was wrong (yet I'm sure I tested
> > > > > it).
> > > > 
> > > > Is the issue here that xen/arch/x86/x86_64/.compat.o.d ought really
> > > > to
> > > > be
> > > > xen/arch/x86/.x86_64.compat.o.d?
> > > 
> > > No, xen/arch/x86/x86_64/.compat.o.d is the correct name. Just
> > > that $(dir $(1)).$(basename $(notdir $(1))).d produces
> > > xen/arch/x86/x86_64/.compat.d (i.e. strips the .o, which is not in
> > > line with $(@D)/.$(@F).d used to generate those files), and hence
> > > neither dependency tracking nor cleaning work.
> > 
> > My tree, which was last built with 98b69a010c24 has:
> > 
> >     ianc@cosworth    :committer-amd64.git$ find xen -name .\*compat\*
> >     xen/arch/x86/.compat.o.d
> >     xen/arch/x86/x86_64/.compat.o.d
> >     xen/arch/x86/efi/.compat.o.d
> >     ianc@cosworth    :committer-amd64.git$ 
> > 
> > Which is very odd, since the prevailing xen/Rules.mk has:
> > 
> >     define gendep
> >         ifneq ($(1),$(subst /,:,$(1)))
> >             DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
> >         endif
> >     endef
> > 
> > Which I agree looks as if it should have stripped the .o as you
> > describe,
> > but it seems not to have done so. AIUI makes "basename" differs from
> > the
> > CLI basename(1) in that the make one will always strip 
> 
> You're looking at the wrong thing I'm afraid: How the files get named
> depends on the $(@D)/.$(@F).d thing in CFLAGS. What gets put in
> $(DEPS) (and hence gets included) depends on the construct above.
> So the correctly named files get generated, but they get neither
> included for proper dependency checking, nor would they get
> deleted by the "clean" rule. And since the inclusion of $(DEPS) has a
> minus sign in front, make wouldn't complain about not finding these
> files.

Aha, gotcha now:

$ make clean
[...]
make[3]: Leaving directory '/local/scratch/ianc/devel/committer-amd64.git/xen/arch/x86/x86_64'
rm -f *.o *~ core .*.d x86_64/.compat.d x86_64/.mm.d x86_64/.physdev.d x86_64/.platform_hypercall.d

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Jonathan Creekmore]

On Wed, Nov 25, 2015 at 10:16 AM, Jan Beulich <JBeulich@suse.com> wrote:
> The use of $(basename ...) here was wrong (yet I'm sure I tested it).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -105,7 +105,7 @@ include Makefile
>  DEPS = .*.d
>  define gendep
>      ifneq ($(1),$(subst /,:,$(1)))
> -        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
> +        DEPS += $(dir $(1)).$(notdir $(1)).d
>      endif
>  endef
>  $(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o))))

I finally got a chance to take a look at this patch today and it does fix the
dependency tracking for files in the xen/common/compat directory. However,
it still doesn't do anything to fix the failure to clean the object
files that are
left behind in that directory by 'make clean'. After running it, I still have:

$ find . -name "*.o*"
./common/compat/kernel.o
./common/compat/domain.o
./common/compat/tmem_xen.o
./common/compat/memory.o
./common/compat/multicall.o
./common/compat/xlat.o

Re: [PATCH] build: fix dependencies for files compiled from their parent directory

[Jan Beulich]

>>> On 30.11.15 at 17:31, <jonathan@thecreekmores.org> wrote:
> On Wed, Nov 25, 2015 at 10:16 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> The use of $(basename ...) here was wrong (yet I'm sure I tested it).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -105,7 +105,7 @@ include Makefile
>>  DEPS = .*.d
>>  define gendep
>>      ifneq ($(1),$(subst /,:,$(1)))
>> -        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
>> +        DEPS += $(dir $(1)).$(notdir $(1)).d
>>      endif
>>  endef
>>  $(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o))))
> 
> I finally got a chance to take a look at this patch today and it does fix 
> the
> dependency tracking for files in the xen/common/compat directory. However,
> it still doesn't do anything to fix the failure to clean the object
> files that are
> left behind in that directory by 'make clean'. After running it, I still 
> have:
> 
> $ find . -name "*.o*"
> ./common/compat/kernel.o
> ./common/compat/domain.o
> ./common/compat/tmem_xen.o
> ./common/compat/memory.o
> ./common/compat/multicall.o
> ./common/compat/xlat.o

Oh, it wasn't even clear to me that these too are a problem.
Mind contributing another patch?

Jan