Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 6/6] osstest: use a locally 
built pkg repository for FreeBSD"):
> On Thu, May 23, 2019 at 12:06:29PM +0100, Ian Jackson wrote:
> > I realise this is a bit late to be saying this, but had you
> > considered making the packages build a different step in the same
> > job ?  That might make a lot of this go away...
> 
> Do you mean to build the packages in build-prep instead of relying on
> having a custom binary repository?

No.  Maybe I am confused.  I thought your usual flight was
  1  install anointed freebsd
  2  build this freebsd
  3+ build this package repo
  4  install this freebsd (from step 2)
  5  rebuild this freebsd (for testing that the build didn't break)
  6  rebuild this package repo (")
  7+ install this package repoo
  8+ build xen

My question is why 2/3 and 5/6 are different jobs.  If you made 2+3 a
single job (with 2 and 3 being separate steps) then there would only
need to be a single anointment.

> IIRC the package building job takes a non-trivial amount of time (2-3h
> IIRC), because it has to build gcc (for SeaBIOS) and python, perl...

You could make the package building job optional if you only want to
do it some of the time.

> > > +     # refkey: freebsd          job: build-<arch>-freebsd
> > > +     # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> > > +            anoint="$anoint \"$anointed\" $flight \
> > > +                    build-$freebsd_arch-$freebsd_name"
> > 
> > Maybe use an array variable for anount, and then you can avoid the
> > shell \" quoting.
> 
> Please bear with me, but can you elaborate on this?

Roughly,

  anoint=()
  ....
     anoint+=("$anointed" $flight build-..)
  ...
  ./mg-anoint "${anoint[@]}"

Note that the \" \" construct has gone, because there is now no
additional layer of shell dequoting.

> > There seems like a lot of repetition here.  For example, FREEBSD_DIST
> > overrides FreeBSDDist but /$arch is appended in two places.  Maybe
> > ${FREEBSD_DIST- ... something ... } would be better ?
> 
> OK, let me try to remove some of the duplication here.

Thanks.

Another suggestion I mentioned IRL which I wanted to write down was:
maybe have mg-anoint have a reporting mode where it prints something
suitable for shell `eval'.

> > This feels very similar to the code above, although it lacks the
> > special handling for the version.
> 
> Maybe I can see about factoring some of this into a helper, but there
> are slight differences in both if branches that I'm not sure can be
> factored out.

Mmmm.

> Why I don't start by fixing the repetition of:
> freebsd_runvars="$freebsd_runvars \... and we take it from there?

Sure, let's see what you come up with.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel