1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/4] Misc travis improvements
  4. View patch

[libvirt] [PATCH 4/4] travis: run 'make install' during build tests

Daniel P. Berrangé posted 4 patches 7 years, 2 months ago
[libvirt] [PATCH 4/4] travis: run 'make install' during build tests
Posted by Daniel P. Berrangé 7 years, 2 months ago 
Running 'make install' is important to catch some VPATH problems

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 0328fcb8f1..61f0e38d40 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -105,12 +105,12 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl; fi
 
 before_script:
-  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS
+  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS --prefix=$(pwd)/../vroot
 
 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 check; fi && make -j3 install
 
 after_failure:
   - echo '============================================================================'
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] travis: run 'make install' during build tests
Posted by Andrea Bolognani 7 years, 2 months ago 
On Fri, 2018-02-23 at 12:01 +0000, Daniel P. Berrangé wrote:
> Running 'make install' is important to catch some VPATH problems
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .travis.yml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 0328fcb8f1..61f0e38d40 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -105,12 +105,12 @@ before_install:
>    - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl; fi
>  
>  before_script:
> -  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS
> +  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS --prefix=$(pwd)/../vroot

It would be nicer if the hard-coded parameters appeared before
the dynamic ones here, in case we later find out we need per-OS
overrides.

Also I don't like "vroot" very much, I would prefer if you used
"install" instead. But that's just my preference, so feel free to
disregard it.

>  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 check; fi && make -j3 install

The install step should be right after building, so that the part
of the command that is executed on all operating systems and the
one that only applies to a subset are still visually separated.


For the second hunk and with the above addressed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

The first hunk might end up being dropped due to the comments
on patch 4/4; if not, and with the comments addressed, the
R-b applies to it too.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] travis: run 'make install' during build tests
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Tue, Feb 27, 2018 at 04:23:01PM +0100, Andrea Bolognani wrote:
> On Fri, 2018-02-23 at 12:01 +0000, Daniel P. Berrangé wrote:
> > Running 'make install' is important to catch some VPATH problems
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .travis.yml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/.travis.yml b/.travis.yml
> > index 0328fcb8f1..61f0e38d40 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -105,12 +105,12 @@ before_install:
> >    - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew upgrade && brew install rpcgen yajl; fi
> >  
> >  before_script:
> > -  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS
> > +  - mkdir build && cd build && ../autogen.sh $CONFIGURE_ARGS --prefix=$(pwd)/../vroot
> 
> It would be nicer if the hard-coded parameters appeared before
> the dynamic ones here, in case we later find out we need per-OS
> overrides.
> 
> Also I don't like "vroot" very much, I would prefer if you used
> "install" instead. But that's just my preference, so feel free to
> disregard it.
> 
> >  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 check; fi && make -j3 install
> 
> The install step should be right after building, so that the part
> of the command that is executed on all operating systems and the
> one that only applies to a subset are still visually separated.

I put the 'install' bit last, because that is the least likely
bit to fail, so preferrable to have 'syntax-check' / 'check' run
first to fail earlier

I think it would be clearer if we just put a custom script: entry
under OS-X in the matrix and got rid of the conditional check. 


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] travis: run 'make install' during build tests
Posted by Andrea Bolognani 7 years, 2 months ago 
On Tue, 2018-02-27 at 15:44 +0000, Daniel P. Berrangé wrote:
> > >  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 check; fi && make -j3 install
> > 
> > The install step should be right after building, so that the part
> > of the command that is executed on all operating systems and the
> > one that only applies to a subset are still visually separated.
> 
> I put the 'install' bit last, because that is the least likely
> bit to fail, so preferrable to have 'syntax-check' / 'check' run
> first to fail earlier

It's also the part that takes the least time, so if syntax-check or
check are going to fail, running install before them won't delay the
result by any significant amount of time - not to mention that the
overall completion time is easily dominated by waiting in the queue
for a macOS builder.

> I think it would be clearer if we just put a custom script: entry
> under OS-X in the matrix and got rid of the conditional check.

I disagree: separating the matrix definition and the script
definition results in a much more readable configuration with zero
duplicated information. In fact, we used to have everything jumbled
together, but we moved away from that setup.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.