[libvirt] [PATCH v2 0/5] Misc improvements & fixes to Travis

Changed in v2: - Dropped VPATH patch, since distcheck is doing that already - Skip pdwtags when building with clang - Run install & dist on OS-X - Test systemd as well as upstart - Pass initscript configure flags to distcheck, not autogen.sh Daniel P. Berrangé (5): make: skip pdwtags when building with CLang travis: drop precise distro jobs travis: run "make distcheck" instead of just "make check" travis: test "make install" and "make dist" on OS-X travis: test upstart/systemd init script handling .travis.yml | 22 +++++++++++----------- src/Makefile.am | 10 +++++++++- 2 files changed, 20 insertions(+), 12 deletions(-) -- 2.14.3

When building with CLang the structs that are emitted by pdwtags appear in a completely different order than with GCC, which causes the comparison against expected data to fail. Ideally the test would not be sensitive to the ordering, because even future GCC could cause changes, but that's not easy to fix. So for now just skip the test when using clang. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/Makefile.am | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8d72f2f1e5..4207421986 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -664,8 +664,16 @@ struct_prefix = ($(libs_prefix)|$(other_prefix)) # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want # the newest of the two, in case configure options changed and a stale # file is left around from an earlier build. +# The pdwtags output is completely different when building with clang +# which causes the comparison against expected output to fail, so skip +# if using clang as CC. PDWTAGS = \ - $(AM_V_GEN)if (pdwtags --help) > /dev/null 2>&1; then \ + $(AM_V_GEN)$CC -v 2>&1 | grep clang >/dev/null; \ + if test $$? == 1; then \ + echo 'WARNING: skipping pdwtags test with Clang' >&2; \ + exit 0; \ + fi; \ + if (pdwtags --help) > /dev/null 2>&1; then \ o=`ls -t $(<:.lo=.$(OBJEXT)) \ $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \ 2>/dev/null | sed -n 1p`; \ -- 2.14.3

On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
When building with CLang the structs that are emitted by pdwtags appear in a completely different order than with GCC, which causes the comparison against expected data to fail.
Ideally the test would not be sensitive to the ordering, because even future GCC could cause changes, but that's not easy to fix. So for now just skip the test when using clang.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/Makefile.am | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
Where have I seen this already? Oh, right :) https://www.redhat.com/archives/libvir-list/2016-March/msg00514.html
diff --git a/src/Makefile.am b/src/Makefile.am index 8d72f2f1e5..4207421986 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -664,8 +664,16 @@ struct_prefix = ($(libs_prefix)|$(other_prefix)) # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want # the newest of the two, in case configure options changed and a stale # file is left around from an earlier build. +# The pdwtags output is completely different when building with clang +# which causes the comparison against expected output to fail, so skip +# if using clang as CC. PDWTAGS = \ - $(AM_V_GEN)if (pdwtags --help) > /dev/null 2>&1; then \ + $(AM_V_GEN)$CC -v 2>&1 | grep clang >/dev/null; \ + if test $$? == 1; then \ + echo 'WARNING: skipping pdwtags test with Clang' >&2; \ + exit 0; \ + fi; \
You could do if $(CC) -v 2>&1 | grep -q clang; then ... instead, it's shorter.
+ if (pdwtags --help) > /dev/null 2>&1; then \ o=`ls -t $(<:.lo=.$(OBJEXT)) \ $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \ 2>/dev/null | sed -n 1p`; \
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

The precise distro is marked deprecated in travis and will be dropped entirely in 2 months time. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3f26a1eeee..33887e7284 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,12 +4,8 @@ cache: ccache matrix: include: - - compiler: gcc - dist: precise - compiler: gcc dist: trusty - - compiler: clang - dist: precise - compiler: clang dist: trusty - compiler: clang @@ -89,9 +85,6 @@ env: # The custom $PATH is just to pick up some extra binaries installed # through homebrew on macOS and it's completely harmless on Linux - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" - # The hyperv driver fails to build with clang on precise due to this - # error being raised in one of openwsman header files - - CFLAGS="-Wno-error=variadic-macros" - VIR_TEST_DEBUG=1 before_install: -- 2.14.3

On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
The precise distro is marked deprecated in travis and will be dropped entirely in 2 months time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Running "make distcheck" includes the "make check", and "make dist" targets. It ensures that we have CLEANFILES and uninstall rules setup correctly, as well as validating VPATH builds succeed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 33887e7284..0cfdf80950 100644 --- a/.travis.yml +++ b/.travis.yml @@ -96,7 +96,7 @@ before_script: script: # Many unit tests still fail on macOS, and there are a bunch of issues with # syntax-check as well, so skip those steps on that platform for now - - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check && make -j3 check; fi + - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check && make -j3 distcheck; fi after_failure: - echo '============================================================================' -- 2.14.3

On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
Running "make distcheck" includes the "make check", and "make dist"
Double spacing here.
targets. It ensures that we have CLEANFILES and uninstall rules setup correctly, as well as validating VPATH builds succeed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

We can't use "make distcheck" on OS-X because many unit tests fail. We can still get coverage of some of the things "distcheck" validates, by running the "install" and "dist" targets. This is particularly useful because many conditional features are disabled on OS-X, and this helps make sure we can still successfully install & dist when these bits are disabled. The default script is getting unreadable since it is all on one long line. Rather than adding further conditional clauses to it, we make use of the travis matrix config override for the script. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0cfdf80950..da9fe8af91 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,11 @@ matrix: dist: trusty - compiler: clang os: osx + script: + # We can't run make distcheck/syntax-check because they + # fail on OS-X, but doing 'intsall' and 'dist' gives us + # some useful coverage + - make -j3 && make -j3 install && make -j3 dist addons: apt: @@ -91,12 +96,10 @@ before_install: - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl; fi before_script: - - ./autogen.sh + - ./autogen.sh --prefix=$(pwd)/install-root script: - # Many unit tests still fail on macOS, and there are a bunch of issues with - # syntax-check as well, so skip those steps on that platform for now - - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check && make -j3 distcheck; fi + - make -j3 && make -j3 syntax-check && make -j3 distcheck after_failure: - echo '============================================================================' -- 2.14.3

