New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Travis: Refactor CI files, use container infrastructure. #2938

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@fwalch
Member

fwalch commented Jul 2, 2015

  • Split build steps to utilize the Travis build lifecycle.
  • Move shell code from .travis.yml into Bash files in .ci/,
    one file for each step of the Travis build lifecycle.
  • Use configuration variables in .travis.yml to change
    build behavior (e.g. build 32-bit with BUILD_32BIT=ON).
  • Keep all configuration in environment variables in
    .travis.yml. In scripts, concatenate environment variables
    according to configuration to change to different behavior.
  • Add GCC 5 Linux builds.
  • Use Travis's caching feature [1] for third-party dependencies
    and pip packages.
  • Allow failures MSan, as the errors it reports have to be
    fixed first.

Valgrind is still disabled, but can be enabled by setting
env: VALGRIND=ON for a job in .travis.yml.

[1] http://docs.travis-ci.com/user/caching

WIP since I couldn't test the OS X builds, so they might not work currently. Edit: Updated PR description with new commit message.
Also, since we don't have sudo, we cannot create new groups for the tests:

# Adds user to a dummy group.
# That allows to test changing the group of the file by `os_fchown`.

@fwalch fwalch changed the title from [RFC] Travis: Refactor CI files, use container infrastructure. to [WIP] Travis: Refactor CI files, use container infrastructure. Jul 2, 2015

@marvim marvim added the WIP label Jul 2, 2015

@ZyX-I

This comment has been minimized.

Show comment
Hide comment
@ZyX-I

ZyX-I Jul 2, 2015

Contributor

I am wondering whether this will allow re-enabling valgrind in tests. Various caches should cut off the installation time and containers also waste less resources on themselves.

Contributor

ZyX-I commented Jul 2, 2015

I am wondering whether this will allow re-enabling valgrind in tests. Various caches should cut off the installation time and containers also waste less resources on themselves.

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 2, 2015

Member

@ZyX-I Good question, let's re-enable it and see what happens.

Member

fwalch commented Jul 2, 2015

@ZyX-I Good question, let's re-enable it and see what happens.

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 4, 2015

Member

One note about the caches: The Travis docu says that PRs currently use caches of the branch they're supposed to be merged into. This means that a PR that changes the dependencies could break other PR's builds. To try to prevent this, I added a commit to only update the third-party dependencies cache if the build was successful (i.e. all tests passed).

Member

fwalch commented Jul 4, 2015

One note about the caches: The Travis docu says that PRs currently use caches of the branch they're supposed to be merged into. This means that a PR that changes the dependencies could break other PR's builds. To try to prevent this, I added a commit to only update the third-party dependencies cache if the build was successful (i.e. all tests passed).

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 4, 2015

Member

The Valgrind builds still take 40min.. I think that's too long to run on every PR. But we could set up "nightly builds" with bot-ci or the QuickBuild server.

Member

fwalch commented Jul 4, 2015

The Valgrind builds still take 40min.. I think that's too long to run on every PR. But we could set up "nightly builds" with bot-ci or the QuickBuild server.

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 4, 2015

Member

The Travis docu says that PRs currently use caches of the branch they're supposed to be merged into.

Looking at the build output, this doesn't seem to hold anymore: fetching PR.2938/cache--compiler-clang-3.6.tgz. So I guess we don't have to worry about this :-)

Member

fwalch commented Jul 4, 2015

The Travis docu says that PRs currently use caches of the branch they're supposed to be merged into.

Looking at the build output, this doesn't seem to hold anymore: fetching PR.2938/cache--compiler-clang-3.6.tgz. So I guess we don't have to worry about this :-)

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 4, 2015

Member

Okay, this seems to work now, with and without cached dependencies. Marking RFC to get comments. If you have the access permissions, you can delete the Travis cache and restart one of the jobs to see how it recompiles and stores the deps.

One strange thing I noticed: pip is upgraded in before_install.. or is it?

# When upgrading pip:
You are using pip version 6.0.8, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting pip from https://pypi.python.org/packages/py2.py3/p/pip/pip-7.1.0-py2.py3-none-any.whl#md5=b108384a762825ec20345bb9b5b7209f
  Using cached pip-7.1.0-py2.py3-none-any.whl
Installing collected packages: pip
Successfully installed pip-6.0.8
# Next invocation of pip:
You are using pip version 6.0.8, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

Note how it says it will install 7.1.0, but then reports that it installed 6.0.8. It should be installed to ~/.local/bin, which is in the PATH.

