Activate osx builds on travis, at low priority
We want to activate osx builds on Travis CI. But they can be very slow, so we need to customise the settings.
If possible, we want osx builds to behave as follows:
- if there is a pending build, and more commits are pushed to the branch, cancel the pending build and re-queue the latest commits
- let travis show builds as "complete" if osx takes a long time, but still show osx failures eventually
Activity
changed milestone to %Tor: 0.3.5.x-final
added 034-backport 034-removed-20180328 034-triage-20180328 035-removed-20180711 component::core tor/tor fast-fix milestone::Tor: 0.3.5.x-final not-just-linux owner::teor points::0.5 priority::medium resolution::implemented reviewer::catalyst severity::normal status::closed teor-was-assigned tor-ci type::enhancement labels
this can be configured by:
- uncommenting the "osx" line
- turning on the Auto Cancellation feature https://docs.travis-ci.com/user/customizing-the-build/#Building-only-the-latest-commit
- setting the osx job in allow_failures https://docs.travis-ci.com/user/customizing-the-build/#Rows-that-are-Allowed-to-Fail AND setting fast_finish: true https://docs.travis-ci.com/user/customizing-the-build/#Fast-Finishing
Replying to teor:
I wish auto-cancellation was available as a default in the travis config file.
I just checked my travis settings, and auto-cancellation is on by default. I don't think we need to do anything here.
Also, I am not sure we want to make osx "allow_failures" just so we can set "fast_finish" on it.
The comments in travis.yml don't seem to match the documentation: https://docs.travis-ci.com/user/customizing-the-build/#Fast-Finishing
Are there any other options?
Apparently not.
See my branch travis-osx, building now at https://travis-ci.org/teor2345/tor
Edit: add link to documentation
Trac:
Type: defect to enhancement
Status: assigned to needs_informationThis rust failure shows that the fast_finish comment is wrong: https://travis-ci.org/teor2345/tor/jobs/316721014
The other builds continued despite the failure.
Trac:
Status: needs_information to needs_revisionwe should try to minimize the number of macOS jobs that run, both to be a better netizen and pragmatically because they are limited to two at a time, so if you have a bunch they will not run in a good order.
for example, I think it is probably unnecessary to test three ways of compiling rust with both gcc and clang.
I think we should do:
- Linux:
- gcc, rust online
- gcc, rust offline
- gcc, no rust
- clang, no rust
- macOS:
- gcc, rust online
- clang, no rust
with a view to changing number five to offline, whenever that gets fixed. this will utilise all of the allowed parallel build slots (4/6 Linux, 2/6 macOS) and ensure the best immediate feedback. for better immediate feedback, we could also enable cargo cache (https://docs.travis-ci.com/user/caching/#Rust-Cargo-cache).
- Linux:
Ok, I've run out of time for this, there are commits for this issue and #24630 (moved) in my branch travis-osx-v2. Feel free to keep hacking on it!
Travis has recently upgraded its macOS builders, so we should try enabling macOS again: https://blog.travis-ci.com/2018-01-30-macos-update
This should only take an hour or two to rebase onto master and test.
Putting this in 0.3.5, because a lack of macOS CI is holding up tickets like #17949 (moved).
Trac:
Milestone: Tor: unspecified to Tor: 0.3.5.x-finalPlease see my branch travis-osx-v7 on https://github.com/teor2345/tor.git
I rewrote and simplified the build matrix.
We have the following builds on all platforms (Linux, macOS) and compilers (gcc, clang):
- no Rust (with hardening, which is the default)
- Rust online (without hardening due to #25386 (moved)/#26398 (moved))
We have the following builds on Linux gcc:
- coverage
- no hardening
- distcheck
- disable module dirauth
- Rust offline (without hardening due to #25386 (moved)/#26398 (moved))
- distcheck with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
- disable module dirauth with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
That gives us 15 builds rather than the previous 13, so CI should complete in a similar time. (I'm seeing 5 builders allocated at a time when building this branch.)
Trac:
Status: assigned to needs_reviewi386 macOS builds were really complex, so I didn't set them up. I'm happy to proceed with rust building on x86_64 for now.
Please see my branch travis-osx-v8 on https://github.com/teor2345/tor.git
Travis CI is at: https://travis-ci.org/teor2345/tor/builds/408372308
Replying to teor:
I rewrote and simplified the build matrix.
We have the following builds on all platforms (Linux, macOS) and compilers (gcc, clang):
- no Rust (with hardening, which is the default)
- Rust online (without hardening due to #25386 (moved)/#26398 (moved))
We have the following builds on Linux gcc:
- coverage
- no hardening
- distcheck
- disable module dirauth
- Rust offline (without hardening due to #25386 (moved)/#26398 (moved))
- distcheck with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
- disable module dirauth with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
That gives us 15 builds rather than the previous 13, so CI should complete in a similar time. (I'm seeing 5 builders allocated at a time when building this branch.)
The previous commit on master ran for 29 minutes: https://travis-ci.org/torproject/tor/builds/408312564
These new builds ran for 24 minutes. So I think we're fine.
Trac:
Status: needs_revision to needs_reviewReplying to teor:
Please see my branch travis-osx-v7 on https://github.com/teor2345/tor.git
I rewrote and simplified the build matrix.
We have the following builds on all platforms (Linux, macOS) and compilers (gcc, clang):
- no Rust (with hardening, which is the default)
- Rust online (without hardening due to #25386 (moved)/#26398 (moved))
We have the following builds on Linux gcc:
- coverage
- no hardening
- distcheck
- disable module dirauth
- Rust offline (without hardening due to #25386 (moved)/#26398 (moved))
- distcheck with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
- disable module dirauth with Rust online (without hardening due to #25386 (moved)/#26398 (moved))
That gives us 15 builds rather than the previous 13, so CI should complete in a similar time.
overall, I like this. I'm not super convinced that we need all these miscellaneous builds, e.g. no hardening when we already have Rust without hardening, but I guess they could help diagnose some CI failures, and I think we're far from exceeding the total maximum jobs.
(I'm seeing 5 builders allocated at a time when building this branch.)
unfortunately, I can't find where I found this information, but I believe the maximum at the time that I wrote "4/6" and "2/6" was the lower of: 5 Linux jobs, 2 Mac jobs, 6 total jobs (at any given time). therefore, adding Mac jobs and decreasing Linux jobs is expected to lower the total time.
I fixed a comment typo and some redundant env initialisations.
I also added a commit that sorts the config into chronological order, and groups related items together.
Then I added extra logging:
- rustup version for rust builds
- config.log for failures
And caching (see #26952 (moved)):
- ccache
- cargo cache for rust builds Cached builds are about 30% faster.
Please see travis-osx-v9. There were some fixups to earlier commits, so I force-pushed it.
I like a lot of these changes. Simplifying stuff so we don't have to list every matrix row explicitly seems quite helpful. I think there are a few minor changes remaining.
I think
$TRAVIS_BUILD_DIR/targetisn't the right directory for caching Rust build output; it's probably more correct to use$TRAVIS_BUILD_DIR/src/rust/target, because that's where we pointCARGO_TARGET_DIRin all our cargo invocations.Also we should probably stop setting
CARGO_HOMEexplicitly so more stuff ends up in$HOME/.cargoto cache. (see #26497 (moved); we could put the fix in that ticket, or maybe do it in this branch but make that ticket a child of this one)Trac:
Status: needs_review to needs_revisionTravis updated their default macOS image to macOS 10.13 / Xcode 9.4 last Tuesday 31 July: https://blog.travis-ci.com/2018-07-19-xcode9-4-default-announce
So I restarted the build of travis-osx-v9, and it seems to be fine: https://travis-ci.org/teor2345/tor/builds/408771608
I have two branches working their way through Travis right now:
- travis-osx-master
- travis-osx-034
They include all the changes from #26497 (moved), #26952 (moved), and they merge hello71's changes from #26038 (moved).
I would like to backport earlier than 0.3.4, but it's a bit more complicated, so I'm going to take a break and let travis complete.
Trac:
Status: needs_revision to needs_reviewOk, so this was quite challenging to backport, because there were a lot of little bugs. But I think it is worth it to have working CI.
Here are the branches I created today:
- travis-osx-029, which backports:
- the travis config rewrite
- macOS support
- fix a bug where missing macOS dependencies weren't being installed (#27090 (moved), but backported as a precaution)
- ccache (#26952 (moved))
- asciidoc (#27087 (moved))
- the distcheck configure options fix (#27088 (moved)) for consistency
- It passes Travis:
- A full matrix is running in Travis:
- travis-osx-032 backports:
- the lzma and zstd (macOS only) install from 0.3.1
- make zstd actually work on macOS (#27090 (moved))
- rust cargo online mode
- rust cargo offline mode
- It's running in Travis:
- A full matrix is running in Travis:
I'm going to leave Travis to run overnight, and hopefully it's all successful in the morning. I expect 0.3.3 to take a little while, and then 0.3.4 and 0.3.5 should be trivial.
Trac:
Status: needs_review to needs_revision- travis-osx-029, which backports:
Thanks! I'm commenting only on master, because I haven't looked at the others yet.
This mostly looks good.
mkdirinbefore_installshould create$TRAVIS_BUILD_DIR/src/rust/target, not$TRAVIS_BUILD_DIR/target.Please explicitly delete the
CARGO_HOMEsettings from src/rust/tor_rust/include.am and src/test/test_rust.sh instead of merging the changes from #26038 (moved). There are other changes in #26038 (moved) that I think we don't want, like:- deleting the
registrysetting in[source.crates-io]in src/rust/.cargo/config.in; I'm not sure whether this breaks anything, but I'd rather play it safe and leave it there until we know why we should delete it. - warning if
CARGO_HOMEis set; I'm not sure the added complexity is worth it, especially given that the checks are added in at least four places. - explicit setting of
target-dirin[build]in src/rust/.cargo/config.in; this seems redundant with the explicit setting ofCARGO_TARGET_DIRin various cargo invocations.
We could use
target-dirin the cargo config instead of explicitly settingCARGO_TARGET_DIRfor each cargo invocation, but in that case we should make a comment that it assumes that cargo is always invoked from a specific directory (which it should be after this change set).- deleting the
Replying to catalyst:
(I think these comments should go in #26038 (moved), but I'm responding to them here because you made them here.)
Please explicitly delete the
CARGO_HOMEsettings from src/rust/tor_rust/include.am and src/test/test_rust.sh instead of merging the changes from #26038 (moved). There are other changes in #26038 (moved) that I think we don't want, like:- deleting the
registrysetting in[source.crates-io]in src/rust/.cargo/config.in; I'm not sure whether this breaks anything, but I'd rather play it safe and leave it there until we know why we should delete it.
I believe cargo vendor used to print this line a long time ago, and now it has been the default for some time, so they stopped printing it. I could be remembering wrong though.
- warning if
CARGO_HOMEis set; I'm not sure the added complexity is worth it, especially given that the checks are added in at least four places.
This is not correct. My patch only checks if CARGO_HOME is set once. It is supposed to check separately if CARGO_HOME is a relative path at use time, on the basis that the user may have changed their CARGO_HOME since they used configure. However, it does this wrong, since it should actually error instead of just warn, and also there is more than one place that CARGO_HOME is used. I think I do agree that checking for relative path can be done a single time in configure with AC_ARG_VAR though.
- explicit setting of
target-dirin[build]in src/rust/.cargo/config.in; this seems redundant with the explicit setting ofCARGO_TARGET_DIRin various cargo invocations.
We could use
target-dirin the cargo config instead of explicitly settingCARGO_TARGET_DIRfor each cargo invocation, but in that case we should make a comment that it assumes that cargo is always invoked from a specific directory (which it should be after this change set).This is also not correct, although it is my fault for writing a poor comment. Paths in cargo configuration are interpreted relative to the parent of the .cargo directory, so as long as the correct cargo config is read, the target will always be set to the same place. Therefore, "./target" is equivalent to writing "@abs_top_builddir@/src/rust/target". Whether it is read or not, however, is unrelated to CARGO_TARGET_DIR, but in fact the CARGO_HOME. I assume however that if you can set CARGO_HOME correctly then you can also set the current directory correctly.
- deleting the
Let's deal with #26038 (moved) in a separate branch after these branches merge.
Replying to catalyst:
Thanks! I'm commenting only on master, because I haven't looked at the others yet.
This mostly looks good.
mkdirinbefore_installshould create$TRAVIS_BUILD_DIR/src/rust/target, not$TRAVIS_BUILD_DIR/target.Done in my 0.3.2 and later branches.
Please explicitly delete the
CARGO_HOMEsettings from src/rust/tor_rust/include.am and src/test/test_rust.sh instead of merging the changes from #26038 (moved). There are other changes in #26038 (moved) that I think we don't want...Done in my 0.3.2 branch, and re-done in 0.3.3 because a file moved.
Replying to teor:
Ok, so this was quite challenging to backport, because there were a lot of little bugs. But I think it is worth it to have working CI.
(And it's good to have consistent CI as well.)
Here are the revised branches I created:
- travis-osx-029 contains:
- the travis config rewrite
- macOS support
- fix a bug where missing macOS dependencies weren't being installed (#27090 (moved), but backported as a precaution)
- ccache (#26952 (moved))
- asciidoc (#27087 (moved))
- the distcheck configure options fix (#27088 (moved)) for consistency
- It passes Travis:
- And the full matrix also passes Travis:
- (I added a changes file and fixed some typos after the linked builds)
- travis-osx-032 also contains:
- the lzma and zstd (macOS only) install from 0.3.1
- make zstd actually work on macOS (#27090 (moved))
- rust cargo online mode
- rust cargo offline mode
- backport src/test/test_rust.sh from master
- stop setting $CARGO_HOME (#26497 (moved))
- Fix $abs_top_srcdir in test_rust.sh (#27093 (moved))
- Use cargo cache (#26952 (moved))
- It passes Travis:
- And the full matrix also passes Travis:
- (I added a changes file and fixed some typos after the linked builds)
- travis-osx-033 also contains:
- stop setting $CARGO_HOME, but in a different include.am file (#26497 (moved))
- use a consistent working directory in builds and tests (#26497 (moved))
- I didn't backport this patch to 0.3.2, because it has a different (but consistent) working directory
- Use TOR_RUST_DEPENDENCIES (the name changed in 0.3.3)
- Check Rust with distcheck (backport from 0.3.4, not supported in 0.3.2)
- use --all-features in test_rust.sh for 0.3.3 and 0.3.4 (existing code)
- It passes Travis:
- And the full matrix is running in Travis:
- travis-osx-034 also contains:
- Check disable module dirauth with and without rust (#27088 (moved) fix)
- It is running in Travis:
- And the full matrix is running in Travis:
- travis-osx-master also contains:
- revert use --all-features in test_rust.sh for 0.3.3 and 0.3.4
- It is running in Travis:
- And the full matrix is running in Travis:
I don't know if we want to fix #27091 (moved) in jenkins or in tor, I've asked weasel on IRC. We shouldn't merge until we work out how to fix jenkins.
Trac:
Status: needs_revision to needs_review- travis-osx-029 contains:
I added a missing asciidoc dependency to the macOS build and merged it forward.
New builds are under https://travis-ci.org/teor2345/tor/branches . I made some custom travis configs that run macOS asciidoc, their branches end in "-all". (We shouldn't merge the "-all" branches, because their travis configs run all the combinations.)
Here's the first branch where macOS asciidoc worked:
Weasel fixed #27091 (moved) for jenkins, but we need a long-term fix based on whatever rust does, or whatever debian does to package rust crates.
Overall this looks very good! I think this is a lot of work and will make things more maintainable in the future.
I would like to know if it ends up being significantly slower to default to using offline builds for most
--enable-rustbuilds instead of cached online builds. I think the more places we use--enable-cargo-online-mode, the more places we can have trouble with a transient network failure.Trac:
Status: needs_review to needs_informationPlease merge my branches from https://github.com/teor2345/tor.git
- travis-osx-029 to maint-0.2.9
- travis-osx-032 to maint-0.3.2
- travis-osx-033 to maint-0.3.3
- travis-osx-034 to maint-0.3.4
- travis-osx-master to master
The follow-up tickets are:
- #27252 (moved) in 0.3.5 - reduce online rust (and look for other redundant jobs)
- #27162 (moved) in 0.3.6 - consider running Travis on beta, nightly, and tor's lowest supported rust
mentioned in issue #24630 (moved)
mentioned in issue #26038 (moved)
mentioned in issue #26497 (moved)
mentioned in issue #26564 (moved)
mentioned in issue #26952 (moved)
mentioned in issue #26972 (moved)
mentioned in issue #27087 (moved)
mentioned in issue #27088 (moved)
