[libvirt PATCH] ci: Makefile: Add distcheck target

From time to time we see failures in our gitlab pipelines that we as developers don't see locally, simply because we don't run "make distcheck" that often which is what the gitlab containers run and which can indeed reveal mistakes like not shipping the necessary test data. Since most of us prefer local testing over waiting for the gitlab pipeline to finish, introduce the distcheck target to the CI which we have inside the repo under ci/.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/Makefile b/ci/Makefile index 7840cc8d89..e1c626ed34 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -239,6 +239,9 @@ ci-build@%: ci-check@%: $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check" +ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck" + ci-list-images: @echo @echo "Available x86 container images:" -- 2.26.2

On 6/17/20 12:00 PM, Erik Skultety wrote:
From time to time we see failures in our gitlab pipelines that we as developers don't see locally, simply because we don't run "make distcheck" that often which is what the gitlab containers run and which can indeed reveal mistakes like not shipping the necessary test data. Since most of us prefer local testing over waiting for the gitlab pipeline to finish, introduce the distcheck target to the CI which we have inside the repo under ci/.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/ci/Makefile b/ci/Makefile index 7840cc8d89..e1c626ed34 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -239,6 +239,9 @@ ci-build@%: ci-check@%: $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
+ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck" + ci-list-images: @echo @echo "Available x86 container images:"
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On Wed, 2020-06-17 at 12:00 +0200, Erik Skultety wrote:
+ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck"
I'm not sure we want to have such a proxy for all possible make targets, especially since CI_MAKE_ARGS exists and is specifically intended for the purpose of running $ make ci-build@centos-8 CI_MAKE_ARGS="syntax-check distcheck" or whatever. Anyway, if you're going to add new proxy targets please at least document them in ci-help, so that users can find out about them without having to dig into the Makefile. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Jun 17, 2020 at 03:18:48PM +0200, Andrea Bolognani wrote:
On Wed, 2020-06-17 at 12:00 +0200, Erik Skultety wrote:
+ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck"
I'm not sure we want to have such a proxy for all possible make targets, especially since CI_MAKE_ARGS exists and is specifically intended for the purpose of running
$ make ci-build@centos-8 CI_MAKE_ARGS="syntax-check distcheck"
I think that ship has sailed the moment we added ci-check. What I'll do is respin this patch with extending the ci-help output as well as adding CI_MAKE_ARGS to the help output, because like you said, one has to skim through the Makefile to see what's available to them. Erik

On Wed, 2020-06-17 at 16:05 +0200, Erik Skultety wrote:
On Wed, Jun 17, 2020 at 03:18:48PM +0200, Andrea Bolognani wrote:
On Wed, 2020-06-17 at 12:00 +0200, Erik Skultety wrote:
+ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck"
I'm not sure we want to have such a proxy for all possible make targets, especially since CI_MAKE_ARGS exists and is specifically intended for the purpose of running
$ make ci-build@centos-8 CI_MAKE_ARGS="syntax-check distcheck"
I think that ship has sailed the moment we added ci-check. What I'll do is respin this patch with extending the ci-help output as well as adding CI_MAKE_ARGS to the help output, because like you said, one has to skim through the Makefile to see what's available to them.
No ships have sailed, we can still make all the changes we deem necessary as well as avoid those we consider undesirable: ci-check existing does not mean that we have blanket permission for adding ci-cov, ci-pdf, ci-mostlyclean and everything else. Anyway, if you think ci-distcheck is worth having I will not stand in the way - as long as you document it properly :) -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Jun 17, 2020 at 04:42:00PM +0200, Andrea Bolognani wrote:
On Wed, 2020-06-17 at 16:05 +0200, Erik Skultety wrote:
On Wed, Jun 17, 2020 at 03:18:48PM +0200, Andrea Bolognani wrote:
On Wed, 2020-06-17 at 12:00 +0200, Erik Skultety wrote:
+ci-distcheck@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="distcheck"
I'm not sure we want to have such a proxy for all possible make targets, especially since CI_MAKE_ARGS exists and is specifically intended for the purpose of running
$ make ci-build@centos-8 CI_MAKE_ARGS="syntax-check distcheck"
I think that ship has sailed the moment we added ci-check. What I'll do is respin this patch with extending the ci-help output as well as adding CI_MAKE_ARGS to the help output, because like you said, one has to skim through the Makefile to see what's available to them.
No ships have sailed, we can still make all the changes we deem necessary as well as avoid those we consider undesirable: ci-check existing does not mean that we have blanket permission for adding ci-cov, ci-pdf, ci-mostlyclean and everything else.
Anyway, if you think ci-distcheck is worth having I will not stand in the way - as long as you document it properly :)
I sent a patch for expanding the ci-help output which you've already reviewed and I settled on the decision that it's enough to document it. Consider this patch dropped. Erik
participants (3)
-
Andrea Bolognani
-
Erik Skultety
-
Michal Privoznik