Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CommandType struct #3198

Merged
merged 2 commits into from
Oct 13, 2016
Merged

Remove CommandType struct #3198

merged 2 commits into from
Oct 13, 2016

Conversation

matklad
Copy link
Member

@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
Member 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.

@@ -33,6 +33,10 @@ pub struct Compilation<'cfg> {
/// Output directory for rust dependencies
pub deps_output: PathBuf,

/// Library search path for compiler plugins and build scripts
/// which have dynamic dependencies.
pub plugins_dylib_path: PathBuf,
Copy link
Member 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
Collaborator

bors commented Oct 13, 2016

📌 Commit 7165bd5 has been approved by alexcrichton

@bors
Copy link
Collaborator

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
Collaborator

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...

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.

None yet

4 participants