Skip to content

[RDY] ci: Travis: improve/revisit caching #10358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 30, 2019
Merged

[RDY] ci: Travis: improve/revisit caching #10358

merged 2 commits into from
Jul 30, 2019

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jun 28, 2019 •
edited
Loading

This is meant to not fall back to using the cache for the "master"
target branch, for release pull requests (targeting not "master").

TODO:

  • do not store /home/travis/.cache/nvim-deps/build/downloads

@blueyed blueyed added ci automation for build, test, and release enhancement labels Jun 28, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Jun 28, 2019

Does not work like this..

touch: cannot touch '/home/travis/.cache/nvim-deps-master/.travis_cache_marker': No such file or directory

Would also need to move/handle this in before_script then also.

@blueyed blueyed changed the title ci: Travis: use TRAVIS_BRANCH in cache marker [skip appveyor] [WIP] ci: Travis: use TRAVIS_BRANCH in cache marker [skip appveyor] Jun 28, 2019
@marvim marvim added the WIP label Jun 28, 2019
@blueyed blueyed force-pushed the travis-cache-branch branch from b7eb31e to 04ce175 Compare June 28, 2019 01:06
@blueyed
Copy link
Contributor Author

blueyed commented Jun 28, 2019

I've left .cache/nvim-deps-downloads alone for now, but should it a) also use the branch name, or b) even be not cached in the first place also (similar to pip), i.e. it is better handled by http caches already I'd assume.

justinmk
justinmk reviewed Jun 28, 2019
View reviewed changes
justinmk
justinmk reviewed Jun 28, 2019
View reviewed changes
@justinmk
Copy link
Member

justinmk commented Jun 28, 2019 •
edited
Loading

ow, but should it a) also use the branch name, or b) even be not cached in the first place also (similar to pip), i.e. it is better handled by http caches already I'd assume.

I'd say (a) for now. git clone can be slow, and very noisy in the logs...

@blueyed

This comment has been minimized.

Sign in to view
@justinmk

This comment has been minimized.

Sign in to view
@blueyed blueyed mentioned this pull request Jul 3, 2019
Merged
@blueyed

This comment has been minimized.

Sign in to view
@blueyed blueyed mentioned this pull request Jul 3, 2019
Merged
blueyed added a commit to blueyed/neovim that referenced this pull request Jul 4, 2019
@blueyed
build: bundle: clean binary dir with new downloads
59eb863 
This is required to (re)build e.g. libluv when the version changes
(which triggers a new download).

With `make deps`, changing the `LUV_URL`/`LUV_SHA256`, and `make deps` again:

Before:

> Up-to-date: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

After:

> Installing: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

See with neovim#10358 - where .deps
contained libluv 1.29, the merge updates it to 1.30, but then it failed
to link because `libluv.a` is considered to be up-to-date (after
downloading the new version).

Note that header files get installed, since they have the original time
stamp, but `libluv.a` is being generated (does not use the timestamp
from the archive here, but needs to get rebuild).

It could be argued that the build system of the included project should
catch/handle this, but it seems to be good practice to clean the binary
/ build dir with a new download to start from scratch.

Ref: https://gitlab.kitware.com/cmake/cmake/issues/19452

Also fixes cmake/BuildLuv / luv-static: use name with -DTARGET for
download command, and pass (shared) `SRC_DIR` explicitly instead.
blueyed added a commit to blueyed/neovim that referenced this pull request Jul 4, 2019
@blueyed
TEST: cherry-pick: build: bundle: clean binary dir with new downloads
45c046c 
This is required to (re)build e.g. libluv when the version changes
(which triggers a new download).

With `make deps`, changing the `LUV_URL`/`LUV_SHA256`, and `make deps` again:

Before:

> Up-to-date: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

After:

> Installing: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

See with neovim#10358 - where .deps
contained libluv 1.29, the merge updates it to 1.30, but then it failed
to link because `libluv.a` is considered to be up-to-date (after
downloading the new version).

Note that header files get installed, since they have the original time
stamp, but `libluv.a` is being generated (does not use the timestamp
from the archive here, but needs to get rebuild).

It could be argued that the build system of the included project should
catch/handle this, but it seems to be good practice to clean the binary
/ build dir with a new download to start from scratch.

Ref: https://gitlab.kitware.com/cmake/cmake/issues/19452

