[libvirt PATCH v2 00/35] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions

since v2 [1]: - moved the custom meson.build directory check before running setup to individual callers for better robustness, but actually let meson deal with an existing setup directory most of the time - drops the ci/Makefile - only set our verbose env variables when running without a TTY - switched the bright yellow (93m) color to green (32m) - added KeyboardInterrupt handling in the ci/helper Python code The changes above require the following lcitool MR to be merged first as they're built on top of it [2] [1] https://listman.redhat.com/archives/libvir-list/2023-August/241471.html [2] https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/431 Erik Skultety (35): ci: build.sh: Add variables from .gitlab-ci.yml ci: build.sh: Add GIT_ROOT env helper variable ci: build.sh: Don't mention that MESON_ARGS are available via CLI ci: build.sh: Add a wrapper function executing 'shell' commands ci: build.sh: Add a wrapper function over meson's setup ci: build.sh: Add a wrapper function over the 'build' job ci: build.sh: Add a helper function to create the dist tarball ci: build.sh: Add a wrapper function over the 'test' job ci: build.sh: Add a wrapper function over the 'codestyle' job ci: build.sh: Add a wrapper function over the 'potfile' job ci: build.sh: Add a wrapper function over the 'rpmbuild' job ci: build.sh: Add a wrapper function over the 'website' job ci: build.sh: Drop changing working directory to CI_CONT_DIR ci: build.sh: Drop direct invocation of meson/ninja commands ci: build.sh: Drop MESON_ARGS definition from global level ci: Rename build.sh -> jobs.sh .gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection .gitlab-ci.yml: Convert the native build job to the build.sh usage .gitlab-ci.yml: Convert the cross build job to the build.sh usage .gitlab-ci.yml: Convert the website build job to the build.sh usage .gitlab-ci.yml: Convert the codestyle job to the build.sh usage .gitlab-ci.yml: Convert the potfile job to the build.sh usage ci: helper: Drop _lcitool_get_targets method ci: helper: Don't make ':' literal a static part of the image tag ci: helper: Add --lcitool-path CLI option ci: helper: Add a required_deps higher order helper/decorator ci: helper: Add Python code hangling git clones ci: helper: Add a helper to create a local repo clone Pythonic way ci: helper: Rework _lcitool_run method logic ci: helper: Add an action to run the container workload via lcitool ci: helper: Add a job argparse subparser ci: helper: Drop original actions ci: helper: Drop the --meson-args/--ninja-args CLI options ci: helper: Drop the _make_run method ci: Drop the now unused Makefile .gitlab-ci.yml | 47 +++++----- ci/Makefile | 245 ------------------------------------------------- ci/build.sh | 23 ----- ci/helper | 181 +++++++++++++++++++++--------------- ci/jobs.sh | 79 ++++++++++++++++ 5 files changed, 211 insertions(+), 364 deletions(-) delete mode 100644 ci/Makefile delete mode 100644 ci/build.sh create mode 100644 ci/jobs.sh -- 2.41.0

These are common variables we wish to use in containerized environments both in GitLab and locally. Having these defined in a single place rather than twice is highly preferable. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index d5ed8ad104..5a9298c4b4 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -2,7 +2,17 @@ cd "$CI_CONT_SRCDIR" -export VIR_TEST_DEBUG=1 +export CCACHE_BASEDIR="$(pwd)" +export CCACHE_DIR="$CCACHE_BASEDIR/ccache" +export CCACHE_MAXSIZE="500M" +export PATH="$CCACHE_WRAPPERSDIR:$PATH" + +# Enable these conditionally since their best use case is during +# non-interactive workloads without having a Shell +if ! [ -t 1 ]; then + export VIR_TEST_VERBOSE="1" + export VIR_TEST_DEBUG="1" +fi # $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:02PM +0200, Erik Skultety wrote:
These are common variables we wish to use in containerized environments both in GitLab and locally. Having these defined in a single place rather than twice is highly preferable.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>:
A script went wrong here and other following patches. I'll assume you'll fix it, so will ack stuff regardless
--- ci/build.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/ci/build.sh b/ci/build.sh index d5ed8ad104..5a9298c4b4 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -2,7 +2,17 @@
cd "$CI_CONT_SRCDIR"
-export VIR_TEST_DEBUG=1 +export CCACHE_BASEDIR="$(pwd)" +export CCACHE_DIR="$CCACHE_BASEDIR/ccache" +export CCACHE_MAXSIZE="500M" +export PATH="$CCACHE_WRAPPERSDIR:$PATH" + +# Enable these conditionally since their best use case is during +# non-interactive workloads without having a Shell +if ! [ -t 1 ]; then + export VIR_TEST_VERBOSE="1" + export VIR_TEST_DEBUG="1" +fi
# $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Sep 11, 2023 at 03:02:15PM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 11, 2023 at 03:43:02PM +0200, Erik Skultety wrote:
These are common variables we wish to use in containerized environments both in GitLab and locally. Having these defined in a single place rather than twice is highly preferable.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>:
A script went wrong here and other following patches. I'll assume you'll fix it, so will ack stuff regardless
Sigh, must have run the same borked git command line in 2 separate terminals, sure, consider this fixed. Erik

