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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  .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@xxxxxxxxxx>

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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux