Re: [libvirt] [PATCH v4 1/7] tests: add targets for building libvirt inside Docker containers
On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote:
[...]
It is also possible to do cross compiled builds via the Debian
containers
make ci-build@debian-9-cross-s390x
You're not advertising cross-compilation images until commit 6/7,
so either drop this from the commit message or squash that commit
into this one. I see no reason not to do the latter.
[...]
+# Docker defaults to pulling the ':latest' tag but
+# if the Docker repo above uses different conventions
+# this can override it
+CI_IMAGE_TAG = :master
This should probably contain only "master", since that's the tag
itself and ":" is just a separator. We can change it later.
[...]
+ci-prepare-tree: ci-check-docker
+ @if test "$(CI_REUSE)" != "1" ; then \
+ rm -rf $(CI_SCRATCHDIR) ; \
+ fi
The equivalent code for CI_CLEAN looks like
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
Please pick either style and use it consistently throughout.
+ @if ! test -d $(CI_SCRATCHDIR) ; then \
+ mkdir -p $(CI_HOST_SRCDIR); \
Please create CI_SCRATCHDIR instead of CI_HOST_SRCDIR here: 'git
clone' will then take care of the latter.
[...]
+ @echo "Available targets:"
+ @echo
+ @echo " ci-build@\$$IMAGE - run a default 'make'"
+ @echo " ci-check@\$$IMAGE - run a 'make check'"
+ @echo " ci-shell@\$$IMAGE - run an interactive shell"
Double space between "IMAGE" and "-".
[...]
+ @echo "Available make variables:"
+ @echo
+ @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after
completion"
+ @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content"
Double space before "-" here as well.
There are actually many more variables that users can provide to
tweak the build environment or steps, chief among them CI_MAKE_ARGS:
we should document at least the ones we expect to be more widely
applicable. That's another thing we can deal with later, though.
With the small issues pointed out above addressed, and at long last,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
Thanks for bearing with me :)
--
Andrea Bolognani / Red Hat / Virtualization