We'll use this one in many of the job functions future patches will introduce, it's a neat shortcut to avoid using relative paths. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 5a9298c4b4..0549b01ca0 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -14,6 +14,8 @@ if ! [ -t 1 ]; then export VIR_TEST_DEBUG="1" fi +GIT_ROOT="$(git rev-parse --show-toplevel)" + # $MESON_OPTS is an env that can optionally be set in the container, # populated at build time from the Dockerfile. A typical use case would # be to pass options to trigger cross-compilation -- 2.41.0

Previous patches have removed the code that allowed injecting arbitrary meson arguments, same for ninja args. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index 0549b01ca0..7cf07ba5a8 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -21,13 +21,7 @@ GIT_ROOT="$(git rev-parse --show-toplevel)" # be to pass options to trigger cross-compilation # # $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's -# populated either from a GitLab's job configuration or from command line as -# `$ helper build --meson-args='-Dopt1 -Dopt2'` when run in a local -# containerized environment -# -# The contents of $MESON_ARGS (defined locally) should take precedence over -# those of $MESON_OPTS (defined when the container was built), so they're -# passed to meson after them +# populated from a GitLab's job configuration meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) -- 2.41.0

This would normally be not needed at all, but the problem here is the Shell-in-YAML which GitLab interprets. It outputs every command that appears as a line in the 'script' segment in a color-coded fashion for easy identification of problems. Well, that useful feature is lost when there's indirection and one script calls into another in which case it would only output the respective script name which would make failure investigation harder. This simple helper tackles that by echoing the command to be run by any script/function with a color escape sequence so that we don't lose track of the *actual* shell commands being run as part of the GitLab job pipelines. An example of what the output then might look like: [RUN COMMAND]: 'meson compile -C build install-web' Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 7cf07ba5a8..5883542b45 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -27,3 +27,8 @@ meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) ninja -C build $NINJA_ARGS + +run_cmd() { + printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*" + $@ +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:05PM +0200, Erik Skultety wrote:
This would normally be not needed at all, but the problem here is the Shell-in-YAML which GitLab interprets. It outputs every command that appears as a line in the 'script' segment in a color-coded fashion for easy identification of problems. Well, that useful feature is lost when there's indirection and one script calls into another in which case it would only output the respective script name which would make failure investigation harder. This simple helper tackles that by echoing the command to be run by any script/function with a color escape sequence so that we don't lose track of the *actual* shell commands being run as part of the GitLab job pipelines. An example of what the output then might look like: [RUN COMMAND]: 'meson compile -C build install-web'
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

The reason for this wrapper is that all job functions introduced in future patches will refer to this one instead of open-coding the same 'meson setup' invocation N times. It also prevents 'setup' to be called multiple times as some future job functions might actually do just that in a transitive manner. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 5883542b45..477ccbc7d1 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -32,3 +32,8 @@ run_cmd() { printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*" $@ } + +run_meson_setup() { + run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS || \ + (cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1) +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:06PM +0200, Erik Skultety wrote:
The reason for this wrapper is that all job functions introduced in future patches will refer to this one instead of open-coding the same 'meson setup' invocation N times. It also prevents 'setup' to be called multiple times as some future job functions might actually do just that in a transitive manner.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 477ccbc7d1..f908bbc5d4 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -37,3 +37,8 @@ run_meson_setup() { run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1) } + +run_build() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup + run_cmd meson compile -C build $BUILD_ARGS +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:07PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh index 477ccbc7d1..f908bbc5d4 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -37,3 +37,8 @@ run_meson_setup() { run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1) } + +run_build() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup
test -f
+ run_cmd meson compile -C build $BUILD_ARGS +}
With that test change Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper function does not correspond to a particular GitLab job, it just logically separates the necessary step of creating a dist tarball from the RPM build job that takes over. One notable change here is the need to update git's file index which causes issues in local container executions which rely on a shallow copy of the libvirt repo created as: $ git clone --local Even if all changes have been committed, git often complained otherwise. Updating the index in a GitLab environment is a NOP. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index f908bbc5d4..ab56c5e5eb 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -42,3 +42,13 @@ run_build() { test -d $GIT_ROOT/build/build.ninja || run_meson_setup run_cmd meson compile -C build $BUILD_ARGS } + +run_dist() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup + + # dist is unhappy in local container environment complaining about + # uncommitted changes in the repo which is often not the case - refreshing + # git's index solves the problem + git update-index --refresh + run_cmd meson dist -C build --no-tests +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:08PM +0200, Erik Skultety wrote:
This helper function does not correspond to a particular GitLab job, it just logically separates the necessary step of creating a dist tarball from the RPM build job that takes over. One notable change here is the need to update git's file index which causes issues in local container executions which rely on a shallow copy of the libvirt repo created as:
$ git clone --local
Even if all changes have been committed, git often complained otherwise. Updating the index in a GitLab environment is a NOP.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh index f908bbc5d4..ab56c5e5eb 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -42,3 +42,13 @@ run_build() { test -d $GIT_ROOT/build/build.ninja || run_meson_setup run_cmd meson compile -C build $BUILD_ARGS } + +run_dist() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup
test -f
+ + # dist is unhappy in local container environment complaining about + # uncommitted changes in the repo which is often not the case - refreshing + # git's index solves the problem + git update-index --refresh + run_cmd meson dist -C build --no-tests +}
With the test fix Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index ab56c5e5eb..29b6a3306d 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -52,3 +52,8 @@ run_dist() { git update-index --refresh run_cmd meson dist -C build --no-tests } + +run_test() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup + run_cmd meson test -C build $TEST_ARGS +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:09PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh index ab56c5e5eb..29b6a3306d 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -52,3 +52,8 @@ run_dist() { git update-index --refresh run_cmd meson dist -C build --no-tests } + +run_test() { + test -d $GIT_ROOT/build/build.ninja || run_meson_setup
test -f
+ run_cmd meson test -C build $TEST_ARGS +}
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 29b6a3306d..e6c3225691 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -57,3 +57,11 @@ run_test() { test -d $GIT_ROOT/build/build.ninja || run_meson_setup run_cmd meson test -C build $TEST_ARGS } + +run_codestyle() { + BUILD_ARGS="libvirt-pot-dep" + TEST_ARGS="--suite syntax-check --no-rebuild --print-errorlogs" + + run_build + run_test +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:10PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 8 ++++++++ 1 file changed, 8 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. There's one notable difference such that we pass '-j1' to the meson compile command otherwise we'd have to execute the 'run_build' function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets in a serial manner. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index e6c3225691..d6361f3ade 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -65,3 +65,14 @@ run_codestyle() { run_build run_test } + +run_potfile() { + # since meson would run jobs for each of the following target in parallel, + # we'd have dependency issues such that one target might depend on a + # generated file which hasn't been generated yet by the other target, hence + # we limit potfile job to a single build job (luckily potfile build has + # negligible performance impact) + BUILD_ARGS="-j1 libvirt-pot-dep libvirt-pot" + + run_build +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:11PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart. There's one notable difference such that we pass '-j1' to the meson compile command otherwise we'd have to execute the 'run_build' function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets in a serial manner.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index d6361f3ade..c558b4c9ca 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -76,3 +76,12 @@ run_potfile() { run_build } + +run_rpmbuild() { + run_dist + run_cmd rpmbuild \ + --clean \ + --nodeps \ + --define '_without_mingw 1' \ + -ta build/meson-dist/libvirt-*.tar.xz +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:12PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This helper is a shell function transcript of its original GitLab CI counterpart. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index c558b4c9ca..8e1619d483 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -85,3 +85,10 @@ run_rpmbuild() { --define '_without_mingw 1' \ -ta build/meson-dist/libvirt-*.tar.xz } + +run_website_build() { + export DESTDIR="${GIT_ROOT}/install" + BUILD_ARGS="install-web" + + run_build +} -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:13PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI counterpart.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Firstly, this would mangle with "sourcing" this file in either execution environment later down the road. Secondly, we won't need this as future ci/helper patches will generate a throwaway script that will take care of a correct execution of a build job in a similar fashion as if the job ran in a GitLab environment. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index 8e1619d483..fd326dad8d 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -1,7 +1,5 @@ #!/bin/sh -cd "$CI_CONT_SRCDIR" - export CCACHE_BASEDIR="$(pwd)" export CCACHE_DIR="$CCACHE_BASEDIR/ccache" export CCACHE_MAXSIZE="500M" -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:14PM +0200, Erik Skultety wrote:
Firstly, this would mangle with "sourcing" this file in either execution environment later down the road. Secondly, we won't need this as future ci/helper patches will generate a throwaway script that will take care of a correct execution of a build job in a similar fashion as if the job ran in a GitLab environment.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

We've moved all invocations to the respective helper function which we'll execute both from gitlab CI jobs and local environments so we don't need to have them on the global level as it would also not work with "sourcing" this file to populate the environment with function definitions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index fd326dad8d..ac649ed9a9 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -21,10 +21,7 @@ GIT_ROOT="$(git rev-parse --show-toplevel)" # $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's # populated from a GitLab's job configuration -meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ -(cat build/meson-logs/meson-log.txt && exit 1) - -ninja -C build $NINJA_ARGS +MESON_ARGS="$MESON_ARGS $MESON_OPTS" run_cmd() { printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*" -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:15PM +0200, Erik Skultety wrote:
We've moved all invocations to the respective helper function which we'll execute both from gitlab CI jobs and local environments so we don't need to have them on the global level as it would also not work with "sourcing" this file to populate the environment with function definitions.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/build.sh | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index ac649ed9a9..96ee289c4f 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -13,16 +13,6 @@ if ! [ -t 1 ]; then fi GIT_ROOT="$(git rev-parse --show-toplevel)" - -# $MESON_OPTS is an env that can optionally be set in the container, -# populated at build time from the Dockerfile. A typical use case would -# be to pass options to trigger cross-compilation -# -# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's -# populated from a GitLab's job configuration - -MESON_ARGS="$MESON_ARGS $MESON_OPTS" - run_cmd() { printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*" $@ -- 2.41.0

After the recent changes, this script no longer executes any logic anymore, it merely defines the jobs run in the GitLab environment. In order to use it, one has to source the file in the environment and then run one of the job "functions". For that, the 'build.sh' name is no longer descriptive enough and 'jobs.sh' feels more suitable and less misleading. Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/{build.sh => jobs.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ci/{build.sh => jobs.sh} (100%) diff --git a/ci/build.sh b/ci/jobs.sh similarity index 100% rename from ci/build.sh rename to ci/jobs.sh -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:17PM +0200, Erik Skultety wrote:
After the recent changes, this script no longer executes any logic anymore, it merely defines the jobs run in the GitLab environment. In order to use it, one has to source the file in the environment and then run one of the job "functions". For that, the 'build.sh' name is no longer descriptive enough and 'jobs.sh' feels more suitable and less misleading.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/{build.sh => jobs.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ci/{build.sh => jobs.sh} (100%)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This is one of the preparation steps that if not done would otherwise collide with local container executions where we: 1) don't collect artifacts 2) are not limited by GitLab's environment and hence moving build artifacts to unusual places would only cause confusion when doing local build inspection in an interactive container shell session Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 944a7b7817..1c6af8f8b3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -31,11 +31,16 @@ include: - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; - mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; else meson compile -C build; meson test -C build --no-suite syntax-check --print-errorlogs; fi + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; + - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; + then + mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/; + fi .native_build_job_prebuilt_env: extends: @@ -77,6 +82,8 @@ include: - *script_variables - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) - DESTDIR=$(pwd)/install meson compile -C build install-web + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; - mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' @@ -155,6 +162,8 @@ potfile: - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - meson compile -C build libvirt-pot-dep - meson compile -C build libvirt-pot + after_script: + - test "$CI_JOB_STATUS" != "success" && exit 1; - cp po/libvirt.pot libvirt.pot artifacts: expose_as: 'Potfile' -- 2.41.0

Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1c6af8f8b3..c837812091 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,15 +25,13 @@ include: - ccache/ key: "$CI_JOB_NAME" script: - - *script_variables - - meson setup build --werror $MESON_ARGS || (cat build/meson-logs/meson-log.txt && exit 1) - - meson dist -C build --no-tests + - source ci/jobs.sh - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then - rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; + run_rpmbuild; else - meson compile -C build; - meson test -C build --no-suite syntax-check --print-errorlogs; + run_build; + run_test; fi after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; -- 2.41.0

On Mon, Sep 11, 2023 at 03:43:19PM +0200, Erik Skultety wrote:
Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1c6af8f8b3..c837812091 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,15 +25,13 @@ include: - ccache/ key: "$CI_JOB_NAME" script: - - *script_variables - - meson setup build --werror $MESON_ARGS || (cat build/meson-logs/meson-log.txt && exit 1) - - meson dist -C build --no-tests + - source ci/jobs.sh - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then - rpmbuild --clean --nodeps --define "_without_mingw 1" -ta build/meson-dist/libvirt-*.tar.xz; + run_rpmbuild; else - meson compile -C build; - meson test -C build --no-suite syntax-check --print-errorlogs; + run_build; + run_test;
I missed a regression here - we're loosing the --no-suite and --print-errorlogs args when running tests, so we can no longer diagnose the failures.
fi after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; -- 2.41.0
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c837812091..f4c5b47f15 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -56,10 +56,12 @@ include: - ccache/ key: "$CI_JOB_NAME" script: - - *script_variables - - meson setup build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build - - if test "$CROSS" = "i686" ; then meson test -C build --no-suite syntax-check --print-errorlogs ; fi + - source ci/jobs.sh + - run_build + - if test "$CROSS" = "i686" ; + then + run_test; + fi .cross_build_job_prebuilt_env: extends: -- 2.41.0

Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f4c5b47f15..d41cc63ec2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -79,9 +79,8 @@ include: # https://gitlab.com/libvirt/libvirt/-/jobs/artifacts/master/download?job=webs... .website_job: script: - - *script_variables - - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) - - DESTDIR=$(pwd)/install meson compile -C build install-web + - source ci/jobs.sh + - run_website_build after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; - mv install/usr/share/doc/libvirt/html/ website -- 2.41.0

Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d41cc63ec2..35f1a1e135 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -114,10 +114,8 @@ website_local_env: .codestyle_job: stage: sanity_checks script: - - *script_variables - - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep - - meson test -C build --suite syntax-check --no-rebuild --print-errorlogs + - source ci/jobs.sh + - run_codestyle codestyle_prebuilt_env: extends: -- 2.41.0

Individual shell command executions are replaced by respective functions in the ci/build.sh base script. This will make sure we use the same recipes in GitLab jobs as well as in local executions. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- .gitlab-ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 35f1a1e135..1cdabed941 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -156,9 +156,8 @@ potfile: before_script: - *script_variables script: - - meson setup build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - - meson compile -C build libvirt-pot-dep - - meson compile -C build libvirt-pot + - source ci/jobs.sh + - run_potfile after_script: - test "$CI_JOB_STATUS" != "success" && exit 1; - cp po/libvirt.pot libvirt.pot -- 2.41.0

This method unused anywhere, so drop it. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ci/helper b/ci/helper index fb562d55e1..8986772153 100755 --- a/ci/helper +++ b/ci/helper @@ -163,10 +163,6 @@ class Application: output = subprocess.check_output([self._args.lcitool] + args) return output.decode("utf-8") - def _lcitool_get_targets(self): - output = self._lcitool_run(["targets"]) - return output.splitlines() - def _check_stale_images(self): namespace = self._args.namespace gitlab_uri = self._args.gitlab_uri -- 2.41.0

':' is just a connecting character, we can add it to the appropriate place later in the Python script later, but it doesn't make sense to be part of the image 'tag' string. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/helper b/ci/helper index 8986772153..d3de6d96cb 100755 --- a/ci/helper +++ b/ci/helper @@ -40,7 +40,7 @@ class Parser: ) containerparser.add_argument( "--image-tag", - default=":latest", + default="latest", help="use container images with non-default tags", ) -- 2.41.0

We'll soon be relying solely on lcitool so we need to be able to run it from a user-provided location if it's not installed in a known location. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/helper b/ci/helper index d3de6d96cb..75552774f6 100755 --- a/ci/helper +++ b/ci/helper @@ -43,6 +43,12 @@ class Parser: default="latest", help="use container images with non-default tags", ) + containerparser.add_argument( + "--lcitool-path", + dest="lcitool", + default="lcitool", + help="path to lcitool (default: $PATH)", + ) # Options that are common to all actions that call the # project's build system -- 2.41.0

Since we'll depend on GitPython for repo cloning, we need to make sure to emit a user friendly error if the module is not installed. This patch introduces a helper which future patches will use as a decorator. Inspiration for this helper has been taken out of lcitool where we use an identical helper for this purpose. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ci/helper b/ci/helper index 75552774f6..4727145b28 100755 --- a/ci/helper +++ b/ci/helper @@ -14,6 +14,28 @@ import textwrap import util +def required_deps(*deps): + module2pkg = { + "git": "GitPython" + } + + def inner_decorator(func): + def wrapped(*args, **kwargs): + cmd = func.__name__[len('_action_'):] + for dep in deps: + try: + import importlib + importlib.import_module(dep) + except ImportError: + pkg = module2pkg[dep] + msg = f"'{pkg}' not found (required by the '{cmd}' command)" + print(msg, file=sys.stderr) + sys.exit(1) + func(*args, **kwargs) + return wrapped + return inner_decorator + + class Parser: def __init__(self): # Options that are common to all actions that use containers -- 2.41.0

This helper will be utilized by a future patch which will add the lcitool container execution logic. The reason why the required_deps decorator isn't being used here is because this is a property. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ci/helper b/ci/helper index 4727145b28..6aca089db4 100755 --- a/ci/helper +++ b/ci/helper @@ -159,9 +159,18 @@ class Parser: class Application: + @property + def repo(self): + if self._repo is None: + from git import Repo + + self._repo = Repo(search_parent_directories=True) + return self._repo + def __init__(self): self._basedir = pathlib.Path(__file__).resolve().parent self._args = Parser().parse() + self._repo = None def _make_run(self, target): args = [ -- 2.41.0

A proper Python equivalent of 'git clone --local'. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/helper b/ci/helper index 6aca089db4..392702ae41 100755 --- a/ci/helper +++ b/ci/helper @@ -196,6 +196,10 @@ class Application: if pty.spawn(["make"] + args) != 0: sys.exit("error: 'make' failed") + @staticmethod + def _prepare_repo_copy(repo, dest): + return repo.clone(dest, local=True) + def _lcitool_run(self, args): output = subprocess.check_output([self._args.lcitool] + args) return output.decode("utf-8") -- 2.41.0

This method wasn't even utilized before this patch. This patch adds all the necessary logic to successfully execute a container workload via lcitool (which will later allow us to ditch ci/Makefile). Because container executions via lcitool creates the following inside the container: $ ls script datadir where 'datadir' is the workload directory (in this case a local git repo clone) and 'script' is the code that runs whatever the workload is over 'datadir'. In order to satisfy the ^above, our helper generates a trivial temporary 'script' that will source ci/build.sh and run whatever was specified as --job essentially to simulate the exact steps a GitLab pipeline job would go through. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/ci/helper b/ci/helper index 392702ae41..fce370f995 100755 --- a/ci/helper +++ b/ci/helper @@ -11,6 +11,9 @@ import subprocess import sys import textwrap +from pathlib import Path +from tempfile import TemporaryDirectory + import util @@ -201,8 +204,56 @@ class Application: return repo.clone(dest, local=True) def _lcitool_run(self, args): - output = subprocess.check_output([self._args.lcitool] + args) - return output.decode("utf-8") + positional_args = ["container"] + opts = ["--user", self._args.login] + tmpdir = TemporaryDirectory(prefix="scratch", + dir=Path(self.repo.working_dir, "ci")) + + repo_dest_path = Path(tmpdir.name, "libvirt.git").as_posix() + repo_clone = self._prepare_repo_copy(self.repo, repo_dest_path) + opts.extend(["--workload-dir", repo_clone.working_dir]) + + if self._args.job == "shell": + positional_args.append("shell") + else: + job2func = { + "test": "run_test", + "build": "run_build", + "codestyle": "run_codestyle", + "potfile": "run_potfile", + "rpmbuild": "run_rpmbuild", + "website": "run_website_build", + } + + if self._args.engine != "auto": + positional_args.extend(["--engine", self._args.engine]) + + with open(Path(tmpdir.name, "script"), "w") as f: + script_path = f.name + contents = textwrap.dedent(f"""\ + #!/bin/sh + + cd datadir + . ci/jobs.sh + + {job2func[self._args.job]} + """) + + f.write(contents) + + positional_args.append("run") + opts.extend(["--script", script_path]) + + opts.append(f"{self._args.image_prefix}{self._args.target}:{self._args.image_tag}") + proc = None + try: + proc = subprocess.run([self._args.lcitool] + positional_args + opts) + except KeyboardInterrupt: + sys.exit(1) + finally: + # this will take care of the generated script file above as well + tmpdir.cleanup() + return proc.returncode def _check_stale_images(self): namespace = self._args.namespace -- 2.41.0

Just like with the other CLI sub-commands, add an action to run a GitLab spec job in a local container via lcitool. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/helper b/ci/helper index fce370f995..f7b0204ea0 100755 --- a/ci/helper +++ b/ci/helper @@ -295,6 +295,10 @@ class Application: def _action_shell(self): self._make_run(f"ci-shell@{self._args.target}") + @required_deps("git") + def _action_run(self): + return self._lcitool_run(self._args.job) + def _action_list_images(self): registry_uri = util.get_registry_uri(self._args.namespace, self._args.gitlab_uri) -- 2.41.0

The idea behind this subcommand is to follow whatever build job we have defined in the GitLab CI pipeline, so that we only have a single source of truth for the recipes. Adds 'shell' as an extra option for interactive container build debugging. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ci/helper b/ci/helper index f7b0204ea0..bc5de008b2 100755 --- a/ci/helper +++ b/ci/helper @@ -139,6 +139,21 @@ class Parser: ) shellparser.set_defaults(func=Application._action_shell) + jobparser = subparsers.add_parser( + "run", + help="Run a GitLab CI job or 'shell' in a local environment", + parents=[containerparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, + ) + jobparser.add_argument( + "--job", + choices=["build", "codestyle", "potfile", "rpmbuild", + "shell", "test", "website"], + default="build", + help="Run a GitLab CI job or 'shell' in a local environment", + ) + jobparser.set_defaults(func=Application._action_run) + # list-images action listimagesparser = subparsers.add_parser( "list-images", -- 2.41.0

Previous patches added a single 'run' command parametrized with GitLab job specs via '--job' that cover all of these original actions, adding some more in the process. Drop the original actions as we don't need them anymore. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/ci/helper b/ci/helper index bc5de008b2..b90dc56ede 100755 --- a/ci/helper +++ b/ci/helper @@ -112,33 +112,6 @@ class Parser: ) subparsers.required = True - # build action - buildparser = subparsers.add_parser( - "build", - help="run a build in a container", - parents=[containerparser, mesonparser], - formatter_class=argparse.ArgumentDefaultsHelpFormatter, - ) - buildparser.set_defaults(func=Application._action_build) - - # test action - testparser = subparsers.add_parser( - "test", - help="run a build in a container (including tests)", - parents=[containerparser, mesonparser], - formatter_class=argparse.ArgumentDefaultsHelpFormatter, - ) - testparser.set_defaults(func=Application._action_test) - - # shell action - shellparser = subparsers.add_parser( - "shell", - help="start a shell in a container", - parents=[containerparser], - formatter_class=argparse.ArgumentDefaultsHelpFormatter, - ) - shellparser.set_defaults(func=Application._action_shell) - jobparser = subparsers.add_parser( "run", help="Run a GitLab CI job or 'shell' in a local environment", @@ -301,15 +274,6 @@ class Application: """) print(msg.replace("STALE_DETAILS", stale_details)) - def _action_build(self): - self._make_run(f"ci-build@{self._args.target}") - - def _action_test(self): - self._make_run(f"ci-test@{self._args.target}") - - def _action_shell(self): - self._make_run(f"ci-shell@{self._args.target}") - @required_deps("git") def _action_run(self): return self._lcitool_run(self._args.job) -- 2.41.0

These originally allowed customizing the ci/Makefile script which was the core of the local container executions. The problem was that however flexible this may have been, it never mirrored what was being done as part of the GitLab jobs. Motivated by the effort of mirroring GitLab jobs locally, these would only ever make sense to be set/used in interactive shell container sessions where the developer is perfectly capable of using the right meson/ninja CLI options directly without going through another shell variable indirection as it was the case with these ci/helper options. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/ci/helper b/ci/helper index b90dc56ede..c734629731 100755 --- a/ci/helper +++ b/ci/helper @@ -75,21 +75,6 @@ class Parser: help="path to lcitool (default: $PATH)", ) - # Options that are common to all actions that call the - # project's build system - mesonparser = argparse.ArgumentParser(add_help=False) - mesonparser.add_argument( - "--meson-args", - default="", - help="additional arguments passed to meson " - "(eg --meson-args='-Dopt1=enabled -Dopt2=disabled')", - ) - mesonparser.add_argument( - "--ninja-args", - default="", - help="additional arguments passed to ninja", - ) - # Options that are common to actions communicating with a GitLab # instance gitlabparser = argparse.ArgumentParser(add_help=False) -- 2.41.0

We've successfully migrated over to lcitool to take care of the container workload execution, so dropping this 'make' prep code is a prerequisite of finally getting rid of the ci/Makefile script. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/helper | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/ci/helper b/ci/helper index c734629731..93508515ca 100755 --- a/ci/helper +++ b/ci/helper @@ -6,7 +6,6 @@ import argparse import os import pathlib -import pty import subprocess import sys import textwrap @@ -148,30 +147,6 @@ class Application: self._args = Parser().parse() self._repo = None - def _make_run(self, target): - args = [ - "-C", - self._basedir, - target, - ] - - if self._args.action in ["build", "test", "shell"]: - args.extend([ - f"CI_ENGINE={self._args.engine}", - f"CI_USER_LOGIN={self._args.login}", - f"CI_IMAGE_PREFIX={self._args.image_prefix}", - f"CI_IMAGE_TAG={self._args.image_tag}", - ]) - - if self._args.action in ["build", "test"]: - args.extend([ - f"MESON_ARGS={self._args.meson_args}", - f"NINJA_ARGS={self._args.ninja_args}", - ]) - - if pty.spawn(["make"] + args) != 0: - sys.exit("error: 'make' failed") - @staticmethod def _prepare_repo_copy(repo, dest): return repo.clone(dest, local=True) -- 2.41.0