Also fixes cmake/BuildLuv / luv-static: use name with -DTARGET for
download command, and pass (shared) `SRC_DIR` explicitly instead.
blueyed added a commit that referenced this pull request Jul 4, 2019
@blueyed
build: bundle: clean binary dir with new downloads (#10411)
99b870d 
This is required to (re)build e.g. libluv when the version changes
(which triggers a new download).

With `make deps`, changing the `LUV_URL`/`LUV_SHA256`, and `make deps` again:

Before:

> Up-to-date: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

After:

> Installing: /home/daniel/Vcs/neovim/.deps/usr/lib/libluv.a

See with #10358 - where .deps
contained libluv 1.29, the merge updates it to 1.30, but then it failed
to link because `libluv.a` is considered to be up-to-date (after
downloading the new version).

Note that header files get installed, since they have the original time
stamp, but `libluv.a` is being generated (does not use the timestamp
from the archive here, but needs to get rebuild).

It could be argued that the build system of the included project should
catch/handle this, but it seems to be good practice to clean the binary
/ build dir with a new download to start from scratch.

Ref: https://gitlab.kitware.com/cmake/cmake/issues/19452

Also fixes cmake/BuildLuv / luv-static: use name with -DTARGET for
download command, and pass (shared) `SRC_DIR` explicitly instead.
@blueyed blueyed force-pushed the travis-cache-branch branch from 27b4a8b to bffec1c Compare July 10, 2019 15:39
@blueyed blueyed changed the title [WIP] ci: Travis: use TRAVIS_BRANCH in cache marker [skip appveyor] [WIP] ci: Travis: improve/revisit caching Jul 26, 2019
@blueyed blueyed force-pushed the travis-cache-branch branch from bffec1c to 874d80c Compare July 29, 2019 13:13
@blueyed
Copy link
Contributor Author

blueyed commented Jul 30, 2019

Should be good. Will compare logs after rebuild.
Current: https://api.travis-ci.org/v3/job/564970399/log.txt

@blueyed
ci: Travis: revisit/improve caching [skip appveyor]
e107f94 
- use CACHE_NVIM_DEPS

- do not cache pip
  This is handled through http caches in general/better, and it is not
  used much anyway.

- do not cache DEPS_DOWNLOAD_DIR / nvim-deps-downloads
  Built deps are cached, downloads are not needed then.

- display ccache stats before clearing

- ci/before_cache.sh: do not cache ccache stats

- improve du (do not list pages of output for "/home/travis/.cache/go-build")
@blueyed blueyed force-pushed the travis-cache-branch branch from dd608cd to e107f94 Compare July 30, 2019 19:49
@blueyed blueyed changed the title [WIP] ci: Travis: improve/revisit caching [RDY] ci: Travis: improve/revisit caching Jul 30, 2019
@blueyed blueyed requested a review from justinmk July 30, 2019 19:49
@marvim marvim added RDY and removed WIP labels Jul 30, 2019
justinmk
justinmk reviewed Jul 30, 2019
View reviewed changes
.travis.yml Outdated
- UBSAN_OPTIONS="print_stacktrace=1 log_path=$LOG_DIR/ubsan"
# Environment variables for Valgrind.
- VALGRIND_LOG="$LOG_DIR/valgrind-%p.log"
- CACHE_NVIM_DEPS="$HOME/.cache/nvim-deps"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have a _DIR suffix, like BUILD_DIR etc.

justinmk
justinmk reviewed Jul 30, 2019
View reviewed changes
.travis.yml
# Do not fall back to cache for "master" for PR on "release" branch:
# adds the target branch to the cache key.
FOR_TRAVIS_CACHE=$TRAVIS_BRANCH
FOR_TRAVIS_CACHE=v1-$TRAVIS_BRANCH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does "v1" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"version 1" - I've used it to bump/discard the existing cache(s).

@blueyed blueyed merged commit 208f56d into neovim:master Jul 30, 2019
@blueyed blueyed deleted the travis-cache-branch branch July 30, 2019 21:21
timeyyy pushed a commit to timeyyy/neovim that referenced this pull request Aug 9, 2019
@blueyed @timeyyy
ci: Travis: improve/revisit caching (neovim#10358)
796075c 
- use CACHE_NVIM_DEPS_DIR

- do not cache pip
  This is handled through http caches in general/better, and it is not
  used much anyway.

- do not cache DEPS_DOWNLOAD_DIR
  Built deps are cached, downloads are not needed then.

- display ccache stats before clearing

- do not cache ccache stats

- improve output of `du` (do not list pages of output for "/home/travis/.cache/go-build")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinmk justinmk justinmk approved these changes

Assignees
No one assigned
Labels
ci automation for build, test, and release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@blueyed @justinmk @marvim