[libvirt PATCH 00/33] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions
12:55 p.m.
Technically a v2 of:
https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
However, the approach here is slightly different and what that series said
about migration to lcitool container executions as a replacement for
ci/Makefile is actually done here. One of the core problems of the above
pointed out in review was that more Shell logic was introduced including CLI
parsing, conditional executions, etc. which we fought hard to get rid of in the
past. I reworked the Shell functions quite a bit and dropped whatever extra
Shell logic the original series added.
Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and so
I merely extracted the recipes into functions which are then sourced as
ci/build.sh and executed. Now, that on its own would hide the actual commands
being run in the GitLab job log, so before any command is actually executed, it
is formatted with a color sequence so we don't miss that information as that
would be a regression to the status quo.
Lastly, this series then takes the effort inside the ci/build.sh script and
basically mirrors whatever GitLab would do to run a job inside a local
container which is executed by lcitool (yes, we already have that capability).
Please give this a try and I'm already looking forward to comments as I'd like
to expand this effort to local VM executions running the TCK integration tests,
so this series is quite important in that regard.
Erik Skultety (33):
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 over meson's setup
ci: build.sh: Add a wrapper function executing 'shell' commands
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
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 job argparse subparser
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: Drop original actions
ci: helper: Drop the --meson-args/--ninja-args CLI options
ci: helper: Drop the _make_run method
.gitlab-ci.yml | 47 +++++++------
ci/build.sh | 105 +++++++++++++++++++++++++----
ci/helper | 176 ++++++++++++++++++++++++++++---------------------
3 files changed, 218 insertions(+), 110 deletions(-)
--
2.41.0
12:55 p.m.
New subject: [libvirt PATCH 01/33] ci: build.sh: Add variables from .gitlab-ci.yml
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(a)redhat.com>
---
ci/build.sh | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ci/build.sh b/ci/build.sh
index ed9b1f4473..d5586f457c 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -2,7 +2,12 @@
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"
+export VIR_TEST_VERBOSE="1"
+export VIR_TEST_DEBUG="1"
# $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
11:45 a.m.
New subject: [libvirt PATCH 01/33] ci: build.sh: Add variables from .gitlab-ci.yml
On Fri, Aug 25, 2023 at 07:55:09PM +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(a)redhat.com>
---
ci/build.sh | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ci/build.sh b/ci/build.sh
index ed9b1f4473..d5586f457c 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -2,7 +2,12 @@
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"
+export VIR_TEST_VERBOSE="1"
+export VIR_TEST_DEBUG="1"
These last two vars are good for CI, because in a non-interactive env we
need the verbose / debug output upfront to stand a chance of diagnosing
problems from logs after the fact
If I'm running a build locally with our ci/Makefile, I don't especially
want it to be forced into verbose mode by default, as I have an interactive
shell I can do that as & when needed myself.
Perhaps we can do
if ! test -t 1
then
export VIR_TEST_VERBOSE="1"
export VIR_TEST_DEBUG="1"
fi
such that we only set the verbose vars when STDOUT is NOT a TTY.
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 :|
12:22 p.m.
New subject: [libvirt PATCH 01/33] ci: build.sh: Add variables from .gitlab-ci.yml
On Thu, Aug 31, 2023 at 05:45:05PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:09PM +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(a)redhat.com>
> ---
> ci/build.sh | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> diff --git a/ci/build.sh b/ci/build.sh
> index ed9b1f4473..d5586f457c 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -2,7 +2,12 @@
>
> 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"
> +export VIR_TEST_VERBOSE="1"
> +export VIR_TEST_DEBUG="1"
These last two vars are good for CI, because in a non-interactive env we
need the verbose / debug output upfront to stand a chance of diagnosing
problems from logs after the fact
If I'm running a build locally with our ci/Makefile, I don't especially
want it to be forced into verbose mode by default, as I have an interactive
shell I can do that as & when needed myself.
Perhaps we can do
if ! test -t 1
then
export VIR_TEST_VERBOSE="1"
export VIR_TEST_DEBUG="1"
fi
This uncovered an interesting bug in the ci/helper + lcitool combo.
In lcitool we explicitly always ask for a TTY with --tty on podman/docker CLI.
If I drop the argument and interrupt the execution I'll get an unhandled
OSError exception from lcitool and a stacktrace from ci/helper which is nicely
interlaced with meson's own stacktrace if you interrupt the setup phase. I need
to look into lcitool whether there's an easy fix. The other temporary solution
until we incorporate the ci/helper logic into lcitool would be to set another
variable inside the temporary crafted script by ci/helper that would serve the
same purpose.
Erik
12:55 p.m.
New subject: [libvirt PATCH 02/33] ci: build.sh: Add GIT_ROOT env helper variable
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(a)redhat.com>
---
ci/build.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index d5586f457c..152c222244 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -9,6 +9,8 @@ export PATH="$CCACHE_WRAPPERSDIR:$PATH"
export VIR_TEST_VERBOSE="1"
export VIR_TEST_DEBUG="1"
+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
11:45 a.m.
New subject: [libvirt PATCH 02/33] ci: build.sh: Add GIT_ROOT env helper variable
On Fri, Aug 25, 2023 at 07:55:10PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/build.sh | 2 ++
1 file changed, 2 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 03/33] ci: build.sh: Don't mention that MESON_ARGS are available via CLI
Previous patches have removed the code that allowed injecting arbitrary
meson arguments, same for ninja args.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index 152c222244..82016ba29c 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -16,9 +16,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-configure-args=-Dopt1 -Dopt2` when run in a local
-# containerized environment
+# populated from a GitLab's job configuration
MESON_ARGS="$MESON_ARGS $MESON_OPTS"
--
2.41.0
11:45 a.m.
New subject: [libvirt PATCH 03/33] ci: build.sh: Don't mention that MESON_ARGS are available via CLI
On Fri, Aug 25, 2023 at 07:55:11PM +0200, Erik Skultety wrote:
Previous patches have removed the code that allowed injecting
arbitrary
meson arguments, same for ninja args.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 04/33] ci: build.sh: Add a wrapper function over meson's setup
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(a)redhat.com>
---
ci/build.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 82016ba29c..02ff1a8388 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -24,3 +24,20 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
(cat build/meson-logs/meson-log.txt && exit 1)
ninja -C build $NINJA_ARGS
+
+run_meson_setup() {
+ if [ -d "${GIT_ROOT}/build/meson-private" ]; then
+ return
+ fi
+
+ local DUMP_ERR_CMD="cat ${GIT_ROOT}/build/meson-logs/meson-log.txt"
+ local SETUP_CMD="meson \
+ setup \
+ build \
+ --werror \
+ -Dsystem=true $MESON_OPTS $MESON_ARGS"
+
+ local CMD="$SETUP_CMD || ($DUMP_ERR_CMD && exit 1)"
+
+ run_cmd "$CMD"
+}
--
2.41.0
11:37 a.m.
New subject: [libvirt PATCH 04/33] ci: build.sh: Add a wrapper function over meson's setup
On Fri, Aug 25, 2023 at 07:55:12PM +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(a)redhat.com>
---
ci/build.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 82016ba29c..02ff1a8388 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -24,3 +24,20 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
(cat build/meson-logs/meson-log.txt && exit 1)
ninja -C build $NINJA_ARGS
+
+run_meson_setup() {
+ if [ -d "${GIT_ROOT}/build/meson-private" ]; then
+ return
+ fi
This isn't as accurate as meson's own check. I interrupted meson
part way through, and this directry exists, but meson had written
out sufficient files todo a build. Re-running 'meson setup build'
directly happily runs, but this run_meson_setup command becomes
a no-op which is kinda annoying.
+
+ local DUMP_ERR_CMD="cat ${GIT_ROOT}/build/meson-logs/meson-log.txt"
+ local SETUP_CMD="meson \
+ setup \
+ build \
+ --werror \
+ -Dsystem=true $MESON_OPTS $MESON_ARGS"
+
+ local CMD="$SETUP_CMD || ($DUMP_ERR_CMD && exit 1)"
+
+ run_cmd "$CMD"
+}
--
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 :|
11:48 a.m.
New subject: [libvirt PATCH 04/33] ci: build.sh: Add a wrapper function over meson's setup
On Thu, Aug 31, 2023 at 05:37:47PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:12PM +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(a)redhat.com>
> ---
> ci/build.sh | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/ci/build.sh b/ci/build.sh
> index 82016ba29c..02ff1a8388 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -24,3 +24,20 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
> (cat build/meson-logs/meson-log.txt && exit 1)
>
> ninja -C build $NINJA_ARGS
> +
> +run_meson_setup() {
> + if [ -d "${GIT_ROOT}/build/meson-private" ]; then
> + return
> + fi
This isn't as accurate as meson's own check. I interrupted meson
part way through, and this directry exists, but meson had written
out sufficient files todo a build. Re-running 'meson setup build'
directly happily runs, but this run_meson_setup command becomes
a no-op which is kinda annoying.
Perhaps we can just push the test into the caller. eg
test -d $GIT_ROOT/build || run_meson_setup
so commands like 'run_build' "just work", but if someone explicitly
invokes 'run_meson_setup' we just let meson detect a pre-created
build dir using its native logic.
> +
> + local DUMP_ERR_CMD="cat ${GIT_ROOT}/build/meson-logs/meson-log.txt"
> + local SETUP_CMD="meson \
> + setup \
> + build \
> + --werror \
> + -Dsystem=true $MESON_OPTS $MESON_ARGS"
> +
> + local CMD="$SETUP_CMD || ($DUMP_ERR_CMD && exit 1)"
> +
> + run_cmd "$CMD"
> +}
> --
> 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 :|
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 :|
12:55 p.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
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(a)redhat.com>
---
ci/build.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 02ff1a8388..d4fbf0ab37 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
ninja -C build $NINJA_ARGS
+run_cmd() {
+ local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
+
+ printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval
"$CMD"
+}
+
run_meson_setup() {
if [ -d "${GIT_ROOT}/build/meson-private" ]; then
return
--
2.41.0
9:30 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Aug 25, 2023 at 07:55:13PM +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'
FWIW, I happened to come across this
https://docs.gitlab.com/ee/ci/jobs/index.html#custom-collapsible-sections
which lets scripts output messages in a special format that get
turned into collapsible sections when browsing logs in gitlab UI.
Not sure it is something we'd use in this specific patch, but
might be conceptually useful somewhere.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 02ff1a8388..d4fbf0ab37 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
ninja -C build $NINJA_ARGS
+run_cmd() {
+ local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
+
+ printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval
"$CMD"
+}
+
run_meson_setup() {
if [ -d "${GIT_ROOT}/build/meson-private" ]; then
return
--
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 :|
4:46 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Thu, Aug 31, 2023 at 03:30:23PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:13PM +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'
FWIW, I happened to come across this
https://docs.gitlab.com/ee/ci/jobs/index.html#custom-collapsible-sections
which lets scripts output messages in a special format that get
turned into collapsible sections when browsing logs in gitlab UI.
Not sure it is something we'd use in this specific patch, but
might be conceptually useful somewhere.
This actually works kinda nicely. I added this commit on top of
yours:
https://gitlab.com/berrange/libvirt/-/commit/29f123024c265262e70a409daa00...
and an example pipeline is:
https://gitlab.com/berrange/libvirt/-/jobs/4997176074
Notice there are three collapsible sections in the job logs
"Running 'meson setup'"
"Running 'meson build'"
"Running 'meson test'"
collapsing earlier sections makes it somewhat easier to find the
phase you're interested in, though its doesn't work so well when
gitlab truncates the earlier part of the log that's over 500k.
Not a problem for pipelines on master, but bad for forks.
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 :|
5:16 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Sep 01, 2023 at 10:46:06AM +0100, Daniel P. Berrangé wrote:
On Thu, Aug 31, 2023 at 03:30:23PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:13PM +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'
>
> FWIW, I happened to come across this
>
> https://docs.gitlab.com/ee/ci/jobs/index.html#custom-collapsible-sections
>
> which lets scripts output messages in a special format that get
> turned into collapsible sections when browsing logs in gitlab UI.
> Not sure it is something we'd use in this specific patch, but
> might be conceptually useful somewhere.
This actually works kinda nicely. I added this commit on top of
yours:
https://gitlab.com/berrange/libvirt/-/commit/29f123024c265262e70a409daa00...
and an example pipeline is:
https://gitlab.com/berrange/libvirt/-/jobs/4997176074
Notice there are three collapsible sections in the job logs
"Running 'meson setup'"
"Running 'meson build'"
"Running 'meson test'"
collapsing earlier sections makes it somewhat easier to find the
phase you're interested in, though its doesn't work so well when
gitlab truncates the earlier part of the log that's over 500k.
Not a problem for pipelines on master, but bad for forks.
Not bad. We could improve on it and go as far as print the actual command in
the section header and fallback to what I proposed in local environments.
Let's finish this series first and then you can refine and post your patch as a
follow up improvement.
Erik
11:33 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Aug 25, 2023 at 07:55:13PM +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(a)redhat.com>
---
ci/build.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 02ff1a8388..d4fbf0ab37 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
ninja -C build $NINJA_ARGS
+run_cmd() {
+ local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
+
+ printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval
"$CMD"
+}
Bright yellow is unfortunately almost unreadable when used on a
terminal with white/light coloured background. Using green '32m'
ought to be visible in both light/dark terminals reasonabley well.
+
run_meson_setup() {
if [ -d "${GIT_ROOT}/build/meson-private" ]; then
return
--
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 :|
4:10 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Aug 25, 2023 at 07:55:13PM +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(a)redhat.com>
---
ci/build.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 02ff1a8388..d4fbf0ab37 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
ninja -C build $NINJA_ARGS
+run_cmd() {
+ local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
+
+ printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval
"$CMD"
+}
I think we sould just get rid of the $CMD env variable in the caller
entirely and pass in arguments individual. eg so this method becomes
run_cmd() {
printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$*"
$@
}
Then in the callers instead of
local CMD="meson compile -C build $BUILD_ARGS"
run_cmd "$CMD"
We get
run_cmd meson compile -C build "$BUILD_ARGS"
this would have avoided the bug you just posted a fix for where
we set 'local CMD' but forget the actual 'run_cmd' call.
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 :|
4:16 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Sep 01, 2023 at 10:10:55AM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:13PM +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(a)redhat.com>
> ---
> ci/build.sh | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/ci/build.sh b/ci/build.sh
> index 02ff1a8388..d4fbf0ab37 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
>
> ninja -C build $NINJA_ARGS
>
> +run_cmd() {
> + local CMD="$(echo $CMD | tr -s ' ')" # truncate any
additional spaces
> +
> + printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD";
eval "$CMD"
> +}
I think we sould just get rid of the $CMD env variable in the caller
entirely and pass in arguments individual. eg so this method becomes
run_cmd() {
printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$*"
$@
}
Then in the callers instead of
local CMD="meson compile -C build $BUILD_ARGS"
run_cmd "$CMD"
We get
run_cmd meson compile -C build "$BUILD_ARGS"
this would have avoided the bug you just posted a fix for where
we set 'local CMD' but forget the actual 'run_cmd' call.
My only complaint is, again, readability - this particular example is fine, it
would IMO become a mess with commands taking several arguments which would not
fit onto a single line. I don't expect these functions to change much, so while
you're absolutely right about preventing bugs like that, I think having some
reasonable readability (shell--) would be a good tradeoff.
Erik
4:19 a.m.
New subject: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands
On Fri, Sep 01, 2023 at 11:16:09AM +0200, Erik Skultety wrote:
On Fri, Sep 01, 2023 at 10:10:55AM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:13PM +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(a)redhat.com>
> > ---
> > ci/build.sh | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/ci/build.sh b/ci/build.sh
> > index 02ff1a8388..d4fbf0ab37 100644
> > --- a/ci/build.sh
> > +++ b/ci/build.sh
> > @@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
> >
> > ninja -C build $NINJA_ARGS
> >
> > +run_cmd() {
> > + local CMD="$(echo $CMD | tr -s ' ')" # truncate any
additional spaces
> > +
> > + printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n"
"$CMD"; eval "$CMD"
> > +}
>
> I think we sould just get rid of the $CMD env variable in the caller
> entirely and pass in arguments individual. eg so this method becomes
>
> run_cmd() {
> printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$*"
> $@
> }
>
> Then in the callers instead of
>
> local CMD="meson compile -C build $BUILD_ARGS"
> run_cmd "$CMD"
>
> We get
>
> run_cmd meson compile -C build "$BUILD_ARGS"
>
> this would have avoided the bug you just posted a fix for where
> we set 'local CMD' but forget the actual 'run_cmd' call.
My only complaint is, again, readability - this particular example is fine, it
would IMO become a mess with commands taking several arguments which would not
fit onto a single line. I don't expect these functions to change much, so while
you're absolutely right about preventing bugs like that, I think having some
reasonable readability (shell--) would be a good tradeoff.
A line continuation isn't that hard to add for comamnds which get too
long.
Avoiding the intermediate variable is also more robust by eliminating
two levels of extra quoting. Quoting/expansion problems are one of the
more painful aspects of working in shell, so I think its good to cull
extra variables where practical.
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 :|
12:55 p.m.
New subject: [libvirt PATCH 06/33] ci: build.sh: Add a wrapper function over the 'build' job
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index d4fbf0ab37..c2f54707ce 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -47,3 +47,10 @@ run_meson_setup() {
run_cmd "$CMD"
}
+
+run_build() {
+ local CMD="meson compile -C build $BUILD_ARGS"
+
+ run_meson_setup
+ run_cmd "$CMD"
+}
--
2.41.0
12:08 p.m.
New subject: [libvirt PATCH 06/33] ci: build.sh: Add a wrapper function over the 'build' job
On Fri, Aug 25, 2023 at 07:55:14PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index d4fbf0ab37..c2f54707ce 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -47,3 +47,10 @@ run_meson_setup() {
run_cmd "$CMD"
}
+
+run_build() {
+ local CMD="meson compile -C build $BUILD_ARGS"
+
+ run_meson_setup
Per the previous patch, I think we should do
test -d $GIT_ROOT/build/build.ninja || run_meson_setup
so that 'run_meson_setup' can do its thing unconditionally if
invoked explicitly
+ run_cmd "$CMD"
+}
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 :|
12:55 p.m.
New subject: [libvirt PATCH 07/33] ci: build.sh: Add a helper function to create the dist tarball
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(a)redhat.com>
---
ci/build.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index c2f54707ce..c56e45f51f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -54,3 +54,15 @@ run_build() {
run_meson_setup
run_cmd "$CMD"
}
+
+run_dist() {
+ local CMD="meson dist -C build --no-tests"
+
+ 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 "$CMD"
+}
--
2.41.0
12:09 p.m.
New subject: [libvirt PATCH 07/33] ci: build.sh: Add a helper function to create the dist tarball
On Fri, Aug 25, 2023 at 07:55:15PM +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(a)redhat.com>
---
ci/build.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index c2f54707ce..c56e45f51f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -54,3 +54,15 @@ run_build() {
run_meson_setup
run_cmd "$CMD"
}
+
+run_dist() {
+ local CMD="meson dist -C build --no-tests"
+
+ run_meson_setup
Same comment as prev patch about conditionally invoking this
+
+ # 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
This is all very peculiar but I concur, this does appear to "solve" it
for our needs.
+ run_cmd "$CMD"
+}
--
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 :|
12:55 p.m.
New subject: [libvirt PATCH 08/33] ci: build.sh: Add a wrapper function over the 'test' job
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index c56e45f51f..ab471f2741 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -66,3 +66,10 @@ run_dist() {
git update-index --refresh
run_cmd "$CMD"
}
+
+run_test() {
+ local CMD="meson test -C build $TEST_ARGS"
+
+ run_meson_setup
+ run_cmd "$CMD"
+}
--
2.41.0
12:55 p.m.
New subject: [libvirt PATCH 09/33] ci: build.sh: Add a wrapper function over the 'codestyle' job
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index ab471f2741..b2891a0c0f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -73,3 +73,12 @@ run_test() {
run_meson_setup
run_cmd "$CMD"
}
+
+run_codestyle() {
+ BUILD_ARGS="libvirt-pot-dep"
+ TEST_ARGS="--suite syntax-check --no-rebuild --print-errorlogs"
+
+ run_meson_setup
+ run_build
+ run_test
+}
--
2.41.0
11:54 a.m.
New subject: [libvirt PATCH 09/33] ci: build.sh: Add a wrapper function over the 'codestyle' job
On Fri, Aug 25, 2023 at 07:55:17PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index ab471f2741..b2891a0c0f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -73,3 +73,12 @@ run_test() {
run_meson_setup
run_cmd "$CMD"
}
+
+run_codestyle() {
+ BUILD_ARGS="libvirt-pot-dep"
+ TEST_ARGS="--suite syntax-check --no-rebuild --print-errorlogs"
+
+ run_meson_setup
+ run_build
run_build already implies run_meson_setup so we can drop it in this
case.
+ run_test
+}
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 :|
12:55 p.m.
New subject: [libvirt PATCH 10/33] ci: build.sh: Add a wrapper function over the 'potfile' job
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(a)redhat.com>
---
ci/build.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index b2891a0c0f..6990f2d171 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -82,3 +82,15 @@ 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_meson_setup
+ run_build
+}
--
2.41.0
11:55 a.m.
New subject: [libvirt PATCH 10/33] ci: build.sh: Add a wrapper function over the 'potfile' job
On Fri, Aug 25, 2023 at 07:55:18PM +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(a)redhat.com>
---
ci/build.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index b2891a0c0f..6990f2d171 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -82,3 +82,15 @@ 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_meson_setup
+ run_build
run_meson_setup is redundant here too as implied by run_build
Rather than -j1 could we instead invoke 'run_build' twice eg:
BUILD_ARGS=libvirt-pot-dep run_build
BUILD_ARGS=libvirt-pot run_build
+}
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 :|
2:24 a.m.
New subject: [libvirt PATCH 10/33] ci: build.sh: Add a wrapper function over the 'potfile' job
On Thu, Aug 31, 2023 at 05:55:21PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:18PM +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(a)redhat.com>
> ---
> ci/build.sh | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/ci/build.sh b/ci/build.sh
> index b2891a0c0f..6990f2d171 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -82,3 +82,15 @@ 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_meson_setup
> + run_build
run_meson_setup is redundant here too as implied by run_build
Rather than -j1 could we instead invoke 'run_build' twice eg:
BUILD_ARGS=libvirt-pot-dep run_build
BUILD_ARGS=libvirt-pot run_build
There was a reason why I did it that way, but I can't remember what it was.
Unless I manage to remember, I'll revert back to what you suggest.
Erik
12:55 p.m.
New subject: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 6990f2d171..30f4712e4b 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -94,3 +94,14 @@ run_potfile() {
run_meson_setup
run_build
}
+
+run_rpmbuild() {
+ local CMD="rpmbuild \
+ --clean \
+ --nodeps \
+ --define "_without_mingw 1" \
+ -ta build/meson-dist/libvirt-*.tar.xz"
+
+ run_meson_setup
+ run_dist
+}
--
2.41.0
11:55 a.m.
New subject: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 6990f2d171..30f4712e4b 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -94,3 +94,14 @@ run_potfile() {
run_meson_setup
run_build
}
+
+run_rpmbuild() {
+ local CMD="rpmbuild \
+ --clean \
+ --nodeps \
+ --define "_without_mingw 1" \
+ -ta build/meson-dist/libvirt-*.tar.xz"
+
+ run_meson_setup
Redundant here as implied by run_dist
+ run_dist
+}
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 :|
2:29 a.m.
New subject: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
On Thu, Aug 31, 2023 at 05:55:54PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
>
> Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> ---
> ci/build.sh | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/ci/build.sh b/ci/build.sh
> index 6990f2d171..30f4712e4b 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -94,3 +94,14 @@ run_potfile() {
> run_meson_setup
> run_build
> }
> +
> +run_rpmbuild() {
> + local CMD="rpmbuild \
> + --clean \
> + --nodeps \
> + --define "_without_mingw 1" \
> + -ta build/meson-dist/libvirt-*.tar.xz"
> +
> + run_meson_setup
Redundant here as implied by run_dist
While I can drop all these redundant setup calls, the reason why I put them
there was simply readability. Let's face it Shell isn't a particularly nice
structured language to look at especially when it comes to functions, so rather
than having everyone go and look what run_dist/run_build, etc. do, I instead
opted for transparent naming and explicit function calls, IOW so that when you
look at run_rpmbuild you immediately know we run meson setup followed by a
project build, etc., but then if you call run_dist alone in a local interactive
environment then run_dist would also call meson_setup because it can't go
without it, so hence the redundancy in all the functions.
If you still feel like it's undesirable even in ^this case, I will drop the
calls.
Erik
4:16 a.m.
New subject: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
On Fri, Sep 01, 2023 at 09:29:13AM +0200, Erik Skultety wrote:
On Thu, Aug 31, 2023 at 05:55:54PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> > This helper is a shell function transcript of its original GitLab CI
> > counterpart.
> >
> > Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> > ---
> > ci/build.sh | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/ci/build.sh b/ci/build.sh
> > index 6990f2d171..30f4712e4b 100644
> > --- a/ci/build.sh
> > +++ b/ci/build.sh
> > @@ -94,3 +94,14 @@ run_potfile() {
> > run_meson_setup
> > run_build
> > }
> > +
> > +run_rpmbuild() {
> > + local CMD="rpmbuild \
> > + --clean \
> > + --nodeps \
> > + --define "_without_mingw 1" \
> > + -ta build/meson-dist/libvirt-*.tar.xz"
> > +
> > + run_meson_setup
>
> Redundant here as implied by run_dist
While I can drop all these redundant setup calls, the reason why I put them
there was simply readability. Let's face it Shell isn't a particularly nice
structured language to look at especially when it comes to functions, so rather
than having everyone go and look what run_dist/run_build, etc. do, I instead
opted for transparent naming and explicit function calls, IOW so that when you
look at run_rpmbuild you immediately know we run meson setup followed by a
project build, etc., but then if you call run_dist alone in a local interactive
environment then run_dist would also call meson_setup because it can't go
without it, so hence the redundancy in all the functions.
If you still feel like it's undesirable even in ^this case, I will drop the
calls.
In practice I don't think users need to worry about what each
functions runs, because your implementation has ensured that
every single cmd is "self contained" and will run any dependancies
is has. Given that I think it is overkill to have the transitive
expansion of every dependancy, just the immediate dependency is
sufficient.
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 :|
4:05 a.m.
New subject: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 6990f2d171..30f4712e4b 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -94,3 +94,14 @@ run_potfile() {
run_meson_setup
run_build
}
+
+run_rpmbuild() {
+ local CMD="rpmbuild \
+ --clean \
+ --nodeps \
+ --define "_without_mingw 1" \
+ -ta build/meson-dist/libvirt-*.tar.xz"
+
+ run_meson_setup
+ run_dist
+}
--
2.41.0
Consider the following squashed in:
diff --git a/ci/build.sh b/ci/build.sh
index 5eef53f8d1..b990f5eeac 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -87,6 +87,7 @@ run_rpmbuild() {
run_meson_setup
run_dist
+ run_cmd "$CMD"
}
AND
diff --git a/ci/build.sh b/ci/build.sh
index b990f5eeac..a45bc2b110 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -82,7 +82,7 @@ run_rpmbuild() {
local CMD="rpmbuild \
--clean \
--nodeps \
- --define "_without_mingw 1" \
+ --define '_without_mingw 1' \
-ta build/meson-dist/libvirt-*.tar.xz"
run_meson_setup
Regards,
Erik
12:55 p.m.
New subject: [libvirt PATCH 12/33] ci: build.sh: Add a wrapper function over the 'website' job
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 30f4712e4b..6673ec6b18 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -105,3 +105,11 @@ run_rpmbuild() {
run_meson_setup
run_dist
}
+
+run_website_build() {
+ export DESTDIR="$(pwd)/install"
+ BUILD_ARGS="install-web"
+
+ run_meson_setup
+ run_build
+}
--
2.41.0
11:57 a.m.
New subject: [libvirt PATCH 12/33] ci: build.sh: Add a wrapper function over the 'website' job
On Fri, Aug 25, 2023 at 07:55:20PM +0200, Erik Skultety wrote:
This helper is a shell function transcript of its original GitLab CI
counterpart.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/ci/build.sh b/ci/build.sh
index 30f4712e4b..6673ec6b18 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -105,3 +105,11 @@ run_rpmbuild() {
run_meson_setup
run_dist
}
+
+run_website_build() {
+ export DESTDIR="$(pwd)/install"
Perhaps use ${GIT_ROOT} instead of $(pwd) to make this stable
regardless of pwd
+ BUILD_ARGS="install-web"
+
+ run_meson_setup
Redundant call
+ run_build
+}
--
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 :|
12:55 p.m.
New subject: [libvirt PATCH 13/33] ci: build.sh: Drop changing working directory to CI_CONT_DIR
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(a)redhat.com>
---
ci/build.sh | 2 --
1 file changed, 2 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index 6673ec6b18..133952f706 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
11:57 a.m.
New subject: [libvirt PATCH 13/33] ci: build.sh: Drop changing working directory to CI_CONT_DIR
On Fri, Aug 25, 2023 at 07:55:21PM +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(a)redhat.com>
---
ci/build.sh | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 14/33] ci: build.sh: Drop direct invocation of meson/ninja commands
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(a)redhat.com>
---
ci/build.sh | 5 -----
1 file changed, 5 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index 133952f706..b075c49af3 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -18,11 +18,6 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
MESON_ARGS="$MESON_ARGS $MESON_OPTS"
-meson setup build --werror -Dsystem=true $MESON_ARGS || \
-(cat build/meson-logs/meson-log.txt && exit 1)
-
-ninja -C build $NINJA_ARGS
-
run_cmd() {
local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
--
2.41.0
11:59 a.m.
New subject: [libvirt PATCH 14/33] ci: build.sh: Drop direct invocation of meson/ninja commands
On Fri, Aug 25, 2023 at 07:55:22PM +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(a)redhat.com>
---
ci/build.sh | 5 -----
1 file changed, 5 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index 133952f706..b075c49af3 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -18,11 +18,6 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
MESON_ARGS="$MESON_ARGS $MESON_OPTS"
-meson setup build --werror -Dsystem=true $MESON_ARGS || \
-(cat build/meson-logs/meson-log.txt && exit 1)
-
-ninja -C build $NINJA_ARGS
-
run_cmd() {
local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
Now we drop immediate invokation at time of execution, I wonder if the
build.sh name is a little mis-leading.
Might be better renamed to 'functions.sh' or 'commands.sh' perhaps, so
the name doesn't suggest that it actually builds stuff, merely that it
supplies some shell logic ?
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 :|
2:30 a.m.
New subject: [libvirt PATCH 14/33] ci: build.sh: Drop direct invocation of meson/ninja commands
On Thu, Aug 31, 2023 at 05:59:26PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:22PM +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(a)redhat.com>
> ---
> ci/build.sh | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/ci/build.sh b/ci/build.sh
> index 133952f706..b075c49af3 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -18,11 +18,6 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
>
> MESON_ARGS="$MESON_ARGS $MESON_OPTS"
>
> -meson setup build --werror -Dsystem=true $MESON_ARGS || \
> -(cat build/meson-logs/meson-log.txt && exit 1)
> -
> -ninja -C build $NINJA_ARGS
> -
> run_cmd() {
> local CMD="$(echo $CMD | tr -s ' ')" # truncate any
additional spaces
>
Now we drop immediate invokation at time of execution, I wonder if the
build.sh name is a little mis-leading.
Might be better renamed to 'functions.sh' or 'commands.sh' perhaps, so
the name doesn't suggest that it actually builds stuff, merely that it
supplies some shell logic ?
Fair enough, didn't like the name either, but I wanted to keep at least
something we had around :).
Erik
12:55 p.m.
New subject: [libvirt PATCH 15/33] ci: build.sh: Drop MESON_ARGS definition from global level
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index b075c49af3..5eef53f8d1 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -8,16 +8,6 @@ export VIR_TEST_VERBOSE="1"
export VIR_TEST_DEBUG="1"
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() {
local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional
spaces
--
2.41.0
12:01 p.m.
New subject: [libvirt PATCH 15/33] ci: build.sh: Drop MESON_ARGS definition from global level
On Fri, Aug 25, 2023 at 07:55:23PM +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/build.sh | 10 ----------
1 file changed, 10 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 16/33] gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection
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(a)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
9:32 a.m.
New subject: [libvirt PATCH 16/33] gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection
On Fri, Aug 25, 2023 at 07:55:24PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
.gitlab-ci.yml | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 17/33] .gitlab-ci.yml: Convert the native build job to the build.sh usage
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(a)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..6a8115037d 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/build.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
12:11 p.m.
New subject: [libvirt PATCH 17/33] .gitlab-ci.yml: Convert the native build job to the build.sh usage
On Fri, Aug 25, 2023 at 07:55:25PM +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(a)redhat.com>
---
.gitlab-ci.yml | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 18/33] .gitlab-ci.yml: Convert the cross build job to the build.sh usage
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(a)redhat.com>
---
.gitlab-ci.yml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6a8115037d..b59e56941c 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/build.sh
+ - run_build
+ - if test "$CROSS" = "i686" ;
+ then
+ run_test;
+ fi
.cross_build_job_prebuilt_env:
extends:
--
2.41.0
12:11 p.m.
New subject: [libvirt PATCH 18/33] .gitlab-ci.yml: Convert the cross build job to the build.sh usage
On Fri, Aug 25, 2023 at 07:55:26PM +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(a)redhat.com>
---
.gitlab-ci.yml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 19/33] .gitlab-ci.yml: Convert the website build job to the build.sh usage
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(a)redhat.com>
---
.gitlab-ci.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b59e56941c..0450c1f6ad 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -79,9 +79,8 @@ include:
# https://gitlab.com/libvirt/libvirt/-/jobs/artifacts/master/download?job=w...
.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/build.sh
+ - run_website_build
after_script:
- test "$CI_JOB_STATUS" != "success" && exit 1;
- mv install/usr/share/doc/libvirt/html/ website
--
2.41.0
12:11 p.m.
New subject: [libvirt PATCH 19/33] .gitlab-ci.yml: Convert the website build job to the build.sh usage
On Fri, Aug 25, 2023 at 07:55:27PM +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(a)redhat.com>
---
.gitlab-ci.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 20/33] .gitlab-ci.yml: Convert the codestyle job to the build.sh usage
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(a)redhat.com>
---
.gitlab-ci.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0450c1f6ad..0ed6504dc6 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/build.sh
+ - run_codestyle
codestyle_prebuilt_env:
extends:
--
2.41.0
12:12 p.m.
New subject: [libvirt PATCH 20/33] .gitlab-ci.yml: Convert the codestyle job to the build.sh usage
On Fri, Aug 25, 2023 at 07:55:28PM +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(a)redhat.com>
---
.gitlab-ci.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 21/33] .gitlab-ci.yml: Convert the potfile job to the build.sh usage
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(a)redhat.com>
---
.gitlab-ci.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0ed6504dc6..3efde7098a 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/build.sh
+ - run_potfile
after_script:
- test "$CI_JOB_STATUS" != "success" && exit 1;
- cp po/libvirt.pot libvirt.pot
--
2.41.0
12:13 p.m.
New subject: [libvirt PATCH 21/33] .gitlab-ci.yml: Convert the potfile job to the build.sh usage
On Fri, Aug 25, 2023 at 07:55:29PM +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(a)redhat.com>
---
.gitlab-ci.yml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 22/33] ci: helper: Drop _lcitool_get_targets method
This method unused anywhere, so drop it.
Signed-off-by: Erik Skultety <eskultet(a)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
12:14 p.m.
New subject: [libvirt PATCH 22/33] ci: helper: Drop _lcitool_get_targets method
On Fri, Aug 25, 2023 at 07:55:30PM +0200, Erik Skultety wrote:
This method unused anywhere, so drop it.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/helper | 4 ----
1 file changed, 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 23/33] ci: helper: Don't make ':' literal a static part of the image tag
':' 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(a)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
12:15 p.m.
New subject: [libvirt PATCH 23/33] ci: helper: Don't make ':' literal a static part of the image tag
On Fri, Aug 25, 2023 at 07:55:31PM +0200, Erik Skultety wrote:
':' 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(a)redhat.com>
---
ci/helper | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 24/33] ci: helper: Add --lcitool-path CLI option
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(a)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
12:16 p.m.
New subject: [libvirt PATCH 24/33] ci: helper: Add --lcitool-path CLI option
On Fri, Aug 25, 2023 at 07:55:32PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 6 ++++++
1 file changed, 6 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 25/33] ci: helper: Add a job argparse subparser
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(a)redhat.com>
---
ci/helper | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/ci/helper b/ci/helper
index 75552774f6..60fdaed5f4 100755
--- a/ci/helper
+++ b/ci/helper
@@ -114,6 +114,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
12:21 p.m.
New subject: [libvirt PATCH 25/33] ci: helper: Add a job argparse subparser
On Fri, Aug 25, 2023 at 07:55:33PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 26/33] ci: helper: Add a required_deps higher order helper/decorator
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(a)redhat.com>
---
ci/helper | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/ci/helper b/ci/helper
index 60fdaed5f4..780f4161f5 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
12:21 p.m.
New subject: [libvirt PATCH 26/33] ci: helper: Add a required_deps higher order helper/decorator
On Fri, Aug 25, 2023 at 07:55:34PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 27/33] ci: helper: Add Python code hangling git clones
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(a)redhat.com>
---
ci/helper | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/helper b/ci/helper
index 780f4161f5..2a51ce1786 100755
--- a/ci/helper
+++ b/ci/helper
@@ -174,9 +174,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
12:21 p.m.
New subject: [libvirt PATCH 27/33] ci: helper: Add Python code hangling git clones
On Fri, Aug 25, 2023 at 07:55:35PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 9 +++++++++
1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 28/33] ci: helper: Add a helper to create a local repo clone Pythonic way
A proper Python equivalent of 'git clone --local'.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/helper | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/ci/helper b/ci/helper
index 2a51ce1786..d704d1f711 100755
--- a/ci/helper
+++ b/ci/helper
@@ -211,6 +211,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
12:20 p.m.
New subject: [libvirt PATCH 28/33] ci: helper: Add a helper to create a local repo clone Pythonic way
On Fri, Aug 25, 2023 at 07:55:36PM +0200, Erik Skultety wrote:
A proper Python equivalent of 'git clone --local'.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
ci/helper | 4 ++++
1 file changed, 4 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 29/33] ci: helper: Rework _lcitool_run method logic
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(a)redhat.com>
---
ci/helper | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/ci/helper b/ci/helper
index d704d1f711..887e64ece1 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
@@ -216,8 +219,51 @@ 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/build.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}")
+ subprocess.run([self._args.lcitool] + positional_args + opts)
+
+ # this will take care of the generated script file above as well
+ tmpdir.cleanup()
def _check_stale_images(self):
namespace = self._args.namespace
--
2.41.0
12:20 p.m.
New subject: [libvirt PATCH 29/33] ci: helper: Rework _lcitool_run method logic
On Fri, Aug 25, 2023 at 07:55:37PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 30/33] ci: helper: Add an action to run the container workload via lcitool
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(a)redhat.com>
---
ci/helper | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/ci/helper b/ci/helper
index 887e64ece1..7385d3b95c 100755
--- a/ci/helper
+++ b/ci/helper
@@ -305,6 +305,10 @@ class Application:
def _action_shell(self):
self._make_run(f"ci-shell(a){self._args.target}")
+ @required_deps("git")
+ def _action_run(self):
+ 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
12:20 p.m.
New subject: [libvirt PATCH 30/33] ci: helper: Add an action to run the container workload via lcitool
On Fri, Aug 25, 2023 at 07:55:38PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 4 ++++
1 file changed, 4 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 31/33] ci: helper: Drop original actions
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(a)redhat.com>
---
ci/helper | 36 ------------------------------------
1 file changed, 36 deletions(-)
diff --git a/ci/helper b/ci/helper
index 7385d3b95c..07e449e59f 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",
@@ -296,15 +269,6 @@ class Application:
""")
print(msg.replace("STALE_DETAILS", stale_details))
- def _action_build(self):
- self._make_run(f"ci-build(a){self._args.target}")
-
- def _action_test(self):
- self._make_run(f"ci-test(a){self._args.target}")
-
- def _action_shell(self):
- self._make_run(f"ci-shell(a){self._args.target}")
-
@required_deps("git")
def _action_run(self):
self._lcitool_run(self._args.job)
--
2.41.0
12:18 p.m.
New subject: [libvirt PATCH 31/33] ci: helper: Drop original actions
On Fri, Aug 25, 2023 at 07:55:39PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 36 ------------------------------------
1 file changed, 36 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 32/33] ci: helper: Drop the --meson-args/--ninja-args CLI options
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(a)redhat.com>
---
ci/helper | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/ci/helper b/ci/helper
index 07e449e59f..0053fce40e 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
12:18 p.m.
New subject: [libvirt PATCH 32/33] ci: helper: Drop the --meson-args/--ninja-args CLI options
On Fri, Aug 25, 2023 at 07:55:40PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 15 ---------------
1 file changed, 15 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
12:55 p.m.
New subject: [libvirt PATCH 33/33] ci: helper: Drop the _make_run method
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(a)redhat.com>
---
ci/helper | 25 -------------------------
1 file changed, 25 deletions(-)
diff --git a/ci/helper b/ci/helper
index 0053fce40e..e45eb63016 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
12:19 p.m.
New subject: [libvirt PATCH 33/33] ci: helper: Drop the _make_run method
On Fri, Aug 25, 2023 at 07:55:41PM +0200, Erik Skultety wrote:
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(a)redhat.com>
---
ci/helper | 25 -------------------------
1 file changed, 25 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)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 :|
11:17 a.m.
On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
Technically a v2 of:
https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
However, the approach here is slightly different and what that series said
about migration to lcitool container executions as a replacement for
ci/Makefile is actually done here. One of the core problems of the above
pointed out in review was that more Shell logic was introduced including CLI
parsing, conditional executions, etc. which we fought hard to get rid of in the
past. I reworked the Shell functions quite a bit and dropped whatever extra
Shell logic the original series added.
Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and so
I merely extracted the recipes into functions which are then sourced as
ci/build.sh and executed. Now, that on its own would hide the actual commands
being run in the GitLab job log, so before any command is actually executed, it
is formatted with a color sequence so we don't miss that information as that
would be a regression to the status quo.
Lastly, this series then takes the effort inside the ci/build.sh script and
basically mirrors whatever GitLab would do to run a job inside a local
container which is executed by lcitool (yes, we already have that capability).
Please give this a try and I'm already looking forward to comments as I'd like
to expand this effort to local VM executions running the TCK integration tests,
so this series is quite important in that regard.
Do you have a gitlab branch with this contnt somewhere. When i tried to
apply the patches to current git, it was unhappy on the 3rd patch
$ git am -3 ~/cibuild
Applying: ci: build.sh: Add variables from .gitlab-ci.yml
Applying: ci: build.sh: Add GIT_ROOT env helper variable
Applying: ci: build.sh: Don't mention that MESON_ARGS are available via CLI
error: sha1 information is lacking or useless (ci/build.sh).
error: could not build fake ancestor
Patch failed at 0003 ci: build.sh: Don't mention that MESON_ARGS are available via
CLI
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
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 :|
4:21 a.m.
On Thu, Aug 31, 2023 at 05:17:13PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
>
> However, the approach here is slightly different and what that series said
> about migration to lcitool container executions as a replacement for
> ci/Makefile is actually done here. One of the core problems of the above
> pointed out in review was that more Shell logic was introduced including CLI
> parsing, conditional executions, etc. which we fought hard to get rid of in the
> past. I reworked the Shell functions quite a bit and dropped whatever extra
> Shell logic the original series added.
> Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and so
> I merely extracted the recipes into functions which are then sourced as
> ci/build.sh and executed. Now, that on its own would hide the actual commands
> being run in the GitLab job log, so before any command is actually executed, it
> is formatted with a color sequence so we don't miss that information as that
> would be a regression to the status quo.
>
> Lastly, this series then takes the effort inside the ci/build.sh script and
> basically mirrors whatever GitLab would do to run a job inside a local
> container which is executed by lcitool (yes, we already have that capability).
>
> Please give this a try and I'm already looking forward to comments as I'd
like
> to expand this effort to local VM executions running the TCK integration tests,
> so this series is quite important in that regard.
Do you have a gitlab branch with this contnt somewhere. When i tried to
apply the patches to current git, it was unhappy on the 3rd patch
$ git am -3 ~/cibuild
Applying: ci: build.sh: Add variables from .gitlab-ci.yml
Applying: ci: build.sh: Add GIT_ROOT env helper variable
Applying: ci: build.sh: Don't mention that MESON_ARGS are available via CLI
error: sha1 information is lacking or useless (ci/build.sh).
error: could not build fake ancestor
Patch failed at 0003 ci: build.sh: Don't mention that MESON_ARGS are available via
CLI
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Yeah, I had a different merge conflict locally, but I guess both related to
b68d253c4603a041cb993fa133ba25e610a78929 not having been pushed yet and the
time I posted the series.
Here's the fixed branch:
https://gitlab.com/eskultety/libvirt/-/commits/helper-lcitool
And here's a pipeline (with the fix posted for patch 11):
https://gitlab.com/eskultety/libvirt/-/pipelines/988871113
Erik
4:26 a.m.
On Fri, Sep 01, 2023 at 11:21:04AM +0200, Erik Skultety wrote:
On Thu, Aug 31, 2023 at 05:17:13PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> > Technically a v2 of:
> > https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
> >
> > However, the approach here is slightly different and what that series said
> > about migration to lcitool container executions as a replacement for
> > ci/Makefile is actually done here. One of the core problems of the above
> > pointed out in review was that more Shell logic was introduced including CLI
> > parsing, conditional executions, etc. which we fought hard to get rid of in
the
> > past. I reworked the Shell functions quite a bit and dropped whatever extra
> > Shell logic the original series added.
> > Obviously we can't get rid of Shell completely because of .gitlab-ci.yml
and so
> > I merely extracted the recipes into functions which are then sourced as
> > ci/build.sh and executed. Now, that on its own would hide the actual commands
> > being run in the GitLab job log, so before any command is actually executed,
it
> > is formatted with a color sequence so we don't miss that information as
that
> > would be a regression to the status quo.
> >
> > Lastly, this series then takes the effort inside the ci/build.sh script and
> > basically mirrors whatever GitLab would do to run a job inside a local
> > container which is executed by lcitool (yes, we already have that capability).
> >
> > Please give this a try and I'm already looking forward to comments as
I'd like
> > to expand this effort to local VM executions running the TCK integration
tests,
> > so this series is quite important in that regard.
>
> Do you have a gitlab branch with this contnt somewhere. When i tried to
> apply the patches to current git, it was unhappy on the 3rd patch
>
> $ git am -3 ~/cibuild
> Applying: ci: build.sh: Add variables from .gitlab-ci.yml
> Applying: ci: build.sh: Add GIT_ROOT env helper variable
> Applying: ci: build.sh: Don't mention that MESON_ARGS are available via CLI
> error: sha1 information is lacking or useless (ci/build.sh).
> error: could not build fake ancestor
> Patch failed at 0003 ci: build.sh: Don't mention that MESON_ARGS are available
via CLI
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
Yeah, I had a different merge conflict locally, but I guess both related to
b68d253c4603a041cb993fa133ba25e610a78929 not having been pushed yet and the
time I posted the series.
Yeah, I eventually figured out b68d253c4603a041cb993fa133ba25e610a78929 was
the problem and applied your series before that commit to test it.
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 :|
12:28 p.m.
On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
Technically a v2 of:
https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
However, the approach here is slightly different and what that series said
about migration to lcitool container executions as a replacement for
ci/Makefile is actually done here. One of the core problems of the above
pointed out in review was that more Shell logic was introduced including CLI
parsing, conditional executions, etc. which we fought hard to get rid of in the
past. I reworked the Shell functions quite a bit and dropped whatever extra
Shell logic the original series added.
Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and so
I merely extracted the recipes into functions which are then sourced as
ci/build.sh and executed. Now, that on its own would hide the actual commands
being run in the GitLab job log, so before any command is actually executed, it
is formatted with a color sequence so we don't miss that information as that
would be a regression to the status quo.
Lastly, this series then takes the effort inside the ci/build.sh script and
basically mirrors whatever GitLab would do to run a job inside a local
container which is executed by lcitool (yes, we already have that capability).
Please give this a try and I'm already looking forward to comments as I'd like
to expand this effort to local VM executions running the TCK integration tests,
so this series is quite important in that regard.
Overall I'm fine with what's proposed here.
Two general thoughts
* ci/Makefile appears pretty much redundant - ci/helper can provide
the same level of functionality AFAICT, and it'd be nice to kill
an outstanding usage of 'make' given our goal to standardize on
meson + python
* ci/helper looks almost entirely independent of libvirt, aside
from the list of 'choices' for the --job arg, and the --namespace
arg default value, it would work with any virt project we have if
the project created its own ci/build.sh file
Can we fold all its logic into lcitool, and just have that as
the entrypoint ? In ci/manifest.yml we can get the project
namespace, and we could possibly just extra the commands by
crudely regex matching 'ci/build.sh' content against the
pattern '^run_.*\(\)$ {'
The removal of ci/Makefil feels like it could be done in this series,
but its fine if the ci/helper suggestion is left as separate future
work.
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 :|
3:16 a.m.
On Thu, Aug 31, 2023 at 06:28:16PM +0100, Daniel P. Berrangé wrote:
On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
>
> However, the approach here is slightly different and what that series said
> about migration to lcitool container executions as a replacement for
> ci/Makefile is actually done here. One of the core problems of the above
> pointed out in review was that more Shell logic was introduced including CLI
> parsing, conditional executions, etc. which we fought hard to get rid of in the
> past. I reworked the Shell functions quite a bit and dropped whatever extra
> Shell logic the original series added.
> Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and so
> I merely extracted the recipes into functions which are then sourced as
> ci/build.sh and executed. Now, that on its own would hide the actual commands
> being run in the GitLab job log, so before any command is actually executed, it
> is formatted with a color sequence so we don't miss that information as that
> would be a regression to the status quo.
>
> Lastly, this series then takes the effort inside the ci/build.sh script and
> basically mirrors whatever GitLab would do to run a job inside a local
> container which is executed by lcitool (yes, we already have that capability).
>
> Please give this a try and I'm already looking forward to comments as I'd
like
> to expand this effort to local VM executions running the TCK integration tests,
> so this series is quite important in that regard.
Overall I'm fine with what's proposed here.
Two general thoughts
* ci/Makefile appears pretty much redundant - ci/helper can provide
the same level of functionality AFAICT, and it'd be nice to kill
an outstanding usage of 'make' given our goal to standardize on
meson + python
Huh, the fact that removal of Makefile isn't part of this series is a mistake
on my side - I worked on this on 2 parallel branches trying out 2 slightly
different approaches. I did drop it on one branch but not the other which I
ultimately decided to go with. Since I'll be sending a v2, I'll add that patch.
* ci/helper looks almost entirely independent of libvirt, aside
from the list of 'choices' for the --job arg, and the --namespace
arg default value, it would work with any virt project we have if
the project created its own ci/build.sh file
Can we fold all its logic into lcitool, and just have that as
the entrypoint ? In ci/manifest.yml we can get the project
namespace, and we could possibly just extra the commands by
crudely regex matching 'ci/build.sh' content against the
pattern '^run_.*\(\)$ {'
Technically we could. Extracting the code and injecting it into lcitool is not
a problem, in fact, it would be quite straight forward. The problem is
designing a CLI interface that would make sense for the use case without
breaking the existing one too much. Ideally by introducing just a bunch of
optional args which I don't think is very realistic. Since that will require
thorough thinking and designing I did not want to dive right into that as I
wasn't even sure whether I'd be able to push this conversion through upstream.
The removal of ci/Makefil feels like it could be done in this series,
but its fine if the ci/helper suggestion is left as separate future
work.
Yeah, like I said above, incorporating ci/helper into lcitool is likely going
to be again one of the bigger overhauls so that will be a project on its own.
Thanks for the comments, much appreciated.
Erik
449
days inactive
456
days old
83 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Erik Skultety