Search for target_triple.json only if builtin target not found #58601

gnzlbg · 2019-02-20T17:12:33Z

Before this commit, if the builtin target was found, but an error
happened when instantiating it (e.g. validating the target
specification file failed, etc.), then we ignored those errors
and proceeded to try to find a target_triple.json file, and if
that failed, reported that as an error.

With this commit, if rustc is supposed to provide the builtin target,
and something fails while instantiating it, that error will
get properly propagated.

r? @oli-obk

gnzlbg · 2019-02-20T17:13:32Z

cc @alexcrichton

src/librustc_target/spec/mod.rs

oli-obk · 2019-02-20T17:21:50Z

src/librustc_target/spec/mod.rs

+ match load_specific(target_triple) {
+ Ok(t) => return Ok(t);
+ LoadTargetError::BuiltinTargetNotFound(_) => (),
+ LoadTargetError::Other(e) => return Err(e),


I'm not so sure about these load errors, it seems to me that they are a compiler bug. Either we panic on them and test them, or we allow the user to work around them by specifying a json file. I'd prefer the former.

I'm not so sure about these load errors, it seems to me that they are a compiler bug.

When you target, say, an ios target, creating the Target executes code like this: https://github.com/rust-lang/rust/blob/master/src/librustc_target/spec/apple_ios_base.rs#L29

Which can fail, say on Linux, if that tool is not available.

So in that case, load target failed, for a reason that AFAICT isn't a compiler bug, but an user bug: its running the compiler without the appropriate tools in the path.

I don't know if reporting those errors breaks somebody's workflow. E.g. if they are relying on the builtin target failing for external reasons to override it with a json file. But that seems very brittle, and that use case is better addressed with a --target-file=file.json (or just --target=file.json) CLI option that explicitly requests obtaining the target from a json file.

that makes sense

rust-highfive · 2019-02-20T17:24:18Z

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:2bc5ddfd:start=1550682900316794114,finish=1550682901272244157,duration=955450043
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:08:50]    Compiling rustc-rayon v0.1.1
[00:08:54]    Compiling tempfile v3.0.5
[00:08:54]    Compiling rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:08:56]    Compiling arena v0.0.0 (/checkout/src/libarena)
[00:08:56] error: expected one of `,`, `.`, `?`, `}`, or an operator, found `;`
[00:08:56]      |
[00:08:56]      |
[00:08:56] 1188 |                     Ok(t) => return Ok(t);
[00:08:56]      |                           --             ^ expected one of `,`, `.`, `?`, `}`, or an operator here
[00:08:56]      |                           |
[00:08:56]      |                           while parsing the `match` arm starting here
[00:08:57]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:08:58] error: unreachable statement
[00:08:58]     --> src/librustc_target/spec/mod.rs:1194:17
[00:08:58]      |
[00:08:58]      |
[00:08:58] 1194 | /                 let path = {
[00:08:58] 1195 | |                     let mut target = target_triple.to_string();
[00:08:58] 1196 | |                     target.push_str(".json");
[00:08:58] 1197 | |                     PathBuf::from(target)
[00:08:58]      | |__________________^
[00:08:58]      |
[00:08:58]      = note: `-D unreachable-code` implied by `-D warnings`
[00:08:58] 
---
[00:09:01] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:09:01] expected success, got: exit code: 101
[00:09:01] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:09:01] Build completed unsuccessfully in 0:02:05
[00:09:01] make: *** [all] Error 1
[00:09:01] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:2424f1ca
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Feb 20 17:24:13 UTC 2019
---
travis_time:end:3677af2a:start=1550683453798676427,finish=1550683453803221705,duration=4545278
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:107f830b
$ 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:05346fa8
travis_time:start:05346fa8
$ 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:0c1420d4
$ 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)

rust-highfive · 2019-02-20T18:33:02Z

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:0da2b216:start=1550686929924600452,finish=1550686930819087585,duration=894487133
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:10:24]    Compiling rustc-rayon v0.1.1
[00:10:27]    Compiling rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:10:27]    Compiling tempfile v3.0.5
[00:10:30]    Compiling arena v0.0.0 (/checkout/src/libarena)
[00:10:30] error: expected one of `,`, `.`, `?`, `}`, or an operator, found `;`
[00:10:30]      |
[00:10:30]      |
[00:10:30] 1188 |                     Ok(t) => return Ok(t);
[00:10:30]      |                           --             ^ expected one of `,`, `.`, `?`, `}`, or an operator here
[00:10:30]      |                           |
[00:10:30]      |                           while parsing the `match` arm starting here
[00:10:30]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:10:31] error: unreachable statement
[00:10:31]     --> src/librustc_target/spec/mod.rs:1194:17
[00:10:31]      |
[00:10:31]      |
[00:10:31] 1194 | /                 let path = {
[00:10:31] 1195 | |                     let mut target = target_triple.to_string();
[00:10:31] 1196 | |                     target.push_str(".json");
[00:10:31] 1197 | |                     PathBuf::from(target)
[00:10:31]      | |__________________^
[00:10:31]      |
[00:10:31]      = note: `-D unreachable-code` implied by `-D warnings`
[00:10:31] 
---
[00:10:34] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:10:34] expected success, got: exit code: 101
[00:10:34] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:10:34] Build completed unsuccessfully in 0:02:06
[00:10:34] make: *** [all] Error 1
[00:10:34] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:05a93c0c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Feb 20 18:32:56 UTC 2019
---
travis_time:end:074f3d04:start=1550687577217707182,finish=1550687577222406625,duration=4699443
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04b7ea96
$ 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:162fe958
travis_time:start:162fe958
$ 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:053245f2
$ 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)