Member

fwalch commented Jul 4, 2015

Okay, this seems to work now, with and without cached dependencies. Marking RFC to get comments. If you have the access permissions, you can delete the Travis cache and restart one of the jobs to see how it recompiles and stores the deps.

One strange thing I noticed: pip is upgraded in before_install.. or is it?

# When upgrading pip:
You are using pip version 6.0.8, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting pip from https://pypi.python.org/packages/py2.py3/p/pip/pip-7.1.0-py2.py3-none-any.whl#md5=b108384a762825ec20345bb9b5b7209f
  Using cached pip-7.1.0-py2.py3-none-any.whl
Installing collected packages: pip
Successfully installed pip-6.0.8
# Next invocation of pip:
You are using pip version 6.0.8, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

Note how it says it will install 7.1.0, but then reports that it installed 6.0.8. It should be installed to ~/.local/bin, which is in the PATH.

@fwalch fwalch changed the title from [WIP] Travis: Refactor CI files, use container infrastructure. to [RFC] Travis: Refactor CI files, use container infrastructure. Jul 4, 2015

@marvim marvim added RFC and removed WIP labels Jul 4, 2015

@fwalch fwalch added the ci label Jul 4, 2015

@tarruda

This comment has been minimized.

Show comment
Hide comment
@tarruda

tarruda Jul 5, 2015

Member

@fwalch great improvement, not only to build times but also to the scripts. LGTM

Member

tarruda commented Jul 5, 2015

@fwalch great improvement, not only to build times but also to the scripts. LGTM

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 5, 2015

Member

Rebased after #2807.

Member

fwalch commented Jul 5, 2015

Rebased after #2807.

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 5, 2015

Member

Re-introduced a job with GCC 4.6 to make sure we don't break something for older compilers. Fixed gcov/coveralls warnings (charset.c.gcno:version '501*', prefer '406*') by using a gcov (or llvm-cov) matching the used compiler. Upload still fails:

{u'url': u'', u'message': u'Build processing error.', u'error': True}
coveralls upload failed.

