Re: [Xen-devel] [PATCH 3/3] osstest: add FreeBSD Xen build job

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH 3/3] osstest: add FreeBSD Xen build job"):
> To both the FreeBSD and the xen-unstable flights.
> 
> This is the runvar difference of a xen-unstable flight:

Just to clarify my thinking:

> +    # Create a Xen build job that's going to use the output from the first
> +    # FreeBSD build job.

I think you do it like this so that on the freebsd branch, we use the
new being-tested version of freebsd, and see if it breaks the Xen
build.

> +    case "$xenbranch" in
> +    xen-unstable) build_on_freebsd=true;;
> +    *)            build_on_freebsd=false;;

I don't think this is right.  Why do we only want to do this with
xen-unstable ?  If the problem is that you need "new enough" xen, then
you need the version comparison case pattern used elsewhere, so that
each new branch of Xen has this turned on.

> +      if [ x$arch = xamd64 ] && [ x$build_on_freebsd = xtrue ] ; then
> +        job_create_build build-$arch$xsm_suffix-freebsd build-xen-freebsd    
> \
> +          arch=$arch                                                         
> \
> +          enable_xsm=$enable_xsm $livepatch_runvars                          
> \
> +          tree_qemuu=$TREE_QEMU_UPSTREAM                                     
> \
> +          tree_xen=$TREE_XEN                                                 
> \
> +          tree_seabios=$TREE_SEABIOS                                         
> \
> +          $RUNVARS $BUILD_RUNVARS $BUILD_XEN_RUNVARS $arch_runvars           
> \
> +          host_hostflags=arch-$arch,purpose-build                            
> \
> +          revision_xen=$REVISION_XEN                                         
> \
> +          revision_qemu=$REVISION_QEMU                                       
> \
> +          revision_qemuu=$REVISION_QEMU_UPSTREAM                             
> \
> +          revision_seabios=$REVISION_SEABIOS                                 
> \

These various xen build runvar settings seem to have mostly been
copied from create_build_jobs.  I think they should be abstracted into
a function.

Also of course you have two places where you create the job
build-xen-freebsd.  Can we please have a shell function for that to
avoid the duplication ?

> +proc run-job/build-xen-freebsd {} {
> +    run-ts . xen-build-clang ts-xen-build + host -- clang=y SEABIOSCC=gcc \
> +                                                    
> SEABIOSLD=/usr/local/bin/ld

This is quite ugly.  sg-run-job normally tries to be a bit more
abstract.  I'm not sure exactly what to suggest.

Maybe a ts-xen-build-clang containing these arguments ?  Of course for
a clang build on Linux /usr/local/bin/ld is wrong...

It's also very odd that /usr/local/bin/ld is the right thing here.
I'm not sure how the semantics of /usr/local ("locally installed")
relate to Seabios's need for (presumably) binutils ld ?  Is it
conventional on FreeBSD that /usr/local/bin/ld is binutils, or is that
some kind of happenstance ?

Thanks,
Ian.

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