rustbuild: Fix MSBuild location of `llvm-config.exe` #48757

alexcrichton · 2018-03-05T20:10:47Z

For LLD integration the path to llvm-config needed to change to inside the
build directory itself (for whatever reason) but the build directory is
different on MSBuild than it is on ninja for MSVC builds, so the path to
llvm-config.exe was actually wrong and not working!

This commit removes the Build::llvm_config function in favor of the source of
truth, the Llvm build step itself. The build step was then updated to find the
right build directory for MSBuild as well as ninja for where llvm-config.exe
is located.

Closes #48749

rust-highfive · 2018-03-05T20:10:50Z

r? @aturon

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

Mark-Simulacrum

I think with the comment I'll consider this good to go.

Mark-Simulacrum · 2018-03-06T01:40:39Z

src/bootstrap/compile.rs

@@ -162,7 +162,11 @@ pub fn std_cargo(build: &Build,
        // missing
        // We also only build the runtimes when --enable-sanitizers (or its
        // config.toml equivalent) is used
-        cargo.env("LLVM_CONFIG", build.llvm_config(target));
+        let llvm_config = build.ensure(native::Llvm {
+            target: build.config.build,


Shouldn't this be the target we're passing to std_cargo? Or is this some other llvm-config? Either way, it'd be good to comment on the build.config.build with why we're passing it, and not target.

It also looks like it's maybe worth making this build.ensure(...) call the body of the old llvm_config function and then just calling it so we avoid duplicating it. But that's probably a minor detail, up to you.

Ah good point! I was hoping to keep the ensure here though to canonicalize the definition of where llvm-config comes from in one place (previously it was calculated in two places)

alexcrichton · 2018-03-07T23:21:31Z

re-r? @Mark-Simulacrum

Mark-Simulacrum · 2018-03-07T23:31:28Z

@bors r+

bors · 2018-03-07T23:31:31Z

📌 Commit d04a029 has been approved by Mark-Simulacrum

Zoxc · 2018-03-09T04:51:47Z

@bors p=1

Giving this a little boost so I can actually build rustc locally

bors · 2018-03-09T13:23:37Z

⌛ Testing commit d04a029ed36479840f33449230927533dfd1b4c3 with merge 4616ea633aa27d1361a573290501e70f1fda9203...

bors · 2018-03-09T14:48:15Z

💔 Test failed - status-travis

kennytm · 2018-03-09T14:53:30Z

dist x86_64-apple-darwin failed, seems legit.

[01:08:50] Building stage2 std artifacts (x86_64-apple-darwin -> aarch64-apple-ios)
[01:08:50] Building LLVM for aarch64-apple-ios
[01:08:50] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "target `aarch64-apple-ios` is not configured as a host, only as a target"', libcore/result.rs:945:5

For LLD integration the path to `llvm-config` needed to change to inside the build directory itself (for whatever reason) but the build directory is different on MSBuild than it is on `ninja` for MSVC builds, so the path to `llvm-config.exe` was actually wrong and not working! This commit removes the `Build::llvm_config` function in favor of the source of truth, the `Llvm` build step itself. The build step was then updated to find the right build directory for MSBuild as well as `ninja` for where `llvm-config.exe` is located. Closes rust-lang#48749

alexcrichton · 2018-03-09T15:29:21Z

@bors: r=Mark-Simulacrum

bors · 2018-03-09T15:29:22Z

📌 Commit be902e7 has been approved by Mark-Simulacrum

bors · 2018-03-09T16:18:24Z

⌛ Testing commit be902e7 with merge da31cb5...

rustbuild: Fix MSBuild location of `llvm-config.exe` For LLD integration the path to `llvm-config` needed to change to inside the build directory itself (for whatever reason) but the build directory is different on MSBuild than it is on `ninja` for MSVC builds, so the path to `llvm-config.exe` was actually wrong and not working! This commit removes the `Build::llvm_config` function in favor of the source of truth, the `Llvm` build step itself. The build step was then updated to find the right build directory for MSBuild as well as `ninja` for where `llvm-config.exe` is located. Closes #48749

bors · 2018-03-09T17:42:12Z

💔 Test failed - status-travis

pietroalbini · 2018-03-09T17:52:45Z

macOS crashed with a sigsegv while building stage2 cargo.

@bors retry #48866

bors · 2018-03-09T19:02:19Z

⌛ Testing commit be902e7 with merge 257ec08...

rustbuild: Fix MSBuild location of `llvm-config.exe` For LLD integration the path to `llvm-config` needed to change to inside the build directory itself (for whatever reason) but the build directory is different on MSBuild than it is on `ninja` for MSVC builds, so the path to `llvm-config.exe` was actually wrong and not working! This commit removes the `Build::llvm_config` function in favor of the source of truth, the `Llvm` build step itself. The build step was then updated to find the right build directory for MSBuild as well as `ninja` for where `llvm-config.exe` is located. Closes #48749

bors · 2018-03-09T21:46:54Z

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing 257ec08 to master...

rust-highfive assigned aturon Mar 5, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2018

Mark-Simulacrum requested changes Mar 6, 2018

View reviewed changes

alexcrichton force-pushed the fix-msbuild-build branch from 2c2ffe1 to d04a029 Compare March 6, 2018 03:16

rust-highfive assigned Mark-Simulacrum and unassigned aturon Mar 7, 2018

Mark-Simulacrum approved these changes Mar 7, 2018

View reviewed changes

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 7, 2018

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 Mar 9, 2018

kennytm 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 Mar 9, 2018

alexcrichton force-pushed the fix-msbuild-build branch from d04a029 to be902e7 Compare March 9, 2018 15:29

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 Mar 9, 2018

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 Mar 9, 2018

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 9, 2018

pietroalbini mentioned this pull request Mar 9, 2018

segfault during stage2 build of dist x86_64-apple-darwin xcode 7.3 #48866

Closed

bors merged commit be902e7 into rust-lang:master Mar 9, 2018

alexcrichton deleted the fix-msbuild-build branch March 9, 2018 21:48

kennytm mentioned this pull request Mar 15, 2018

When running Emscripten tests, rustbuild should tell compilertest the version of the Emscripten LLVM, not the host LLVM #49063

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustbuild: Fix MSBuild location of `llvm-config.exe` #48757

rustbuild: Fix MSBuild location of `llvm-config.exe` #48757

alexcrichton commented Mar 5, 2018

rust-highfive commented Mar 5, 2018

Mark-Simulacrum left a comment

Mark-Simulacrum Mar 6, 2018

alexcrichton Mar 6, 2018

alexcrichton commented Mar 7, 2018

Mark-Simulacrum commented Mar 7, 2018

bors commented Mar 7, 2018

Zoxc commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

kennytm commented Mar 9, 2018

alexcrichton commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

pietroalbini commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

rustbuild: Fix MSBuild location of llvm-config.exe #48757

rustbuild: Fix MSBuild location of llvm-config.exe #48757

Conversation

alexcrichton commented Mar 5, 2018

rust-highfive commented Mar 5, 2018

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Mark-Simulacrum Mar 6, 2018

Choose a reason for hiding this comment

alexcrichton Mar 6, 2018

Choose a reason for hiding this comment

alexcrichton commented Mar 7, 2018

Mark-Simulacrum commented Mar 7, 2018

bors commented Mar 7, 2018

Zoxc commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

kennytm commented Mar 9, 2018

alexcrichton commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

pietroalbini commented Mar 9, 2018

bors commented Mar 9, 2018

bors commented Mar 9, 2018

rustbuild: Fix MSBuild location of `llvm-config.exe` #48757

rustbuild: Fix MSBuild location of `llvm-config.exe` #48757