Skip to content

replace llvm-rebuild-trigger with submodule commit hash#59303

Merged
bors merged 1 commit intorust-lang:masterfrom
euclio:remove-rebuild-trigger
Mar 29, 2019
Merged

replace llvm-rebuild-trigger with submodule commit hash#59303
bors merged 1 commit intorust-lang:masterfrom
euclio:remove-rebuild-trigger

Conversation

@euclio
Copy link
Contributor

@euclio euclio commented Mar 19, 2019

As mentioned in #59285.

This PR removes the need to update the llvm-rebuild-trigger file. Instead, the latest commit hash of the appropriate LLVM submodule will be stored in the stamp file and used to detect if a build is required.

Fixes #42405.
Fixes #54959.
Fixes #55537.

@rust-highfive
Copy link
Contributor

rust-highfive commented Mar 19, 2019

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 19, 2019
@rust-highfive
Copy link
Contributor

rust-highfive commented Mar 20, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
travis_time:end:0e283fe8:start=1553039102026059358,finish=1553039197332546611,duration=95306487253
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
Setting environment variables from .travis.yml
---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:16:52] 
[01:16:52] running 120 tests
[01:17:17] .iiiii...i.....i..i...i..i.i...i.ii...i.....i..i....i..........iiii..........i...ii...i.......ii.i.i 100/120
[01:17:21] .i......iii.i.....ii
[01:17:21] 
[01:17:21]  finished in 29.141
[01:17:21] travis_fold:end:test_debuginfo

---
[01:45:31] test builder::__test::test_with_no_doc_stage0 ... ok
[01:45:31] 
[01:45:31] failures:
[01:45:31] 
[01:45:31] ---- builder::__test::build_default stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] 
[01:45:31] ---- builder::__test::build_with_target_flag stdout ----
[01:45:31] ---- builder::__test::build_with_target_flag stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_baseline stdout ----
[01:45:31] ---- builder::__test::dist_baseline stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_with_hosts stdout ----
[01:45:31] ---- builder::__test::dist_with_hosts stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_with_same_targets_and_hosts stdout ----
[01:45:31] ---- builder::__test::dist_with_same_targets_and_hosts stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_with_target_flag stdout ----
[01:45:31] ---- builder::__test::dist_with_target_flag stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_with_targets stdout ----
[01:45:31] ---- builder::__test::dist_with_targets stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::dist_with_targets_and_hosts stdout ----
[01:45:31] ---- builder::__test::dist_with_targets_and_hosts stdout ----
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] ---- builder::__test::test_exclude stdout ----
[01:45:31] Skipping Set({"src/tools/tidy"}) because it is excluded
[01:45:31] Suite("src/test/ui") not skipped for "test::Ui" -- not in ["src/test/run-pass", "src/tools/tidy"]
[01:45:31] Suite("src/test/ui") not skipped for "test::Ui" -- not in ["src/test/run-pass", "src/tools/tidy"]
[01:45:31] thread 'main' panicked at 'git could not determine LLVM submodule commit hash: fatal: Needed a single revision
[01:45:31] ', src/bootstrap/native.rs:87:13
[01:45:31] 
[01:45:31] failures:
[01:45:31]     builder::__test::build_default
[01:45:31]     builder::__test::build_with_target_flag
---
[01:45:31] 
[01:45:31] 
[01:45:31] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:45:31] Build completed unsuccessfully in 0:40:16
[01:45:31] make: *** [check] Error 1
[01:45:31] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0ec1f8b4
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Mar 20 01:32:18 UTC 2019
---
travis_time:end:189b2b4e:start=1553045539726213899,finish=1553045539732469580,duration=6255681
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:03e7a346
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:08966ff6
travis_time:start:08966ff6
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:01ab1139
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@euclio euclio force-pushed the remove-rebuild-trigger branch from b2ebcd9 to 60a091d Compare March 20, 2019 13:32
@cuviper
Copy link
Member

cuviper commented Mar 21, 2019

In CI, some of the submodules (especially LLVM) are unpacked from plain archives, not git:

rust/src/ci/init_repo.sh

Lines 57 to 63 in 48e354d

if [[ " $included " = *" $module "* ]]; then
commit="$(git ls-tree HEAD $module | awk '{print $3}')"
git rm $module
url=${urls[$i]}
url=${url/\.git/}
fetch_github_commit_archive $module "$url/archive/$commit.tar.gz" &
continue

Maybe we can have that fetch_github_commit_archive add a marker with the commit hash?

Or per #59285 (comment), the rebuild trigger doesn't really matter for CI anyway, so perhaps this git check just needs to fail gracefully.

