Re: [Xen-devel] [PATCH v6 19/20] osstest: save/retrieve the last successfully tested FreeBSD build

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v6 19/20] osstest: save/retrieve the last 
successfully tested FreeBSD build"):
> And use it in order to install the hosts for the next FreeBSD flight.

Oh.  Right, here is this.

> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Changes since v5:
>  - New in this version.
> ---
>  cr-daily-branch | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/cr-daily-branch b/cr-daily-branch
> index af17ad20..249cd33d 100755
> --- a/cr-daily-branch
> +++ b/cr-daily-branch
> @@ -71,6 +71,15 @@ fetch_version () {
>  case $branch in
>      distros-*|examine)
>       treeurl=none;;
> +    freebsd-*)
> +        # NB: only needed for the freebsd branches ATM, since other
> +        # branches still don't have jobs that consume freebsd builds.
> +        if [ "x$FREEBSD_AMD64_BUILDJOB" = "x" ]; then
> +            flight_job=`./mg-anoint retrieve "freebsd build $branch amd64"`

This is rather ugly.  At the very least, can we have a loop over
architectures ?

> +            export FREEBSD_AMD64_BUILDJOB=${flight_job/ /.}
> +        fi
> +        # fallthrough
> +        ;&

Can you do this without fallthrough please ?  That is, just make a
seperate case statement.  And why is this so early, and added to the
setting of treeurl ?

I think it would probaby be best to do this after *_REVISION are
calculated, just before ap-fetch-version-baseline-late.  Would that
work ?

>  push=false
> -if grep '^tolerable$' $mrof >/dev/null 2>&1; then push=$wantpush; fi
> +if grep '^tolerable$' $mrof >/dev/null 2>&1; then
> +    push=$wantpush;
> +    case "$branch" in
> +    freebsd-*)
> +        # Save the output of successful FreeBSD build jobs to be re-used.
> +        # NB: hardcode arch to amd64 since that's all osstest covers ATM.
> +        ./mg-anoint anoint "freebsd build $branch amd64" \
> +                           $flight build-freebsd-amd64
> +        ;;
> +    esac
> +fi

No, please don't change this.  Instead I think this should be done in
the section where we do the actual push ?  Or maybe we need another
tracking variable `anoint'.  We need to think about the following
cases:

  OSSTEST_PUSH=false    probably want not to anoint
  $branch.force         I think this should force an anoint
  OLD_REVISON=none      Now forces test, should force anoint

What about supporting

  OSSTEST_ANOINT=false  ?

I think the right approach is probably to add code after $wantpush and
$push are computed, which computes $anoint, in a similar way.

Also I would like you to discuss explicitly (in a comment or commit
message) about whether push or anoint should come first.  If push
comes first then we can end up pushed but not anointed; and, vice
versa.  What are the recovery arrangements from such a failure ?

Thanks,
Ian.

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