On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
We can't use "make distcheck" on OS-X because many unit tests fail. We
s/OS-X/macOS/g There are a lot of instances both in the commit message and in the Travis configuration, please make sure you replace all of them. [...]
@@ -10,6 +10,11 @@ matrix: dist: trusty - compiler: clang os: osx + script: + # We can't run make distcheck/syntax-check because they + # fail on OS-X, but doing 'intsall' and 'dist' gives us
s/intsall/install/ Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Feb 28, 2018 at 04:24:00PM +0100, Andrea Bolognani wrote:
On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
We can't use "make distcheck" on OS-X because many unit tests fail. We
s/OS-X/macOS/g
There are a lot of instances both in the commit message and in the Travis configuration, please make sure you replace all of them.
[...]
@@ -10,6 +10,11 @@ matrix: dist: trusty - compiler: clang os: osx + script: + # We can't run make distcheck/syntax-check because they + # fail on OS-X, but doing 'intsall' and 'dist' gives us
s/intsall/install/
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This isn't quite ready to push after all - it seems our current travis setup for OS-X doesn't have an 'xz' binary installed causing dist to fail. I'll need to figure out which (if any) package we can install to get this binary. 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 Wed, Feb 28, 2018 at 05:01:25PM +0000, Daniel P. Berrangé wrote:
On Wed, Feb 28, 2018 at 04:24:00PM +0100, Andrea Bolognani wrote:
On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
We can't use "make distcheck" on OS-X because many unit tests fail. We
s/OS-X/macOS/g
There are a lot of instances both in the commit message and in the Travis configuration, please make sure you replace all of them.
[...]
@@ -10,6 +10,11 @@ matrix: dist: trusty - compiler: clang os: osx + script: + # We can't run make distcheck/syntax-check because they + # fail on OS-X, but doing 'intsall' and 'dist' gives us
s/intsall/install/
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This isn't quite ready to push after all - it seems our current travis setup for OS-X doesn't have an 'xz' binary installed causing dist to fail. I'll need to figure out which (if any) package we can install to get this binary.
Turns out the brew package is simply called "xz" so I'll squash in @@ -97,7 +97,7 @@ env: - VIR_TEST_DEBUG=1 before_install: - - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl; fi + - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl xz; fi before_script: - ./autogen.sh --prefix=$(pwd)/install-root 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 :|

Enable testing of both the upstart and systemd init script handling. We test a different one in each scenario. Even though trusty only cares about systemd, it is fine for us to test rules that install upstasrt, since we're not actually running these scripts for real. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index da9fe8af91..773cb522ea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,8 +6,12 @@ matrix: include: - compiler: gcc dist: trusty + env: + - DISTCHECK_CONFIGURE_ARGS="--with-init-script=upstart" - compiler: clang dist: trusty + env: + - DISTCHECK_CONFIGURE_ARGS="--with-init-script=systemd" - compiler: clang os: osx script: @@ -99,7 +103,7 @@ before_script: - ./autogen.sh --prefix=$(pwd)/install-root script: - - make -j3 && make -j3 syntax-check && make -j3 distcheck + - make -j3 && make -j3 syntax-check && make -j3 distcheck DISTCHECK_CONFIGURE_ARGS=$DISTCHECK_CONFIGURE_ARGS after_failure: - echo '============================================================================' -- 2.14.3

On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
Enable testing of both the upstart and systemd init script handling. We test a different one in each scenario. Even though trusty only cares about systemd,
This is incorrect: as pointed out earlier, trusty uses upstart.
it is fine for us to test rules that install upstasrt, since we're not actually running these scripts for real.
s/upstasrt/upstart/ [...]
@@ -99,7 +103,7 @@ before_script: - ./autogen.sh --prefix=$(pwd)/install-root
script: - - make -j3 && make -j3 syntax-check && make -j3 distcheck + - make -j3 && make -j3 syntax-check && make -j3 distcheck DISTCHECK_CONFIGURE_ARGS=$DISTCHECK_CONFIGURE_ARGS
s/DISTCHECK_CONFIGURE_ARGS/DISTCHECK_CONFIGURE_FLAGS/g here and above, or it won't actually pick it up. Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Feb 28, 2018 at 04:44:39PM +0100, Andrea Bolognani wrote:
On Wed, 2018-02-28 at 14:55 +0000, Daniel P. Berrangé wrote:
Enable testing of both the upstart and systemd init script handling. We test a different one in each scenario. Even though trusty only cares about systemd,
This is incorrect: as pointed out earlier, trusty uses upstart.
Heh, yes I inverted the sentance :-)
it is fine for us to test rules that install upstasrt, since we're not actually running these scripts for real.
s/upstasrt/upstart/
[...]
@@ -99,7 +103,7 @@ before_script: - ./autogen.sh --prefix=$(pwd)/install-root
script: - - make -j3 && make -j3 syntax-check && make -j3 distcheck + - make -j3 && make -j3 syntax-check && make -j3 distcheck DISTCHECK_CONFIGURE_ARGS=$DISTCHECK_CONFIGURE_ARGS
s/DISTCHECK_CONFIGURE_ARGS/DISTCHECK_CONFIGURE_FLAGS/g
here and above, or it won't actually pick it up.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
-- Andrea Bolognani / Red Hat / Virtualization
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 (2)
-
Andrea Bolognani
-
Daniel P. Berrangé