@cuviper
Copy link
Member

cuviper commented Mar 21, 2019

Anyone building from rustc-src tarballs will also lack git info.

@euclio
Copy link
Contributor Author

euclio commented Mar 24, 2019

@cuviper For those building with rust-src tarballs, is making the git check fail gracefully sufficient?

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 24, 2019

The worst thing that can happen if the git commit cannot be fetched is CMake being re-configured and then no-op build being performed (that's about 10 seconds on my machine).
This is annoying when you do local development and run x.py often, but the tarball scenarios are probably not about local development.

@cuviper
Copy link
Member

cuviper commented Mar 24, 2019

I agree, it should be fine to just assume a rebuild is needed in that case.

@euclio euclio force-pushed the remove-rebuild-trigger branch 2 times, most recently from 3c18f0c to cdb623b Compare March 24, 2019 17:06
@euclio
Copy link
Contributor Author

euclio commented Mar 24, 2019

Ok, I modified the git check to print a message if there's an error pulling the submodule commit and then proceed with the LLVM build.

@cuviper
Copy link
Member

cuviper commented Mar 25, 2019

LGTM, but we should wait for #59285 to land, as there will be a merge conflict on the removed file.

@bors
Copy link
Collaborator

bors commented Mar 27, 2019

☔ The latest upstream changes (presumably #59285) made this pull request unmergeable. Please resolve the merge conflicts.

@Centril
Copy link
Contributor

Centril commented Mar 27, 2019

#59285 has landed.

To reduce Niko's backlog, r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned nikomatsakis Mar 27, 2019
@euclio euclio force-pushed the remove-rebuild-trigger branch from cdb623b to e1daa36 Compare March 27, 2019 14:06
@euclio
Copy link
Contributor Author

euclio commented Mar 27, 2019

Rebased.

@cuviper
Copy link
Member

cuviper commented Mar 27, 2019

Let's see, I should have reviewer privilege now -- of course, others are welcome to review too.

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 27, 2019

📌 Commit e1daa36 has been approved by cuviper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2019
@Centril
Copy link
Contributor

Centril commented Mar 28, 2019

Not including in rollup => @bors p=1

@Centril
Copy link
Contributor

Centril commented Mar 28, 2019

@bors rollup- p=1

@nagisa
Copy link
Member

nagisa commented Mar 28, 2019 •
edited
Loading

What would happen here when people change things within the submodule and then rebuild? Change things and rebuild multiple times?

@euclio
Copy link
Contributor Author

euclio commented Mar 28, 2019 via email

@cuviper
Copy link
Member

cuviper commented Mar 28, 2019

We could use :./path to parse rust's git index, instead of @:./path from HEAD. That still requires LLVM changes to be committed and the submodule updated in the index, but might make local testing a little easier.

If you want it to consider dirty state too, we'll need to use something besides git rev-parse, and it will require more filesystem operations every time to determine that. As a developer, you can also just remove the done stamp file to make it build again.

@bors
Copy link
Collaborator

bors commented Mar 29, 2019

⌛ Testing commit e1daa36 with merge 2002b4b...

bors added a commit that referenced this pull request Mar 29, 2019
@bors
Auto merge of #59303 - euclio:remove-rebuild-trigger, r=cuviper
2002b4b 
replace llvm-rebuild-trigger with submodule commit hash

As mentioned in #59285.

This PR removes the need to update the `llvm-rebuild-trigger` file. Instead, the latest commit hash of the appropriate LLVM submodule will be stored in the stamp file and used to detect if a build is required.

Fixes #42405.
Fixes #54959.
Fixes #55537.
@bors
Copy link
Collaborator

bors commented Mar 29, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: cuviper
Pushing 2002b4b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 29, 2019
@bors bors merged commit e1daa36 into rust-lang:master Mar 29, 2019
@jbeich jbeich mentioned this pull request Mar 30, 2019
Closed
@gyakovlev gyakovlev mentioned this pull request May 26, 2019
Closed
@euclio euclio deleted the remove-rebuild-trigger branch May 31, 2019 14:50
@Yourens
Copy link

Yourens commented May 20, 2020

How to rebuild if llvm directory is in dirty state?

@petrochenkov
Copy link
Contributor

petrochenkov commented May 20, 2020

@Yourens

rm build/<target>/llvm/llvm-finished-building

@Yourens
Copy link

Yourens commented May 20, 2020

@Yourens

rm build/<target>/llvm/llvm-finished-building

Thanks!!! That's really helpful for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nagisa nagisa

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

9 participants

@euclio @rust-highfive @cuviper @petrochenkov @bors @Centril @nagisa @Yourens @nikomatsakis