Re: [PATCH v2 6/7] build system: stubdom targets now depends on mini-os target

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-02-24 at 17:12 +0000, Wei Liu wrote:
> On Tue, Feb 24, 2015 at 05:01:26PM +0000, Ian Campbell wrote:
> > On Tue, 2015-02-24 at 16:52 +0000, Wei Liu wrote:
> > > On Tue, Feb 24, 2015 at 04:33:17PM +0000, Ian Campbell wrote:
> > > > On Fri, 2015-02-20 at 11:17 +0000, Wei Liu wrote:
> > > > > @@ -161,7 +163,7 @@ clean-tools:
> > > > >  	$(MAKE) -C tools clean
> > > > >  
> > > > >  .PHONY: clean-stubdom
> > > > > -clean-stubdom:
> > > > > +clean-stubdom: mini-os-dir
> > > > >  	$(MAKE) -C stubdom crossclean
> > > > >  ifeq (x86_64,$(XEN_TARGET_ARCH))
> > > > >  	XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom crossclean
> > > > > @@ -187,11 +189,12 @@ distclean-tools:
> > > > >  	$(MAKE) -C tools distclean
> > > > >  
> > > > >  .PHONY: distclean-stubdom
> > > > > -distclean-stubdom:
> > > > > +distclean-stubdom: mini-os-dir
> > > > 
> > > > These two are a bit odd, since they will force a clone in order to clean
> > > > (and in the distclean case immediately discard again).
> > > > 
> > > 
> > > That's because stubdom's distclean is quite broken, it just won't work
> > > without mini-os.
> > 
> > If the mini-os dir is not present then I don't think there is any need
> > to distclean the stubdom, is there? How would anything be present?
> > 
> 
> If user builds stubdom then deletes mini-os then wants to clean
> studom.

I think users who randomly delete things can be expected to cope with
randomly cleaning other stuff too ;-)

> 
> If we don't have that dependence, we just print
> 
>   Please run `make mini-os-dir' in top-level directory
> 
> due to a check in stubdom's Makefile. I think this is acceptable too.

That's better than cloning then removing it everytime someone runs
distclean, for sure.

I'd prefer something saying "not cleaning because" but I suppose the
above is from a generic catch-all rule not a *clean specific one.

> > > > The way we handle this with e.g. qemu is to have
> > > > subdir-clean-qemu-xen-traditional-dir:
> > > >         set -e; if test -d qemu-xen-traditional-dir/.; then \
> > > >                 $(MAKE) -C qemu-xen-traditional-dir clean; \
> > > >         fi
> > > > 
> > > > so I think you want a pair of {clean,distclean}-mini-os-dir rules which
> > > > recurse iff the dir exists.
> > > > 
> > > 
> > > No, we don't actually need to enter mini-os dir and make clean /
> > > distclean when doing clean and distclean of stubdom.
> > 
> > Didn't you just contradict what you said further above?
> > 
> 
> I was thinking all those objects are not placed inside min-os's dir so
> there is nothing to clean inside mini-os's directory
> 
> However, stubdom does have
>    $(MAKE) DESTDIR= -C $(MINI_OS) clean
> 
> So there is no need for a separate subdir-clean-mini-os-dir.

I see. What I was trying to say (and did badly, and then misunderstood
your response) was that the stubdom *clean should check for mini-os-dir
(which is different to the qemu case, I admit). 

i.e. what I really was thinking of was:

subdir-clean-stubdom:
        set -e; if test -d mini-os-dir/.; then \
                $(MAKE) -C stubdom clean; \
	else
		echo "Not running clean in stubdom, no mini-os-dir.";\
                echo "run `make mini-os-dir' in top-level before cleaning stubdom" ;\
        fi

(similar for distclean)

> We can also delete that line. But I would avoid touching stubdom's
> Makefile if not necessary.
> 
> Wei.