Skip to content

Conversation

@matklad
Copy link
Contributor

@matklad matklad commented Oct 13, 2016

This removes CommandType struct as well as cargo_rustc::process function. So now all process creation goes thorough methods of Compilation.

This does change search path order from util::dylib_path(), host_dylib_path() to host_dylib_path(), util::dylib_path(), but I hope this is not a problem.

This also uncovers the fact that rustdoc is run sometimes with and sometimes without host_dylib_path. Is this intentional?

@rust-highfive
Copy link

r? @alexcrichton

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

@matklad
Copy link
Contributor Author

matklad commented Oct 13, 2016

Is this intentional?

Ah, looks like it's simpler just to store that dylib path in compilation to avoid passing it around.


/// Library search path for compiler plugins and build scripts
/// which have dynamic dependencies.
pub plugins_dylib_path: PathBuf,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name's not really exciting :(

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Oct 13, 2016

📌 Commit 7165bd5 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Oct 13, 2016

⌛ Testing commit 7165bd5 with merge 5f2cc15...

bors added a commit that referenced this pull request Oct 13, 2016
Remove CommandType struct

This removes `CommandType` struct as well as `cargo_rustc::process` function. So now all process creation goes thorough methods of `Compilation`.

This does change search path order from `util::dylib_path(), host_dylib_path()` to `host_dylib_path(), util::dylib_path()`, but I hope this is not a problem.

This also uncovers the fact that `rustdoc` is run sometimes with and sometimes without `host_dylib_path`. Is this intentional?
@bors
Copy link
Contributor

bors commented Oct 13, 2016

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: alexcrichton
Pushing 5f2cc15 to master...

@bors bors merged commit 7165bd5 into rust-lang:master Oct 13, 2016
@matklad matklad deleted the kill-command-type branch December 14, 2016 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants