Re: [PATCH 1/2] automation: preserve built xen.efi

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 2 Oct 2024, Marek Marczykowski-Górecki wrote:
> On Wed, Oct 02, 2024 at 09:42:13PM +0100, Andrew Cooper wrote:
> > On 02/10/2024 1:42 pm, Marek Marczykowski-Górecki wrote:
> > > It will be useful for further tests.
> > >
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > >  automation/scripts/build | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/automation/scripts/build b/automation/scripts/build
> > > index b3c71fb6fb60..4cd41cb2c471 100755
> > > --- a/automation/scripts/build
> > > +++ b/automation/scripts/build
> > > @@ -47,6 +47,7 @@ if [[ "${CPPCHECK}" == "y" ]] && [[ 
> > > "${HYPERVISOR_ONLY}" == "y" ]]; then
> > >  
> > >      # Preserve artefacts
> > >      cp xen/xen binaries/xen
> > > +    if [[ -f xen/xen.efi ]]; then cp xen/xen.efi binaries/xen.efi; fi
> > 
> > Wouldn't
> > 
> >     # Preserve xen and optionally xen.efi
> >     cp -f xen/xen xen/xen.efi binaries/
> > 
> > do this in a more concise way?
> 
> I don't think so, `cp -f` still fails if the source cannot be found.

I think it would have to be something like:

cp -f xen/xen xen/xen.efi binaries/ || true


> > Alternatively, what about this:
> > 
> > diff --git a/automation/scripts/build b/automation/scripts/build
> > index b3c71fb6fb60..14815ea7ad9c 100755
> > --- a/automation/scripts/build
> > +++ b/automation/scripts/build
> > @@ -41,6 +41,15 @@ cp xen/.config xen-config
> >  # Directory for the artefacts to be dumped into
> >  mkdir -p binaries
> >  
> > +collect_xen_artefacts ()
> > +{
> > +    for A in xen/xen xen/xen.efi; do
> > +        if [[ -f $A ]]; then
> > +            cp $A binaries/
> > +        fi
> > +    done
> > +}
> > +
> >  if [[ "${CPPCHECK}" == "y" ]] && [[ "${HYPERVISOR_ONLY}" == "y" ]]; then
> >      # Cppcheck analysis invokes Xen-only build
> >      xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra --
> > -j$(nproc)
> > @@ -53,7 +62,7 @@ elif [[ "${HYPERVISOR_ONLY}" == "y" ]]; then
> >      make -j$(nproc) xen
> >  
> >      # Preserve artefacts
> > -    cp xen/xen binaries/xen
> > +    collect_xen_artefacts
> >  else
> >      # Full build.  Figure out our ./configure options
> >      cfgargs=()
> > 
> > so we don't triplicate the handling?
> 
> That may be a better idea indeed.

collect_xen_artefacts is also a good option, perhaps even better. A
couple of minor NITs:

function collect_xen_artefacts()
{
    local f
    for f in xen/xen xen/xen.efi; do
        if [[ -f $f ]]; then
            cp $f binaries/
        fi
    done
}