[libvirt PATCH 0/2] ci: Call meson consistently
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/276971193 Andrea Bolognani (2): ci: Don't use --prefix with meson for Cirrus CI builds ci: Call meson consistently .gitlab-ci.yml | 8 ++++---- ci/cirrus/build.yml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) -- 2.26.3
It's no longer used. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- ci/cirrus/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/cirrus/build.yml b/ci/cirrus/build.yml index a6b13b947c..2d3c46a77c 100644 --- a/ci/cirrus/build.yml +++ b/ci/cirrus/build.yml @@ -20,5 +20,5 @@ build_task: - git fetch origin "$CI_COMMIT_REF_NAME" - git reset --hard "$CI_COMMIT_SHA" build_script: - - meson build --prefix=$(pwd)/install-root + - meson build - ninja -C build dist -- 2.26.3
On Fri, Mar 26, 2021 at 11:35:01AM +0100, Andrea Bolognani wrote:
It's no longer used.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
We should always pass --werror and display the contents of the log file in case of failure. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cbc1292839..8b7df68f47 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -529,7 +529,7 @@ website: before_script: - *script_variables script: - - meson build --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt && exit 1) + - meson build --werror --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt && exit 1) - ninja -C build install-web - mv vroot/share/doc/libvirt/html/ website artifacts: @@ -549,7 +549,7 @@ codestyle: before_script: - *script_variables script: - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) + - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - ninja -C build libvirt-pot-dep - meson test -C build --suite syntax-check --no-rebuild || (cat build/meson-logs/testlog.txt && exit 1) @@ -567,7 +567,7 @@ potfile: before_script: - *script_variables script: - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) + - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - ninja -C build libvirt-pot-dep - ninja -C build libvirt-pot - cp po/libvirt.pot libvirt.pot @@ -605,7 +605,7 @@ coverity: script: - curl https://scan.coverity.com/download/linux64 --form project=$COVERITY_SCAN_PROJECT_NAME --form token=$COVERITY_SCAN_TOKEN -o /tmp/cov-analysis-linux64.tgz - tar xfz /tmp/cov-analysis-linux64.tgz - - meson build + - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) - cov-analysis-linux64-*/bin/cov-build --dir cov-int ninja -C build - tar cfz cov-int.tar.gz cov-int - curl https://scan.coverity.com/builds?project=$COVERITY_SCAN_PROJECT_NAME --form token=$COVERITY_SCAN_TOKEN --form email=$GITLAB_USER_EMAIL --form file=@cov-int.tar.gz --form version="$(git describe --tags)" --form description="$(git describe --tags) / $CI_COMMIT_TITLE / $CI_COMMIT_REF_NAME:$CI_PIPELINE_ID" -- 2.26.3
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Fri, 2021-03-26 at 17:00 +0100, Erik Skultety wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Even though you didn't spell that out explicitly, I assume you're okay with me pushing this series during the freeze? -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Mar 26, 2021 at 05:11:04PM +0100, Andrea Bolognani wrote:
On Fri, 2021-03-26 at 17:00 +0100, Erik Skultety wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Even though you didn't spell that out explicitly, I assume you're okay with me pushing this series during the freeze?
I know it's trivial and most likely will not affect the release at all but my understanding of freeze is that we try to stabilize the build by fixing bugs or any issues that would prevent us to deliver the final release. To me that sounds like reasonable definition that we should try to follow and in my mind creates a clear decision process if something should/can be pushed during freeze. Based on that definition I would say that these patches does not qualify and can easily wait once the release is done. Pavel
On Fri, 2021-03-26 at 18:55 +0100, Pavel Hrdina wrote:
On Fri, Mar 26, 2021 at 05:11:04PM +0100, Andrea Bolognani wrote:
Even though you didn't spell that out explicitly, I assume you're okay with me pushing this series during the freeze?
I know it's trivial and most likely will not affect the release at all but my understanding of freeze is that we try to stabilize the build by fixing bugs or any issues that would prevent us to deliver the final release.
To me that sounds like reasonable definition that we should try to follow and in my mind creates a clear decision process if something should/can be pushed during freeze.
Based on that definition I would say that these patches does not qualify and can easily wait once the release is done.
Different people seem to have different understanding of what does and does not qualify for merging during the freeze: in the past, I've argued for the same interpretation that you're offering right now, only to receive significant pushback ¯\_(ツ)_/¯ That said, I agree with both you and Erik that there is basically no reason for this *not* to wait until 7.2.0 is out, so I'll just play it safe and do just that :) -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Mar 29, 2021 at 10:17:51AM +0200, Andrea Bolognani wrote:
On Fri, 2021-03-26 at 18:55 +0100, Pavel Hrdina wrote:
On Fri, Mar 26, 2021 at 05:11:04PM +0100, Andrea Bolognani wrote:
Even though you didn't spell that out explicitly, I assume you're okay with me pushing this series during the freeze?
I know it's trivial and most likely will not affect the release at all but my understanding of freeze is that we try to stabilize the build by fixing bugs or any issues that would prevent us to deliver the final release.
To me that sounds like reasonable definition that we should try to follow and in my mind creates a clear decision process if something should/can be pushed during freeze.
Based on that definition I would say that these patches does not qualify and can easily wait once the release is done.
Different people seem to have different understanding of what does and does not qualify for merging during the freeze: in the past, I've argued for the same interpretation that you're offering right now, only to receive significant pushback ¯\_(ツ)_/¯
That said, I agree with both you and Erik that there is basically no reason for this *not* to wait until 7.2.0 is out, so I'll just play it safe and do just that :)
The wording I remember was that during freeze we can push patches that fix bugs, alter documentation, or have been already posted (with an extra caution). There was no CI back then. Maybe we need to adjust that for the future somehow, but given the fact that this was not written down anywhere I do not know how to do that officially.
-- Andrea Bolognani / Red Hat / Virtualization
On Fri, Mar 26, 2021 at 05:11:04PM +0100, Andrea Bolognani wrote:
On Fri, 2021-03-26 at 17:00 +0100, Erik Skultety wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Even though you didn't spell that out explicitly, I assume you're okay with me pushing this series during the freeze?
I agree with Pavel here. Even though this is far from an intrusive change, unless it's fixing a bug, we should keep our CI unchanged during the freeze period in order to focus on stabilizing any failures before the release. Erik
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
As is often the case, the raisins are mostly hysterical ;) I think the initial idea was to keep things as simple as possible, but I agree with you that when you have seven (!) build recipes, most of which are almost identical, it makes sense to think about consolidating them to a single location. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Apr 01, 2021 at 03:43:11PM +0200, Andrea Bolognani wrote:
On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
As is often the case, the raisins are mostly hysterical ;)
I think the initial idea was to keep things as simple as possible, but I agree with you that when you have seven (!) build recipes, most of which are almost identical, it makes sense to think about consolidating them to a single location.
So I took a look at the consolidation. I went through few iterations after which I ended with two scripts, one that does the meson part and one that does the ninja part. The first one does not need to take any parameters, the other one would be nice if it took one, that's enough. What's weird about it is the way I think of it. The most understandable way would be a `configure` that runs `meson ...` and then another one, which runs `ninja ...` with parameter(s). At that point you can make the second one a Makefile and just do: ./configure make most of the time. But because we can just put the configure part into the makefile we can also just have a makefile that does all of it. It might sound stupid, but it is the easiest way how to provide few targets without extra scripting and everything around it. On the other hand it feels weird just suggesting that, although I cannot pinpoint why that is. Maybe that it would be too controversial? At this point I honestly do not know whether I went too far or it actually makes sense and I am not capable of recognising the difference. Thoughts?
-- Andrea Bolognani / Red Hat / Virtualization
On Thu, Apr 08, 2021 at 12:32:10PM +0200, Martin Kletzander wrote:
On Thu, Apr 01, 2021 at 03:43:11PM +0200, Andrea Bolognani wrote:
On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
As is often the case, the raisins are mostly hysterical ;)
I think the initial idea was to keep things as simple as possible, but I agree with you that when you have seven (!) build recipes, most of which are almost identical, it makes sense to think about consolidating them to a single location.
So I took a look at the consolidation. I went through few iterations after which I ended with two scripts, one that does the meson part and one that does the ninja part. The first one does not need to take any parameters, the other one would be nice if it took one, that's enough. What's weird about it is the way I think of it. The most understandable way would be a `configure` that runs `meson ...` and then another one, which runs `ninja ...` with parameter(s).
At that point you can make the second one a Makefile and just do:
./configure make
most of the time. But because we can just put the configure part into the makefile we can also just have a makefile that does all of it. It might sound stupid, but it is the easiest way how to provide few targets without extra scripting and everything around it. On the other hand it feels weird just suggesting that, although I cannot pinpoint why that is. Maybe that it would be too controversial?
At this point I honestly do not know whether I went too far or it actually makes sense and I am not capable of recognising the difference.
I've not looked back at the full thread, but I'd say that if the answer to any problem involves the words "configure" or "make", then we're likely approaching the problem in the wrong way, because we're really supposed to have removed use of make and autotools (or anything that pretends to look like a configure script), Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 08, 2021 at 11:39:55AM +0100, Daniel P. Berrangé wrote:
On Thu, Apr 08, 2021 at 12:32:10PM +0200, Martin Kletzander wrote:
On Thu, Apr 01, 2021 at 03:43:11PM +0200, Andrea Bolognani wrote:
On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
As is often the case, the raisins are mostly hysterical ;)
I think the initial idea was to keep things as simple as possible, but I agree with you that when you have seven (!) build recipes, most of which are almost identical, it makes sense to think about consolidating them to a single location.
So I took a look at the consolidation. I went through few iterations after which I ended with two scripts, one that does the meson part and one that does the ninja part. The first one does not need to take any parameters, the other one would be nice if it took one, that's enough. What's weird about it is the way I think of it. The most understandable way would be a `configure` that runs `meson ...` and then another one, which runs `ninja ...` with parameter(s).
At that point you can make the second one a Makefile and just do:
./configure make
most of the time. But because we can just put the configure part into the makefile we can also just have a makefile that does all of it. It might sound stupid, but it is the easiest way how to provide few targets without extra scripting and everything around it. On the other hand it feels weird just suggesting that, although I cannot pinpoint why that is. Maybe that it would be too controversial?
At this point I honestly do not know whether I went too far or it actually makes sense and I am not capable of recognising the difference.
I've not looked back at the full thread, but I'd say that if the answer to any problem involves the words "configure" or "make", then we're likely approaching the problem in the wrong way, because we're really supposed to have removed use of make and autotools (or anything that pretends to look like a configure script),
It's only the name that suggested this, it could've been a `ci/build_script.sh`, I just used the name because it, of course, reminded me of the old ways. Basically the whole point of this is to have the line: meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1) only written in one place, and possibly similarly with some ninja invocations. It would be almost exclusively used by the CI, although it could be also useful for some people.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 08, 2021 at 12:56:05PM +0200, Martin Kletzander wrote:
On Thu, Apr 08, 2021 at 11:39:55AM +0100, Daniel P. Berrangé wrote:
On Thu, Apr 08, 2021 at 12:32:10PM +0200, Martin Kletzander wrote:
On Thu, Apr 01, 2021 at 03:43:11PM +0200, Andrea Bolognani wrote:
On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
We should always pass --werror and display the contents of the log file in case of failure.
Any reason why the lines are not in one place? What I did in libnbd (first draft, still up for review) is that I just took all the lines of the script and put them inside `ci/build_script.sh` with only the most basic conditionals to accommodate various types of runs. That way common things are in one place. It could take a parameter (like what ninja target to run) and you can run things after that (like `website` and `potfiles` jobs do. Just an idea.
As is often the case, the raisins are mostly hysterical ;)
I think the initial idea was to keep things as simple as possible, but I agree with you that when you have seven (!) build recipes, most of which are almost identical, it makes sense to think about consolidating them to a single location.
So I took a look at the consolidation. I went through few iterations after which I ended with two scripts, one that does the meson part and one that does the ninja part. The first one does not need to take any parameters, the other one would be nice if it took one, that's enough. What's weird about it is the way I think of it. The most understandable way would be a `configure` that runs `meson ...` and then another one, which runs `ninja ...` with parameter(s).
At that point you can make the second one a Makefile and just do:
./configure make
most of the time. But because we can just put the configure part into the makefile we can also just have a makefile that does all of it. It might sound stupid, but it is the easiest way how to provide few targets without extra scripting and everything around it. On the other hand it feels weird just suggesting that, although I cannot pinpoint why that is. Maybe that it would be too controversial?
At this point I honestly do not know whether I went too far or it actually makes sense and I am not capable of recognising the difference.
I've not looked back at the full thread, but I'd say that if the answer to any problem involves the words "configure" or "make", then we're likely approaching the problem in the wrong way, because we're really supposed to have removed use of make and autotools (or anything that pretends to look like a configure script),
It's only the name that suggested this, it could've been a `ci/build_script.sh`, I just used the name because it, of course, reminded me of the old ways.
Basically the whole point of this is to have the line:
meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1)
only written in one place, and possibly similarly with some ninja invocations. It would be almost exclusively used by the CI, although it could be also useful for some people.
Our ci/build.sh & ci/Makefile commands probably don't belong here at all. If we think about it they're just trying to provide a simple way to run a build inside a container, and this logic works for essentially any app that uses meson. This suggests it probably doesn't belong in libvirt.git. Instead we might want to think about doing this in lcitool, so it can be used from any of our repos that use meson. Most of the logic will work fine for other build systems too. IOW, instead of using a makefile to provide make ci-build@fedora-33 we could just let people run lcitool build -c fedora-33 where -c is shorthand for --container, and could have -v / --vm for doing the VM builds. 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 :|
participants (5)
-
Andrea Bolognani -
Daniel P. Berrangé -
Erik Skultety -
Martin Kletzander -
Pavel Hrdina