This happens for all jobs. It also occurs with other PRs/master (according to the coveralls website, the last build was yesterday, but I merged #2807 today), so maybe coveralls has some problems atm.

Still don't know why pip isn't upgraded (#2938 (comment)).

Member

fwalch commented Jul 5, 2015

Re-introduced a job with GCC 4.6 to make sure we don't break something for older compilers. Fixed gcov/coveralls warnings (charset.c.gcno:version '501*', prefer '406*') by using a gcov (or llvm-cov) matching the used compiler. Upload still fails:

{u'url': u'', u'message': u'Build processing error.', u'error': True}
coveralls upload failed.

This happens for all jobs. It also occurs with other PRs/master (according to the coveralls website, the last build was yesterday, but I merged #2807 today), so maybe coveralls has some problems atm.

Still don't know why pip isn't upgraded (#2938 (comment)).

@@ -1,5 +1,7 @@
set(CMAKE_SYSTEM_PROCESSOR i386)
set(CMAKE_SYSTEM_NAME Linux)
set(CMAKE_SYSTEM_VERSION gnu)
set(CMAKE_C_COMPILER gcc)
if(NOT ${CMAKE_C_COMPILER})
set(CMAKE_C_COMPILER gcc)

This comment has been minimized.

@fwalch

fwalch Jul 5, 2015

Member

@jszakmeister I changed this to be able to use a different compiler (in this case, gcc-5) to build for 32 bit. NOT CMAKE_C_COMPILER and NOT DEFINED CMAKE_C_COMPILER both didn't work. Do you think it's okay like this?

Also, I'm wondering if we should introduce more toolchain files instead of putting all of these CMake flags in .travis.yml.. but not in this PR.

@fwalch

fwalch Jul 5, 2015

Member

@jszakmeister I changed this to be able to use a different compiler (in this case, gcc-5) to build for 32 bit. NOT CMAKE_C_COMPILER and NOT DEFINED CMAKE_C_COMPILER both didn't work. Do you think it's okay like this?

Also, I'm wondering if we should introduce more toolchain files instead of putting all of these CMake flags in .travis.yml.. but not in this PR.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 5, 2015

Re-introduced a job with GCC 4.6 to make sure we don't break something for older compilers

Has there ever been an issue related to this? Or is it just being done preemptively?

ghost commented Jul 5, 2015

Re-introduced a job with GCC 4.6 to make sure we don't break something for older compilers

Has there ever been an issue related to this? Or is it just being done preemptively?

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 5, 2015

Member

Used the wrong gcov for OS X clang, but should be fixed now. Added an additional commit to verify that $LLVM_SYMBOLIZER and $GCOV can be executed.

Has this ever been an issue?

@Pyrohh So far we always used GCC 4.6 (for Linux, OSX used 4.9) on Travis, so it's hard to say. Probably not...

Member

fwalch commented Jul 5, 2015

Used the wrong gcov for OS X clang, but should be fixed now. Added an additional commit to verify that $LLVM_SYMBOLIZER and $GCOV can be executed.

Has this ever been an issue?

@Pyrohh So far we always used GCC 4.6 (for Linux, OSX used 4.9) on Travis, so it's hard to say. Probably not...

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 5, 2015

@Pyrohh So far we always used GCC 4.6 (for Linux, OSX used 4.9) on Travis, so it's hard to say. Probably not...

I'd say we should drop it for now; if anything comes up it will come up quickly. Besides, GCC has a lot of backwards compatibility, so I'd be surprised if anything does break (besides some flags which might not work).

ghost commented Jul 5, 2015

@Pyrohh So far we always used GCC 4.6 (for Linux, OSX used 4.9) on Travis, so it's hard to say. Probably not...

I'd say we should drop it for now; if anything comes up it will come up quickly. Besides, GCC has a lot of backwards compatibility, so I'd be surprised if anything does break (besides some flags which might not work).

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 5, 2015

@fwalch Also, regarding pip: I'd like to see it updated too, but is there much benefit in doing it ourselves? If 6.x.x works fine then lets just stick with that (unless there's some big speed benefit I'm unaware of).

As a side note, I saw travis-ci/travis-ci#3968 and wonder when the "next build environment image is provisioned".

ghost commented Jul 5, 2015

@fwalch Also, regarding pip: I'd like to see it updated too, but is there much benefit in doing it ourselves? If 6.x.x works fine then lets just stick with that (unless there's some big speed benefit I'm unaware of).

As a side note, I saw travis-ci/travis-ci#3968 and wonder when the "next build environment image is provisioned".

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 5, 2015

Member

Okay, dropped the GCC 4.6 build.

Regarding pip, you're right, it's not really necessary to update it, I'm just wondering why it doesn't work.

Edit: Coveralls seems to work again.

Member

fwalch commented Jul 5, 2015

Okay, dropped the GCC 4.6 build.

Regarding pip, you're right, it's not really necessary to update it, I'm just wondering why it doesn't work.

Edit: Coveralls seems to work again.

if [[ "${TRAVIS_OS_NAME}" == osx ]]; then
brew update
fi
pip install --user --upgrade pip

This comment has been minimized.

@ghost

ghost Jul 5, 2015

Since you said it doesn't really matter if this is updated, could you remove this line? or does it work now?

@ghost

ghost Jul 5, 2015

Since you said it doesn't really matter if this is updated, could you remove this line? or does it work now?

This comment has been minimized.

@fwalch

fwalch Jul 6, 2015

Member

It doesn't work (see also travis-ci/travis-ci#4194), but I'd just keep it in there, maybe it will be fixed at some point. The download is cached, so it should be a very small overhead.

@fwalch

fwalch Jul 6, 2015

Member

It doesn't work (see also travis-ci/travis-ci#4194), but I'd just keep it in there, maybe it will be fixed at some point. The download is cached, so it should be a very small overhead.

fwalch added some commits Jul 4, 2015

Travis: Refactor CI files, use container infrastructure.
 * Split build steps to utilize the Travis build lifecycle.
 * Move shell code from `.travis.yml` into Bash files in `.ci/`,
   one file for each step of the Travis build lifecycle.
 * Use configuration variables in `.travis.yml` to change
   build behavior (e.g. build 32-bit with `BUILD_32BIT=ON`).
 * Keep all configuration in environment variables in
   `.travis.yml`. In scripts, concatenate environment variables
   according to configuration to change to different behavior.
 * Add GCC 5 builds for Linux.
 * Use Travis's caching feature [1] for third-party dependencies
   and pip packages.
 * Allow failures MSan, as the errors it reports have to be
   fixed first.

Valgrind is still disabled, but can be enabled by setting
`env: VALGRIND=ON` for a job in `.travis.yml`.

[1] http://docs.travis-ci.com/user/caching
Travis: Use gcov that matches the used compiler.
This fixes gcov/coveralls warnings like the following:

    Segmentation fault (core dumped)
    charset.c.gcno:version '501*', prefer '406*'
    Out of memory allocating 33061786568 bytes after a total of 2522648 bytes

http://stackoverflow.com/a/14676272/249642
@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 6, 2015

Member

Okay, so the pip upgrade problem is due to the way how Travis installs pip on their workers. As I said in #2938 (comment), I'd still keep the upgrade in the scripts, it will probably be fixed by Travis at some point.

Noticed some small things and amended the commits earlier. I think this PR is finished now.

Member

fwalch commented Jul 6, 2015

Okay, so the pip upgrade problem is due to the way how Travis installs pip on their workers. As I said in #2938 (comment), I'd still keep the upgrade in the scripts, it will probably be fixed by Travis at some point.

Noticed some small things and amended the commits earlier. I think this PR is finished now.

@tarruda

This comment has been minimized.

Show comment
Hide comment
@tarruda

tarruda Jul 7, 2015

Member

@justinmk can we merge this? I'd love some faster travis builds in PRs

Member

tarruda commented Jul 7, 2015

@justinmk can we merge this? I'd love some faster travis builds in PRs

tarruda added a commit that referenced this pull request Jul 8, 2015

@tarruda

This comment has been minimized.

Show comment
Hide comment
@tarruda

tarruda Jul 8, 2015

Member

merged

Member

tarruda commented Jul 8, 2015

merged

@tarruda tarruda closed this Jul 8, 2015

@jszakmeister jszakmeister removed the RFC label Jul 8, 2015

@fwalch fwalch deleted the fwalch:travis-containers branch Jul 8, 2015

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk Jul 11, 2015

Member

This was a great contribution, but it seems like gcov errors are now more frequent.

Member

justinmk commented Jul 11, 2015

This was a great contribution, but it seems like gcov errors are now more frequent.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk Jul 11, 2015

isn't the FIXME addressed by the -not -path ...?

justinmk commented on .ci/common/test.sh in f74776b Jul 11, 2015

isn't the FIXME addressed by the -not -path ...?

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 15, 2015

Owner

No, the -not -path will make sure that only files outside of DEPS_BUILD_DIR are looked at, so this will still trigger if there is a legitimate file (e.g. checked in the repo) named core.* outside of DEPS_BUILD_DIR.

Owner

fwalch replied Jul 15, 2015

No, the -not -path will make sure that only files outside of DEPS_BUILD_DIR are looked at, so this will still trigger if there is a legitimate file (e.g. checked in the repo) named core.* outside of DEPS_BUILD_DIR.

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk Jul 11, 2015

Instead of "usermod", may want to just mention before_script.sh.

justinmk commented on .ci/script.sh in f74776b Jul 11, 2015

Instead of "usermod", may want to just mention before_script.sh.

@fwalch

This comment has been minimized.

Show comment
Hide comment
@fwalch

fwalch Jul 15, 2015

Member

This was a great contribution, but it seems like gcov errors are now more frequent.

The gcov errors should have been completely fixed, because now always the appropriate gcov executable is used. Do you mean coveralls upload errors (I have been away for a few days and didn't yet look at builds)? If so, then I don't think this is related to this PR, I noticed that no coveralls upload succeeded for a while when working on this (i.e. I think coveralls had problems with their site).

Member

fwalch commented Jul 15, 2015

This was a great contribution, but it seems like gcov errors are now more frequent.

The gcov errors should have been completely fixed, because now always the appropriate gcov executable is used. Do you mean coveralls upload errors (I have been away for a few days and didn't yet look at builds)? If so, then I don't think this is related to this PR, I noticed that no coveralls upload succeeded for a while when working on this (i.e. I think coveralls had problems with their site).

@justinmk

This comment has been minimized.

Show comment
Hide comment
@justinmk

justinmk Jul 17, 2015

Member

The gcov errors should have been completely fixed, because now always the appropriate gcov executable is used.

@fwalch The gcov errors I am referring to are "gcda merge mismatch" messages, see this recent build for example: https://travis-ci.org/neovim/neovim/jobs/71351785

log.c.gcda:Merge mismatch for function 0
Member

justinmk commented Jul 17, 2015

The gcov errors should have been completely fixed, because now always the appropriate gcov executable is used.

@fwalch The gcov errors I am referring to are "gcda merge mismatch" messages, see this recent build for example: https://travis-ci.org/neovim/neovim/jobs/71351785

log.c.gcda:Merge mismatch for function 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment