rustbuild: detect cxx for all targets #61765

Keruspe · 2019-06-12T10:25:41Z

We need CXX to build llvm-libunwind which can be enabled for alltargets.
As we needed it for all hosts anyways, just move the detection so that it is ran for all targets (which contains all hosts) instead.

Replaces rust-lang#61544 Fixes rust-lang#59917 We need CXX to build llvm-libunwind which can be enabled for all targets. As we needed it for all hosts anyways, just move the detection so that it is ran for all targets (which contains all hosts) instead. Signed-off-by: Marc-Antoine Perennou <[email protected]>

rust-highfive · 2019-06-12T10:25:51Z

r? @nikomatsakis

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

Keruspe · 2019-06-12T10:26:51Z

r? @alexcrichton

alexcrichton · 2019-06-12T14:15:10Z

I don't think this is quite right because it seems like it will cause a failure for any cross-compiled target which doesn't have a C++ compiler configure (which was the original intention). If one is configured the configuration should propagate through, otherwise we should continue to ignore unconfigured C++ compilers for just the standard library.

Keruspe · 2019-06-12T15:12:24Z

I'll do some further testing removing cxx from the i686 configuration to see how things go

Keruspe · 2019-06-13T08:02:20Z

Current master, cxx unset for i686, i686 in targets but not hosts:

CC_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-cc"                                                                                                                                                                                                                                                                                                                                                                                    CFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]                                                                                                                                                                                                                                                                                                                                             AR_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-ar"                                                                                                                                                                                                                                                                                                                                                                                    CC_i686-unknown-linux-gnu = "i686-pc-linux-gnu-cc"                                               
CFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]                                                                                                                                                                                                                                                                                                                                
AR_i686-unknown-linux-gnu = "i686-pc-linux-gnu-ar"                                                                                                                                                                                                                                                                                                                                                                                        
CXX_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-c++"

Current master, cxx unset for i686, i686 in targets and hosts:

CC_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-cc"                                                                                                                                                                                                                                                                                                                                                                                    
CFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]                                                                                                                                                                                                                                                                                                                                             
AR_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-ar"                                                                                                                                                                                                                                                                                                                                                                                    
CC_i686-unknown-linux-gnu = "i686-pc-linux-gnu-cc"                                                                                                                                                                                                                                                                                                                                                                                        
CFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]                                                                                                                                                                                                                                                                                                                                
AR_i686-unknown-linux-gnu = "i686-pc-linux-gnu-ar"                                                                                                                                                                   
CXX_i686-unknown-linux-gnu = "c++"                                                                                                                                                                                   
CXX_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-c++"

So the current code doesn't make anything fail if cxx is not defined, it just uses the fallback value of c++.

Now with this patch, i686 in target but not in hosts, cxx still unset:

CC_i686-unknown-linux-gnu = "i686-pc-linux-gnu-cc"
CFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]
CXX_i686-unknown-linux-gnu = "i686-pc-linux-gnu-cc"
CXXFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]
AR_i686-unknown-linux-gnu = "i686-pc-linux-gnu-ar"
CC_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-cc"
CFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]
CXX_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-c++"
CXXFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]
AR_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-ar"

It indeed produces a different result, as it uses the same value as CC for CXX instead of the fallback c++.

Making it behave the same is easy though, fixing right now

Signed-off-by: Marc-Antoine Perennou <[email protected]>

Keruspe · 2019-06-13T08:06:55Z

With this new patch added, we get the same behaviour as before this merge request when cxx is not set in config.toml:

CC_i686-unknown-linux-gnu = "i686-pc-linux-gnu-cc"
CFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]
CXX_i686-unknown-linux-gnu = "c++"
CXXFLAGS_i686-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m32", "-march=i686"]
AR_i686-unknown-linux-gnu = "i686-pc-linux-gnu-ar"
CC_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-cc"
CFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]
CXX_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-c++"
CXXFLAGS_x86_64-unknown-linux-gnu = ["-ffunction-sections", "-fdata-sections", "-fPIC", "-m64"]
AR_x86_64-unknown-linux-gnu = "x86_64-pc-linux-gnu-ar"

alexcrichton · 2019-06-13T13:52:26Z

I don't think I understand why the patch preserves the original behavior? All fallible locations have switched to being infallible, so where's the failure case handled of the compiler not being available?

Keruspe · 2019-06-13T14:09:09Z

Before these changes, CXX would not be set for targets that are not listed as hosts (the fallible locations as you call those) and cc would fail the build using the fallback c++.

After these changes, CXX is set for all targets, defaulting to the fallback c++ if not configured, thus failing exactly the same way as before if not configured, but working properly when configured.

Signed-off-by: Marc-Antoine Perennou <[email protected]>

Keruspe · 2019-06-13T14:22:35Z

With this last patch, it is only set for targets for which it has been configured, restoring the fallible locations

rust-highfive · 2019-06-13T14:29:30Z

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:062dcbb5:start=1560435826489793496,finish=1560435950409238255,duration=123919444759
$ 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
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
$ export TOOLSTATE_REPO=https://github.com/rust-lang-nursery/rust-toolstate.git
$ export TOOLSTATE_ISSUES_API_URL=https://api.github.com/repos/rust-lang/rust/issues
$ export CI_JOB_NAME=$TRAVIS_JOB_NAME
$ export IMAGE=x86_64-gnu-llvm-6.0
$ export RUST_BACKTRACE=1
$ bash -c 'echo $BASH_VERSION'
---
[00:02:41]    Compiling toml v0.4.10
[00:02:41]    Compiling serde_json v1.0.33
[00:02:44]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:03:13]     Finished dev [unoptimized] target(s) in 1m 20s
[00:03:13] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:13] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:03:13] Build completed unsuccessfully in 0:01:35
[00:03:13] make: *** [prepare] Error 1
[00:03:13] Makefile:69: recipe for target 'prepare' failed
[00:03:13] Makefile:69: recipe for target 'prepare' failed
[00:03:14] Command failed. Attempt 2/5:
[00:03:14]     Finished dev [unoptimized] target(s) in 0.17s
[00:03:14] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:14] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:03:14] Build completed unsuccessfully in 0:00:00
[00:03:14] make: *** [prepare] Error 1
[00:03:14] Makefile:69: recipe for target 'prepare' failed
[00:03:14] Makefile:69: recipe for target 'prepare' failed
[00:03:16] Command failed. Attempt 3/5:
[00:03:16]     Finished dev [unoptimized] target(s) in 0.17s
[00:03:16] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:16] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:03:16] Build completed unsuccessfully in 0:00:00
[00:03:16] make: *** [prepare] Error 1
[00:03:16] Makefile:69: recipe for target 'prepare' failed
[00:03:16] Makefile:69: recipe for target 'prepare' failed
[00:03:19] Command failed. Attempt 4/5:
[00:03:19]     Finished dev [unoptimized] target(s) in 0.17s
[00:03:19] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:19] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:03:19] Build completed unsuccessfully in 0:00:00
[00:03:19] make: *** [prepare] Error 1
[00:03:19] Makefile:69: recipe for target 'prepare' failed
[00:03:19] Makefile:69: recipe for target 'prepare' failed
[00:03:23] Command failed. Attempt 5/5:
[00:03:24]     Finished dev [unoptimized] target(s) in 0.17s
[00:03:24] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:24] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:03:24] Build completed unsuccessfully in 0:00:00
[00:03:24] Makefile:69: recipe for target 'prepare' failed
[00:03:24] make: *** [prepare] Error 1
---
travis_time:end:02c365cf:start=1560436166191651302,finish=1560436166196689278,duration=5037976
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2da72fe3
$ 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:1fc47876
travis_time:start:1fc47876
$ 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:01d767e0
$ 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)

Signed-off-by: Marc-Antoine Perennou <[email protected]>

Keruspe · 2019-06-14T08:11:58Z

@alexcrichton with this last commit, we should be back to the same behaviour as before, but cxx gets properly detected for targets for which it's configured.

I wonder though if instead of setting c++ for all hosts for which it hasn't been configured (as it's currently the case), we should only do that for the build host

Keruspe · 2019-06-16T21:11:25Z

Should I squash everything?

alexcrichton · 2019-06-18T17:36:39Z

src/bootstrap/cc_detect.rs

            }
            let alternative = format!("e{}", gnu_compiler);
            if Command::new(&alternative).output().is_ok() {
                cfg.compiler(alternative);
+                return true;


I think that the return true here and eslewehre in this function may not be what we want? This all seems largely about auto-inference of what gcc should be used, but it doesn't imply that it was configured this way in particular. Notably I think that we should only actually probe for a C++ compiler if it's explicitly configured.

Note though that the C++ compiler can be configured not only via config.toml but also the standard CXX_... variables that cc supports. In that sense I don't think the return value from this function is correct.

This was to behave exactly as before for hosts, but I'm 100% fine with dropping that.

We at least need to set it to the default 'c++' for the build hosts though otherwise the build fails on CI (or we need to explicitely configure cxx in the ci)

For hosts, yes, but this logic seems like if you configure a target for one of these targets and you're missing the inferred C++ compiler it will fail, right?

Signed-off-by: Marc-Antoine Perennou <[email protected]>

Keruspe · 2019-06-21T12:04:03Z

@alexcrichton is it clearer now?

alexcrichton · 2019-06-21T15:25:20Z

@bors: r+

Sure, we can go with this for now.

bors · 2019-06-21T15:25:21Z

📌 Commit 870f13a has been approved by alexcrichton

bors · 2019-06-22T05:06:52Z

⌛ Testing commit 870f13a with merge 08bd23c5c32b753a7ff0700a5d7deb03feeace6d...

bors · 2019-06-22T05:12:08Z

💔 Test failed - checks-travis

rust-highfive · 2019-06-22T05:12:09Z

The job dist-aarch64-linux 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.

[00:03:40] + python2.7 ../x.py dist --host aarch64-unknown-linux-gnu --target aarch64-unknown-linux-gnu
[00:03:41]     Finished dev [unoptimized] target(s) in 0.19s
[00:03:42] travis_fold:end:log-system-info
git could not determine the LLVM submodule commit hash. Assuming that an LLVM build is necessary.
[00:03:42] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `x86_64-unknown-linux-gnu` is not configured as a host, only as a target"', src/libcore/result.rs:999:5
[00:03:42] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host aarch64-unknown-linux-gnu --target aarch64-unknown-linux-gnu
[00:03:42] Build completed unsuccessfully in 0:00:01
travis_time:end:0b14f16c:start=1561180095610756890,finish=1561180320740464693,duration=225129707803
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
---
travis_time:end:0294ffd2:start=1561180321491628837,finish=1561180321497636306,duration=6007469
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00160e70
$ 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:0528b291
travis_time:start:0528b291
$ 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:0010f2c0
$ 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)

Keruspe · 2019-06-24T07:10:51Z

If I understand correctly, the failing ones are because the build host is not in hosts?

Even if it's not in hosts Signed-off-by: Marc-Antoine Perennou <[email protected]>

alexcrichton · 2019-06-25T11:46:09Z

@bors: r+

bors · 2019-06-25T11:46:11Z

📌 Commit 087cd77 has been approved by alexcrichton

bors · 2019-06-25T12:07:26Z

⌛ Testing commit 087cd77 with merge 40ab9d2...

rustbuild: detect cxx for all targets Replaces #61544 Fixes #59917 We need CXX to build llvm-libunwind which can be enabled for alltargets. As we needed it for all hosts anyways, just move the detection so that it is ran for all targets (which contains all hosts) instead.

bors · 2019-06-25T15:12:05Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 40ab9d2 to master...

rust-highfive assigned nikomatsakis Jun 12, 2019

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

rust-highfive assigned alexcrichton and unassigned nikomatsakis Jun 12, 2019

rustbuild: fix default value for cxx

00b6015

Signed-off-by: Marc-Antoine Perennou <[email protected]>

rustbuild: don't set cxx if not configured

fdecfe4

Signed-off-by: Marc-Antoine Perennou <[email protected]>

rustbuild: set cxx for hosts even if not configured

c682ac9

Signed-off-by: Marc-Antoine Perennou <[email protected]>

alexcrichton reviewed Jun 18, 2019

View reviewed changes

rustbuild: only autodetect cxx for hosts

870f13a

Signed-off-by: Marc-Antoine Perennou <[email protected]>

Keruspe force-pushed the rustbuild-cxx branch from cbf361a to 870f13a Compare June 21, 2019 12:02

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

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 22, 2019

jonas-schievink added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2019

rustbuild: always set cxx for build host

087cd77

Even if it's not in hosts Signed-off-by: Marc-Antoine Perennou <[email protected]>

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 25, 2019

bors merged commit 087cd77 into rust-lang:master Jun 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustbuild: detect cxx for all targets #61765

rustbuild: detect cxx for all targets #61765

Keruspe commented Jun 12, 2019

rust-highfive commented Jun 12, 2019

Keruspe commented Jun 12, 2019

alexcrichton commented Jun 12, 2019

Keruspe commented Jun 12, 2019

Keruspe commented Jun 13, 2019

Keruspe commented Jun 13, 2019

alexcrichton commented Jun 13, 2019

Keruspe commented Jun 13, 2019

Keruspe commented Jun 13, 2019

rust-highfive commented Jun 13, 2019

Keruspe commented Jun 14, 2019

Keruspe commented Jun 16, 2019

alexcrichton Jun 18, 2019

Keruspe Jun 20, 2019

alexcrichton Jun 21, 2019

Keruspe commented Jun 21, 2019

alexcrichton commented Jun 21, 2019

bors commented Jun 21, 2019

bors commented Jun 22, 2019

bors commented Jun 22, 2019

rust-highfive commented Jun 22, 2019

Keruspe commented Jun 24, 2019

alexcrichton commented Jun 25, 2019

bors commented Jun 25, 2019

bors commented Jun 25, 2019

bors commented Jun 25, 2019

rustbuild: detect cxx for all targets #61765

rustbuild: detect cxx for all targets #61765

Conversation

Keruspe commented Jun 12, 2019

rust-highfive commented Jun 12, 2019

Keruspe commented Jun 12, 2019

alexcrichton commented Jun 12, 2019

Keruspe commented Jun 12, 2019

Keruspe commented Jun 13, 2019

Keruspe commented Jun 13, 2019

alexcrichton commented Jun 13, 2019

Keruspe commented Jun 13, 2019

Keruspe commented Jun 13, 2019

rust-highfive commented Jun 13, 2019

Keruspe commented Jun 14, 2019

Keruspe commented Jun 16, 2019

alexcrichton Jun 18, 2019

Choose a reason for hiding this comment

Keruspe Jun 20, 2019

Choose a reason for hiding this comment

alexcrichton Jun 21, 2019

Choose a reason for hiding this comment

Keruspe commented Jun 21, 2019

alexcrichton commented Jun 21, 2019

bors commented Jun 21, 2019

bors commented Jun 22, 2019

bors commented Jun 22, 2019

rust-highfive commented Jun 22, 2019

Keruspe commented Jun 24, 2019

alexcrichton commented Jun 25, 2019

bors commented Jun 25, 2019

bors commented Jun 25, 2019

bors commented Jun 25, 2019