Re: [Xen-devel] [PATCH for 4.6] build: fix tarball stubdom build

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.08.15 at 10:47, <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Aug 28, 2015 at 12:41:15AM -0600, Jan Beulich wrote:
>> >>> On 27.08.15 at 18:24, <wei.liu2@xxxxxxxxxx> wrote:
>> > On Thu, Aug 27, 2015 at 10:05:56AM -0600, Jan Beulich wrote:
>> >> >>> On 27.08.15 at 17:54, <wei.liu2@xxxxxxxxxx> 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@xxxxxxxxxx>
>> >> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> >> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> >> > ---
>> >> >  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).

Jan


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