[PATCH] build: fix clean rule to cover objects in unvisited subdirs

[PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

In commit 8b6ef9c152edceabecc7f90c811cd538a7b7a110,
several files in xen/common/compat were changed to be built
using the Makefile in xen/common, by appending the compat
prefix to the object files. Additionally, the
xen/common/compat directory was removed from the subdirs-y
variable, so it is no longer visited by the clean rule. This
resulted in some object files being built by inclusion into
obj-y, but not cleaned because they lived in a directory that
was unvisited by the clean rules. Appending obj-y to the clean
rule causes object files of this type to be cleaned up along
with files in the visited directory.

CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Keir Fraser <keir@xen.org>
CC: Tim Deegan <tim@xen.org>
Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com>
---
 xen/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 02db110..539722f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -173,7 +173,7 @@ FORCE:
 
 .PHONY: clean
 clean:: $(addprefix _clean_, $(subdir-all))
-	rm -f *.o *~ core $(DEPS)
+	rm -f *.o *~ core $(DEPS) $(obj-y)
 _clean_%/: FORCE
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
 
-- 
2.6.2

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jan Beulich]

>>> On 30.11.15 at 18:29, <jonathan.creekmore@gmail.com> wrote:
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -173,7 +173,7 @@ FORCE:
>  
>  .PHONY: clean
>  clean:: $(addprefix _clean_, $(subdir-all))
> -	rm -f *.o *~ core $(DEPS)
> +	rm -f *.o *~ core $(DEPS) $(obj-y)

