Re: [PATCH 2/6] CI: Remove guesswork about which artefacts to preserve

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 4 Jan 2023, Andrew Cooper wrote:
> On 04/01/2023 1:10 am, Stefano Stabellini wrote:
> > On Fri, 30 Dec 2022, Andrew Cooper wrote:
> >> Preserve the artefacts based on the `make` rune we actually ran, rather 
> >> than
> >> guesswork about which rune we would have run based on other settings.
> >>
> >> Note that the ARM qemu smoke tests depend on finding binaries/xen even from
> >> full builds.  Also that the Jessie-32 containers build tools but not Xen.
> >>
> >> This means the x86_32 builds now store relevant artefacts.  No change in 
> >> other
> >> configurations.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> ---
> >> CC: Doug Goldstein <cardoe@xxxxxxxxxx>
> >> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >> CC: Michal Orzel <michal.orzel@xxxxxxx>
> >> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> >> ---
> >>  automation/scripts/build | 22 ++++++++++++++--------
> >>  1 file changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/automation/scripts/build b/automation/scripts/build
> >> index 5dafa72ba540..8dee1cbbc251 100755
> >> --- a/automation/scripts/build
> >> +++ b/automation/scripts/build
> >> @@ -70,18 +70,24 @@ if [[ "${CC}" == "gcc" && `cc-ver` -lt 0x040600 ]]; 
> >> then
> >>      cfgargs+=("--with-system-seabios=/bin/false")
> >>  fi
> >>  
> >> +# Directory for the artefacts to be dumped into
> >> +mkdir binaries
> >> +
> >>  if [[ "${hypervisor_only}" == "y" ]]; then
> >> +    # Xen-only build
> >>      make -j$(nproc) xen
> >> +
> >> +    # Preserve artefacts
> >> +    cp xen/xen binaries/xen
> >>  else
> >> +    # Full build
> >>      ./configure "${cfgargs[@]}"
> >>      make -j$(nproc) dist
> >> -fi
> >>  
> >> -# Extract artifacts to avoid getting rewritten by customised builds
> >> -mkdir binaries
> >> -if [[ "${XEN_TARGET_ARCH}" != "x86_32" ]]; then
> >> -    cp xen/xen binaries/xen
> >> -    if [[ "${hypervisor_only}" != "y" ]]; then
> >> -        cp -r dist binaries/
> >> -    fi
> >> +    # Preserve artefacts
> >> +    # Note: Some smoke tests depending on finding binaries/xen on a full 
> >> build
> >> +    # even though dist/ contains everything, while some containers don't 
> >> even
> >> +    # build Xen
> >> +    cp -r dist binaries/
> >> +    if [[ -f xen/xen ]] ; then cp xen/xen binaries/xen; fi
> > why the "if" ?
> >
> > You could just:
> >
> > cp xen/xen binaries/xen
> >
> > unconditionally?
> 
> No - the script is `set -e` and xen/xen doesn't exist in the Jessie32
> containers.
> 
> That's why the old logic had an "if not x86_32" guard (except it also
> guarded the regular dist -> binaries/ copy which is problematic).
> 
> At a guess, the other working option would be `cp xen/xen binaries/xen || :`
> 
> I don't much care which of these two gets used, but pretty much anything
> else results in a failed test.

I didn't realize that. I think your version is this patch is better,
keep it as is