Update drone caching strategy #715
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #715 +/- ##
===========================================
+ Coverage 97.11% 97.21% +0.10%
===========================================
Files 155 155
Lines 8461 8486 +25
===========================================
+ Hits 8217 8250 +33
+ Misses 244 236 -8 see 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
GCOVR code coverage report https://715.url.prtest.cppalliance.org/gcovr/index.html |
|
Running "git pull" or "submodule update" every time is a good idea. Then we can recache the newer versions. |
| @@ -410,7 +410,7 @@ def generate(compiler_ranges, cxx_range, max_cxx=2, coverage=True, docs=True, as | |||
|
|
|||
| if cache_dir != None and image_supports_caching(image, compiler): | |||
| environment['drone_cache_mount'] = cache_dir | |||
| if image in images_used: | |||
| if image in images_used or buildtype != "boost": | |||
There was a problem hiding this comment.
Who's expected to build the original cache on which we "git pull" or "submodule update" for the first time?
Don't we still need drone_cache_rebuild in one job to upload the new libs with "git pull" or "submodule update" work?
We "git pull" or "submodule update" do not do anything, can we set drone_cache_rebuild from true to false to skip the upload step?
There was a problem hiding this comment.
@alandefreitas
notice in this modification,
if image in images_used or buildtype != "boost":
"!=" instead of a "=". The effect is excluding "docs" from being the rebuilder instance. "boost" will rebuild.
Who's expected to build the original cache on which we "git pull" or "submodule update" for the first time?
common_install.sh runs every time, regardless, and creates the original boost-root folder.
Don't we still need drone_cache_rebuild
drone_cache_rebuild is still present. Hasn't been removed from the procedure.
When "git pull" or "submodule update" do not do anything, can we set drone_cache_rebuild from true to false to skip the upload step?
this is the same issue that has been present for a while. It only affects the rebuilder instances, and only takes 5 seconds. Not so bad. I will open an issue with meltwater but probably we can live with that delay.
There was a problem hiding this comment.
"!=" instead of a "=". The effect is excluding "docs" from being the rebuilder instance. "boost" will rebuild.
Oh... sorry. So this is just fixing something else in passing. Great.
only takes 5 seconds
👍
There was a problem hiding this comment.
issue with meltwater
actually, I will delay asking them, because it's likely beyond their ability to fix right now, I have already opened an issue to be able to cache absolute paths instead of relative paths which would allow boost-root to be immediately cached instead of copying it back and forth.
.drone/drone.sh
Outdated
| BOOST_ROOT="$(pwd)" | ||
| export BOOST_ROOT | ||
| ls "$cache_dir/boost" | ||
| export BOOST_ROOT="$(pwd)" |
There was a problem hiding this comment.
This is effectively just removing the ls "$cache_dir/boost" line above.
I've been splitting commands such as export BOOST_ROOT="$(pwd)" into
BOOST_ROOT="$(pwd)"
export BOOST_ROOT
because shellcheck gives me the error SC2155. Not a huge problem but still annoying because it hides more serious problems.
There was a problem hiding this comment.
shellcheck. Ok switched it back. The commits should be squashed.
|
|
||
| if [ "$drone_cache_rebuild" == true ]; then | ||
| if command -v apt-get &>/dev/null; then | ||
| apt-get install -y rsync |
There was a problem hiding this comment.
Should the images include rsync? I couldn't replicate the command with cp but this conditional looks like a workaround.
There was a problem hiding this comment.
The images should include rsync but they don't at the moment. This workaround isn't too bad.
| $python_executable tools/boostdep/depinst/depinst.py --include benchmark --include example --include examples --include tools "$SELF" | ||
| cd $DRONE_WORKSPACE | ||
| fi | ||
| . ./ci/common_install.sh |
There was a problem hiding this comment.
That's quite nice. It does the whole thing properly in both paths.
|
This strategy is quite smart :) The cache baseline only needs to change when there's a new tag, depinst.py ensures everything is up to date, and anything updated is reached. Perfect. |
By the way, that's not strictly required. It seems to be a nice idea to refresh on a git tag, every three or four months. But conceivably you could remove the check and never recreate the cache so it would be a perpetual eternal cache because "git pull" is being run every time. |
|
GCOVR code coverage report https://715.url.prtest.cppalliance.org/gcovr/index.html |
|
GCOVR code coverage report https://715.url.prtest.cppalliance.org/gcovr/index.html |
|
caching is currently implemented as a container 'meltwater/drone-cache' which means the function |
|
GCOVR code coverage report https://715.url.prtest.cppalliance.org/gcovr/index.html |
Definitely. This is a plain error. Is this PR good to go in your opinion? |
|
Each time we find something new, but yes it should be good-to-go. |
A proposed change to the caching methodology.
The cache key has been the boostorg/boost commit SHA, which changes around every 4 hours and thus invalidates the cache most of the times you attempt to use it. If a few hours have passed and there is a pull request or a commit, those will be cache misses.
The new strategy is to basically always use the cache.
common_install.sh runs every time. "git pull" or "submodule update" only take a few seconds because they don't need to download a full git repository. b2 recompilation will be skipped.