While for the moment it would do, using a minimalistic approach
like this will make us touch this again the moment we gain files in
subdirectories that can be built just optionally. At the very least
I'd therefore suggest also adding $(obj-n) and $(obj-) here. It
might even be reasonable to grab subdirectories from $(obj-...)
and add $(foreach d,$(filtered-subdirs), $(d)/*.o). Or, completely
differently, have xen/common/Makefile just have an add-on
clean:: (and require this also for future other cases like this).

Jan

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

> On Dec 1, 2015, at 6:41 AM, Jan Beulich <JBeulich@suse.com> wrote:
> 
>>>> On 30.11.15 at 18:29, <jonathan.creekmore@gmail.com> wrote:
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -173,7 +173,7 @@ FORCE:
>> 
>> .PHONY: clean
>> clean:: $(addprefix _clean_, $(subdir-all))
>> -	rm -f *.o *~ core $(DEPS)
>> +	rm -f *.o *~ core $(DEPS) $(obj-y)
> 
> While for the moment it would do, using a minimalistic approach
> like this will make us touch this again the moment we gain files in
> subdirectories that can be built just optionally. At the very least
> I'd therefore suggest also adding $(obj-n) and $(obj-) here. It
> might even be reasonable to grab subdirectories from $(obj-...)
> and add $(foreach d,$(filtered-subdirs), $(d)/*.o). Or, completely
> differently, have xen/common/Makefile just have an add-on
> clean:: (and require this also for future other cases like this).

I am not sure why this would require us to touch it again if we gain
files in subdirectories that can be built optionally. If you are running
a clean using the same configuration as you built, then the “optional”
subdirectories would be in the subdir-all list, so that subdirectory
would be processed just like it is now. Adding in obj-n and obj- seems
to imply that, for the current options selected, you are concerned about
object files showing up that you explicitly stated are not to be built 
(since that is how they show up in the obj-n list). My understanding of
the build system is that the only object files built are in obj-y for each
subdirectory. 

Initially, I tried eliminating the *.o completely and just pulling in obj-y and
obj-bin-y to the clean rule to be more like the Linux kernel clean rules, 
but I did not immediately see how, with $FOO.init.o in the obj-bin-y list,
I was seeing $FOO.o objects left behind instead. Thus, I left the *.o to
clean those files up.

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

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jan Beulich]

>>> On 01.12.15 at 16:52, <jonathan.creekmore@gmail.com> wrote:

>> On Dec 1, 2015, at 6:41 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> 
>>>>> On 30.11.15 at 18:29, <jonathan.creekmore@gmail.com> wrote:
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -173,7 +173,7 @@ FORCE:
>>> 
>>> .PHONY: clean
>>> clean:: $(addprefix _clean_, $(subdir-all))
>>> -	rm -f *.o *~ core $(DEPS)
>>> +	rm -f *.o *~ core $(DEPS) $(obj-y)
>> 
>> While for the moment it would do, using a minimalistic approach
>> like this will make us touch this again the moment we gain files in
>> subdirectories that can be built just optionally. At the very least
>> I'd therefore suggest also adding $(obj-n) and $(obj-) here. It
>> might even be reasonable to grab subdirectories from $(obj-...)
>> and add $(foreach d,$(filtered-subdirs), $(d)/*.o). Or, completely
>> differently, have xen/common/Makefile just have an add-on
>> clean:: (and require this also for future other cases like this).
> 
> I am not sure why this would require us to touch it again if we gain
> files in subdirectories that can be built optionally. If you are running
> a clean using the same configuration as you built, then the “optional”
> subdirectories would be in the subdir-all list, so that subdirectory
> would be processed just like it is now.

The question isn't about things referenced through $(subdir-all) or
however it is called, but about files with path name conditionally
appearing in $(obj-y). Leaving aside the path name aspect, just
consider you do a build with kexec=y and later a clean with
kexec=n, which would (if the files were in subdirectories, or if
you indeed removed *.o from the clean rule) leave these around.

> Adding in obj-n and obj- seems
> to imply that, for the current options selected, you are concerned about
> object files showing up that you explicitly stated are not to be built 
> (since that is how they show up in the obj-n list). My understanding of
> the build system is that the only object files built are in obj-y for each
> subdirectory. 

For one build run, yes. But then you can (a) build individual object
files and (b) as mentioned above change configuration (implying
that you know what you're doing). Also you could, using the
example above, do a kexec=y build, then a kexec=n one, then
notice you needed to clean in between, so you then clean using
kexec=n and build again with that option, but cleaning again
would still leave the kexec files around.

And btw., we have a similar issue already when you switch
between arches (no cleaning happens cross-arch).

> Initially, I tried eliminating the *.o completely and just pulling in obj-y and
> obj-bin-y to the clean rule to be more like the Linux kernel clean rules, 
> but I did not immediately see how, with $FOO.init.o in the obj-bin-y list,
> I was seeing $FOO.o objects left behind instead. Thus, I left the *.o to
> clean those files up.

Because for $FOO.init.o the corresponding $FOO.o wouldn't appear
in $(obj-y) - quite obviously we don't want to link both objects. I.e.
you need to clear not just $(obj-y) but also $(obj-bin-y) and
$(patsubst %.init.o,%.o,$(obj-bin-y)).

Jan

Jan

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

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

> On Dec 1, 2015, at 10:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
> 
> For one build run, yes. But then you can (a) build individual object
> files and (b) as mentioned above change configuration (implying
> that you know what you're doing). Also you could, using the
> example above, do a kexec=y build, then a kexec=n one, then
> notice you needed to clean in between, so you then clean using
> kexec=n and build again with that option, but cleaning again
> would still leave the kexec files around.
> 
> And btw., we have a similar issue already when you switch
> between arches (no cleaning happens cross-arch).

OK, so you are working on a different assumption than I was. I was
treating the clean rule as needing to be run when you are wanting to
explicitly rebuild all object files needed for the current build configuration 
(i.e., only cleaning files that would be linked into the current hypervisor build).
It sounds like you are expecting the clean rule to clean out all object
files no matter whether they are part of the current build configuration
or not. 

Working on that assumption, it seems like running a:
    find . -name “*.o” -type f -delete 
from the xen/ directory would accomplish that and would be less
fragile than trying to grab various different variables and munge
them to try to grab all possible .o files specified by the system. Plus,
the find command would likely execute quicker. 

Does something like that seem acceptable?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jan Beulich]

>>> On 01.12.15 at 17:34, <jonathan.creekmore@gmail.com> wrote:
>> On Dec 1, 2015, at 10:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> 
>> For one build run, yes. But then you can (a) build individual object
>> files and (b) as mentioned above change configuration (implying
>> that you know what you're doing). Also you could, using the
>> example above, do a kexec=y build, then a kexec=n one, then
>> notice you needed to clean in between, so you then clean using
>> kexec=n and build again with that option, but cleaning again
>> would still leave the kexec files around.
>> 
>> And btw., we have a similar issue already when you switch
>> between arches (no cleaning happens cross-arch).
> 
> OK, so you are working on a different assumption than I was. I was
> treating the clean rule as needing to be run when you are wanting to
> explicitly rebuild all object files needed for the current build 
> configuration 
> (i.e., only cleaning files that would be linked into the current hypervisor 
> build).
> It sounds like you are expecting the clean rule to clean out all object
> files no matter whether they are part of the current build configuration
> or not. 
> 
> Working on that assumption, it seems like running a:
>     find . -name “*.o” -type f -delete 
> from the xen/ directory would accomplish that and would be less
> fragile than trying to grab various different variables and munge
> them to try to grab all possible .o files specified by the system. Plus,
> the find command would likely execute quicker. 
> 
> Does something like that seem acceptable?

I can't see an immediate reason why it would not be, as long
as it's clear that this won't eliminate the need to recurse into
the subdirectories. But I'd certainly recommend to wait for
other feedback (namely by other hypervisor maintainers)
before you go that route.

Also please note that -delete is not a standard primary, so
would need replacing.

Also the same global approach could then perhaps be used to
remove all the .*.d files.

Jan

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

Re: [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Ian Campbell]

On Tue, 2015-12-01 at 09:44 -0700, Jan Beulich wrote:
> > > > On 01.12.15 at 17:34, <jonathan.creekmore@gmail.com> wrote:
> > > On Dec 1, 2015, at 10:07 AM, Jan Beulich <JBeulich@suse.com> wrote:
> > > 
> > > For one build run, yes. But then you can (a) build individual object
> > > files and (b) as mentioned above change configuration (implying
> > > that you know what you're doing). Also you could, using the
> > > example above, do a kexec=y build, then a kexec=n one, then
> > > notice you needed to clean in between, so you then clean using
> > > kexec=n and build again with that option, but cleaning again
> > > would still leave the kexec files around.
> > > 
> > > And btw., we have a similar issue already when you switch
> > > between arches (no cleaning happens cross-arch).
> > 
> > OK, so you are working on a different assumption than I was. I was
> > treating the clean rule as needing to be run when you are wanting to
> > explicitly rebuild all object files needed for the current build 
> > configuration 
> > (i.e., only cleaning files that would be linked into the current
> > hypervisor 
> > build).
> > It sounds like you are expecting the clean rule to clean out all object
> > files no matter whether they are part of the current build
> > configuration
> > or not. 
> > 
> > Working on that assumption, it seems like running a:
> >     find . -name “*.o” -type f -delete 
> > from the xen/ directory would accomplish that and would be less
> > fragile than trying to grab various different variables and munge
> > them to try to grab all possible .o files specified by the system.
> > Plus,
> > the find command would likely execute quicker. 
> > 
> > Does something like that seem acceptable?
> 
> I can't see an immediate reason why it would not be, as long
> as it's clear that this won't eliminate the need to recurse into
> the subdirectories. But I'd certainly recommend to wait for
> other feedback (namely by other hypervisor maintainers)
> before you go that route.
> 
> Also please note that -delete is not a standard primary, so
> would need replacing.
> 
> Also the same global approach could then perhaps be used to
> remove all the .*.d files.

I can't think of a good reason not to do both of these.


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