Before this commit, if the builtin target was found, but an error happened when instantiating it (e.g. validating the target specification file failed, etc.), then we ignored those errors and proceeded to try to find a `target_triple.json` file, and if that failed, reported that as an error. With this commit, if rustc is supposed to provide the builtin target, and something fails while instantiating it, that error will get properly propagated.

oli-obk · 2019-02-21T08:25:57Z

@bors r+ rollup

bors · 2019-02-21T08:25:58Z

📌 Commit 103ed0c has been approved by oli-obk

@oli-obk

Search for target_triple.json only if builtin target not found Before this commit, if the builtin target was found, but an error happened when instantiating it (e.g. validating the target specification file failed, etc.), then we ignored those errors and proceeded to try to find a `target_triple.json` file, and if that failed, reported that as an error. With this commit, if rustc is supposed to provide the builtin target, and something fails while instantiating it, that error will get properly propagated. r? @oli-obk

@oli-obk

Search for target_triple.json only if builtin target not found Before this commit, if the builtin target was found, but an error happened when instantiating it (e.g. validating the target specification file failed, etc.), then we ignored those errors and proceeded to try to find a `target_triple.json` file, and if that failed, reported that as an error. With this commit, if rustc is supposed to provide the builtin target, and something fails while instantiating it, that error will get properly propagated. r? @oli-obk

@ghost

Rollup of 17 pull requests Successful merges: - #57656 (Deprecate the unstable Vec::resize_default) - #58059 (deprecate before_exec in favor of unsafe pre_exec) - #58064 (override `VecDeque::try_rfold`, also update iterator) - #58198 (Suggest removing parentheses surrounding lifetimes) - #58431 (fix overlapping references in BTree) - #58555 (Add a note about 2018e if someone uses `try {` in 2015e) - #58588 (remove a bit of dead code) - #58589 (cleanup macro after 2018 transition) - #58591 (Dedup a rustdoc diagnostic construction) - #58600 (fix small documentation typo) - #58601 (Search for target_triple.json only if builtin target not found) - #58606 (Docs: put Future trait into spotlight) - #58607 (Fixes #58586: Make E0505 erronous example fail for the 2018 edition) - #58615 (miri: explain why we use static alignment in ref-to-place conversion) - #58620 (introduce benchmarks of BTreeSet.intersection) - #58621 (Update miri links) - #58632 (Make std feature list sorted) Failed merges: r? @ghost

rust-highfive assigned oli-obk Feb 20, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 20, 2019

oli-obk reviewed Feb 20, 2019

View reviewed changes

gnzlbg force-pushed the json_error branch from 68af8fd to 103ed0c Compare February 20, 2019 18:38

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 Feb 21, 2019

Centril mentioned this pull request Feb 22, 2019

Rollup of 18 pull requests #58637

Closed

Centril mentioned this pull request Feb 22, 2019

Rollup of 17 pull requests #58644

Merged

bors merged commit 103ed0c into rust-lang:master Feb 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search for target_triple.json only if builtin target not found #58601

Search for target_triple.json only if builtin target not found #58601

gnzlbg commented Feb 20, 2019

gnzlbg commented Feb 20, 2019

oli-obk Feb 20, 2019

gnzlbg Feb 20, 2019

gnzlbg Feb 20, 2019 •

edited

Loading

oli-obk Feb 21, 2019

rust-highfive commented Feb 20, 2019

rust-highfive commented Feb 20, 2019

oli-obk commented Feb 21, 2019

bors commented Feb 21, 2019

Search for target_triple.json only if builtin target not found #58601

Search for target_triple.json only if builtin target not found #58601

Conversation

gnzlbg commented Feb 20, 2019

gnzlbg commented Feb 20, 2019

oli-obk Feb 20, 2019

Choose a reason for hiding this comment

gnzlbg Feb 20, 2019

Choose a reason for hiding this comment

gnzlbg Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

oli-obk Feb 21, 2019

Choose a reason for hiding this comment

rust-highfive commented Feb 20, 2019

rust-highfive commented Feb 20, 2019

oli-obk commented Feb 21, 2019

bors commented Feb 21, 2019

gnzlbg Feb 20, 2019 •

edited

Loading