All the functionality this script provided has been incorporated either in the Python ci/helper tool or lcitool directly. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Erik Skultety <eskultet@redhat.com>: --- ci/Makefile | 245 ---------------------------------------------------- 1 file changed, 245 deletions(-) delete mode 100644 ci/Makefile diff --git a/ci/Makefile b/ci/Makefile deleted file mode 100644 index 8f1be4318d..0000000000 --- a/ci/Makefile +++ /dev/null @@ -1,245 +0,0 @@ -# -*- makefile -*- -# vim: filetype=make - -# The root directory of the libvirt.git checkout -CI_GIT_ROOT = $(shell git rev-parse --show-toplevel) - -# The root directory for all CI-related contents -CI_ROOTDIR = $(CI_GIT_ROOT)/ci - -# The directory holding content on the host that we will -# expose to the container. -CI_SCRATCHDIR = $(CI_ROOTDIR)/scratch - -# The directory holding the clone of the git repo that -# we will expose to the container -CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src - -# The directory holding the source inside the -# container, i.e. where we want to expose -# the $(CI_HOST_SRCDIR) directory from the host -CI_CONT_SRCDIR = $(CI_USER_HOME)/libvirt - -# Script containing build instructions -CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh - -# Location of the container images we're going to pull -# Can be useful to override to use a locally built -# image instead -CI_IMAGE_PREFIX = registry.gitlab.com/libvirt/libvirt/ci- - -# The default tag is ':latest' but if the container -# repo above uses different conventions this can override it -CI_IMAGE_TAG = :latest - -# We delete the virtual root after completion, set -# to 0 if you need to keep it around for debugging -CI_CLEAN = 1 - -# We'll always freshly clone the virtual root each -# time in case it was not cleaned up before. Set -# to 1 if you want to try restarting a previously -# preserved env -CI_REUSE = 0 - -# We need the user's login and home directory to prepare the -# environment the way some programs expect it -CI_USER_LOGIN = $(shell whoami) -CI_USER_HOME = $(shell eval echo "~$(CI_USER_LOGIN)") - -# We also need the container process to run with current host IDs -# so that it can access the passed in build directory -CI_UID = $(shell id -u "$(CI_USER_LOGIN)") -CI_GID = $(shell id -g "$(CI_USER_LOGIN)") - -CI_ENGINE = auto -# Container engine we are going to use, can be overridden per make -# invocation, if it is not we try podman and then default to docker. -ifeq ($(CI_ENGINE),auto) - override CI_ENGINE = $(shell podman version >/dev/null 2>&1 && echo podman || echo docker) -endif - -# IDs you run as do not need to exist in -# the container's /etc/passwd & /etc/group files, but -# if they do not, then libvirt's 'ninja test' will fail -# many tests. - -# We do not directly mount /etc/{passwd,group} as Docker -# is liable to mess with SELinux labelling which will -# then prevent the host accessing them. And podman cannot -# relabel the files due to it running rootless. So -# copying them first is safer and less error-prone. -CI_PWDB_MOUNTS = \ - --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \ - --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \ - $(NULL) - -CI_HOME_MOUNTS = \ - --volume $(CI_SCRATCHDIR)/home:$(CI_USER_HOME):z \ - $(NULL) - -CI_SCRIPT_MOUNTS = \ - --volume $(CI_SCRATCHDIR)/build:$(CI_USER_HOME)/build:z \ - $(NULL) - -# Docker containers can have very large ulimits -# for nofiles - as much as 1048576. This makes -# libvirt very slow at exec'ing programs. -CI_ULIMIT_FILES = 1024 - -ifeq ($(CI_ENGINE),podman) - # Podman cannot reuse host namespace when running non-root - # containers. Until support for --keep-uid is added we can - # just create another mapping that will do that for us. - # Beware, that in {uid,git}map=container_id:host_id:range, the - # host_id does actually refer to the uid in the first mapping - # where 0 (root) is mapped to the current user and rest is - # offset. - # - # In order to set up this mapping, we need to keep all the - # user IDs to prevent possible errors as some images might - # expect UIDs up to 90000 (looking at you fedora), so we don't - # want the overflowuid to be used for them. For mapping all - # the other users properly, some math needs to be done. - # Don't worry, it's just addition and subtraction. - # - # 65536 ought to be enough (tm), but for really rare cases the - # maximums might need to be higher, but that only happens when - # your /etc/sub{u,g}id allow users to have more IDs. Unless - # --keep-uid is supported, let's do this in a way that should - # work for everyone. - CI_MAX_UID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" /etc/subuid) - CI_MAX_GID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" /etc/subgid) - ifeq ($(CI_MAX_UID),) - CI_MAX_UID = 65536 - endif - ifeq ($(CI_MAX_GID),) - CI_MAX_GID = 65536 - endif - CI_UID_OTHER = $(shell echo $$(($(CI_UID)+1))) - CI_GID_OTHER = $(shell echo $$(($(CI_GID)+1))) - CI_UID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_UID)-$(CI_UID)))) - CI_GID_OTHER_RANGE = $(shell echo $$(($(CI_MAX_GID)-$(CI_GID)))) - - ifneq ($(CI_UID), 0) - CI_PODMAN_ARGS = \ - --uidmap 0:1:$(CI_UID) \ - --uidmap $(CI_UID):0:1 \ - --uidmap $(CI_UID_OTHER):$(CI_UID_OTHER):$(CI_UID_OTHER_RANGE) \ - --gidmap 0:1:$(CI_GID) \ - --gidmap $(CI_GID):0:1 \ - --gidmap $(CI_GID_OTHER):$(CI_GID_OTHER):$(CI_GID_OTHER_RANGE) \ - $(NULL) - endif -endif - -# Args to use when cloning a git repo. -# -c stop it complaining about checking out a random hash -# -q stop it displaying progress info for local clone -# --local ensure we don't actually copy files -CI_GIT_ARGS = \ - -c advice.detachedHead=false \ - -q \ - --local \ - $(NULL) - -# Args to use when running the container -# --rm stop inactive containers getting left behind -# --user we execute as the same user & group account -# as dev so that file ownership matches host -# instead of root:root -# --workdir we change to user's home dir in the container -# before running the workload -# --volume to pass in the cloned git repo & config -# --ulimit lower files limit for performance reasons -# --interactive -# --tty Ensure we have ability to Ctrl-C the build -CI_ENGINE_ARGS = \ - --rm \ - --interactive \ - --tty \ - --user "$(CI_UID)":"$(CI_GID)" \ - --workdir "$(CI_USER_HOME)" \ - --env CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \ - --env MESON_ARGS="$(MESON_ARGS)" \ - --env NINJA_ARGS="$(NINJA_ARGS)" \ - $(CI_PODMAN_ARGS) \ - $(CI_PWDB_MOUNTS) \ - $(CI_HOME_MOUNTS) \ - $(CI_SCRIPT_MOUNTS) \ - --volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \ - --ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \ - --cap-add=SYS_PTRACE \ - $(NULL) - -ci-check-engine: - @echo -n "Checking if $(CI_ENGINE) is available..." && \ - $(CI_ENGINE) version 1>/dev/null && echo "yes" - -ci-prepare-tree: ci-check-engine - @test "$(CI_REUSE)" != "1" && rm -rf $(CI_SCRATCHDIR) || : - @if ! test -d $(CI_SCRATCHDIR) ; then \ - mkdir -p $(CI_SCRATCHDIR); \ - cp /etc/passwd $(CI_SCRATCHDIR); \ - cp /etc/group $(CI_SCRATCHDIR); \ - mkdir -p $(CI_SCRATCHDIR)/home; \ - cp "$(CI_BUILD_SCRIPT)" $(CI_SCRATCHDIR)/build; \ - chmod +x "$(CI_SCRATCHDIR)/build"; \ - echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \ - git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || exit 1; \ - for mod in $$(git submodule | awk '{ print $$2 }' | sed -E 's,^../,,g') ; \ - do \ - test -f $(CI_GIT_ROOT)/$$mod/.git || continue ; \ - echo "Cloning $(CI_GIT_ROOT)/$$mod to $(CI_HOST_SRCDIR)/$$mod"; \ - git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT)/$$mod $(CI_HOST_SRCDIR)/$$mod || exit 1; \ - done ; \ - fi - -ci-run-command@%: ci-prepare-tree - $(CI_ENGINE) run \ - $(CI_ENGINE_ARGS) \ - $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \ - $(CI_COMMAND) - @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || : - -ci-shell@%: - $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="/bin/bash" - -ci-build@%: - $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build" - -ci-test@%: - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS=test - -ci-help: - @echo - @echo - @echo - @echo " !!! PLEASE DON'T USE THIS DIRECTLY !!!" - @echo - @echo " Use the ci/helper script instead" - @echo - @echo " !!! PLEASE DON'T USE THIS DIRECTLY !!!" - @echo - @echo - @echo - @echo "Build libvirt inside containers used for CI" - @echo - @echo "Available targets:" - @echo - @echo " ci-build@\$$IMAGE - run a default 'ninja' build" - @echo " ci-test@\$$IMAGE - run a 'ninja test'" - @echo " ci-shell@\$$IMAGE - run an interactive shell" - @echo " ci-help - show this help message" - @echo - @echo "Available make variables:" - @echo - @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" - @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content" - @echo " CI_ENGINE=auto - container engine to use (podman, docker)" - @echo " CI_USER_LOGIN= - which user should run in the container (default is $$USER)" - @echo " CI_IMAGE_PREFIX= - override to prefer a locally built image, (default is $(CI_IMAGE_PREFIX))" - @echo " CI_IMAGE_TAG=:latest - optionally use in conjunction with 'CI_IMAGE_PREFIX'" - @echo " CI_MESON_ARGS= - extra arguments passed to meson" - @echo " CI_NINJA_ARGS= - extra arguments passed to ninja" - @echo -- 2.41.0
participants (2)
-
Daniel P. Berrangé
-
Erik Skultety