| Message ID | 20221230003848.3241-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CI: Fixes/cleanup in preparation for RISCV | expand |
Hi Andrew, On 30/12/2022 01:38, Andrew Cooper wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > 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@citrix.com> I'd prefer to keep using "artifacts" and not "artefacts" as the former is what GitLab uses and what we use in build/test.yaml. Apart from that: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
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@citrix.com> > --- > CC: Doug Goldstein <cardoe@cardoe.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Anthony PERARD <anthony.perard@citrix.com> > CC: Michal Orzel <michal.orzel@amd.com> > CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > 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? If you are OK with this change you can do it on commit Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
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@citrix.com> >> --- >> CC: Doug Goldstein <cardoe@cardoe.com> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Anthony PERARD <anthony.perard@citrix.com> >> CC: Michal Orzel <michal.orzel@amd.com> >> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> >> --- >> 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. ~Andrew
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@citrix.com> > >> --- > >> CC: Doug Goldstein <cardoe@cardoe.com> > >> CC: Stefano Stabellini <sstabellini@kernel.org> > >> CC: Anthony PERARD <anthony.perard@citrix.com> > >> CC: Michal Orzel <michal.orzel@amd.com> > >> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> > >> --- > >> 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
On 02/01/2023 2:00 pm, Michal Orzel wrote: > Hi Andrew, > > On 30/12/2022 01:38, Andrew Cooper wrote: >> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >> >> >> 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@citrix.com> > I'd prefer to keep using "artifacts" and not "artefacts" as the former is what GitLab uses > and what we use in build/test.yaml. Xen is written in British English. We're forced to compromise for external dependencies, but xen.git is mostly British not American. ~Andrew
On 04/01/2023 13:12, Andrew Cooper wrote: > > > On 02/01/2023 2:00 pm, Michal Orzel wrote: >> Hi Andrew, >> >> On 30/12/2022 01:38, Andrew Cooper wrote: >>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >>> >>> >>> 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@citrix.com> >> I'd prefer to keep using "artifacts" and not "artefacts" as the former is what GitLab uses >> and what we use in build/test.yaml. > > Xen is written in British English. We're forced to compromise for > external dependencies, but xen.git is mostly British not American. True, but from the consistency perspective and easy grepping, it is beneficial to stick to what a subsystem uses by default. > > ~Andrew ~Michal
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 fi
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@citrix.com> --- CC: Doug Goldstein <cardoe@cardoe.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Anthony PERARD <anthony.perard@citrix.com> CC: Michal Orzel <michal.orzel@amd.com> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- automation/scripts/build | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)