Re: [PATCH for 4.6] build: fix tarball stubdom build

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Aug 28, 2015 at 03:49:06AM -0600, Jan Beulich wrote:
> >>> On 28.08.15 at 10:47, <wei.liu2@citrix.com> wrote:
> > On Fri, Aug 28, 2015 at 12:41:15AM -0600, Jan Beulich wrote:
> >> >>> On 27.08.15 at 18:24, <wei.liu2@citrix.com> wrote:
> >> > On Thu, Aug 27, 2015 at 10:05:56AM -0600, Jan Beulich wrote:
> >> >> >>> On 27.08.15 at 17:54, <wei.liu2@citrix.com> wrote:
> >> >> > When we create a source code tarball, mini-os is extracted to
> >> >> > extras/mini-os directory. When building a source code tarball, we
> >> >> > shouldn't clone mini-os again.
> >> >> > 
> >> >> > Only clone mini-os when that directory doesn't exist. This fixes tarball
> >> >> > build and doesn't affect non-tarball build.
> >> >> > 
> >> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> >> > Cc: Ian Campbell <ian.campbell@citrix.com>
> >> >> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >> >> > ---
> >> >> >  Makefile | 10 ++++++----
> >> >> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >> >> > 
> >> >> > diff --git a/Makefile b/Makefile
> >> >> > index e8a75ff..ba0df70 100644
> >> >> > --- a/Makefile
> >> >> > +++ b/Makefile
> >> >> > @@ -19,10 +19,12 @@ include Config.mk
> >> >> >  
> >> >> >  .PHONY: mini-os-dir
> >> >> >  mini-os-dir:
> >> >> > -	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh \
> >> >> > -		$(MINIOS_UPSTREAM_URL) \
> >> >> > -		$(MINIOS_UPSTREAM_REVISION) \
> >> >> > -		$(XEN_ROOT)/extras/mini-os
> >> >> > +	if [ ! -d $(XEN_ROOT)/extras/mini-os ]; then \
> >> >> > +		GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh \
> >> >> > +			$(MINIOS_UPSTREAM_URL) \
> >> >> > +			$(MINIOS_UPSTREAM_REVISION) \
> >> >> > +			$(XEN_ROOT)/extras/mini-os ; \
> >> >> > +	fi
> >> >> 
> >> >> Wouldn't his better be done (avoiding the need for the shell
> >> >> conditional) by simply renaming the make target from
> >> >> mini-os-dir to extras/mini-os (and dropping the .PHONY)?
> >> > 
> >> > All targets for external trees follow some conventions defined in
> >> > tools/Makefile. Although I didn't strictly follow the same rules in
> >> > mini-os's case, I don't want it to deviate from the original rules too
> >> > much.
> >> 
> >> That's not true: ${target}-dir are actual directories (and hence
> >> not phony targets) in tools/Makefile (they might also be symlinks,
> >> but that's not of interest for the purposes here). Which is
> >> precisely what I'm asking to be done here too, just that I'd see
> >> the existing naming (extra/mini-os) preserved rather than
> >> changed to extra/mini-os-dir).
> > 
> > Yeah. That's why I said "I didn't strictly follow the same rules".  To
> > strictly follow rules, mini-os source code should have been in
> > mini-os-dir (a symlink to mini-os-dir-remote or real directory
> > containing source code).  Target mini-os-dir should have been
> > mini-os-dir-find. That would cause less confusion. But because stubdom
> > build system is very fragile, I didn't want to touch its Makefile too
> > much so the name mini-os is preserved. And because mini-os doesn't
> > support out-of-tree build, mini-os-dir again was not necessary.
> > 
> > I can submit another patch to make mini-os-dir mini-os-dir-find?
> 
> Not sure that's worth it, or even consistent with the other cases.
> 
> > Other
> > than that trying to wire up everything to consistently follow the rules
> > is too much effort for too little gain.
> 
> I agree. But you don't really comment on the suggestion to make
> the build target a real one, allowing the shell conditional to be
> dropped (again, since Ian already applied your patch).
> 

Didn't mean to ignore that.

Yes. Using real target is one way of doing it. Feel free to submit patch
to change to that after 4.7 window opens. I've also added that to my
list but its priority is very low.

Wei.

> Jan