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 ExecEngine abstraction #3177

Merged
merged 1 commit into from
Oct 7, 2016
Merged

Conversation

matklad
Copy link
Member

@matklad matklad commented Oct 7, 2016

Hi! Not sure what was the idea behind exec engine (perhaps to allow swapping it out during tests?), but looks like it does absolutely nothing at the moment, and can be removed.

@rust-highfive
Copy link

r? @alexcrichton

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

@alexcrichton
Copy link
Member

This was added way long ago in #1107 at the time to enable projects like cargo-emscripten. @tomaka is this still necessary or have those related projects since moved on?

@tomaka
Copy link
Contributor

tomaka commented Oct 7, 2016

@alexcrichton I wrote the ExecEngine stuff in order to add flags when Cargo invokes Rust. In other words, exactly what cargo rustc now does.
So I guess it can be removed as it is superseded by cargo rustc.

@alexcrichton
Copy link
Member

@bors: r+

Ok, thanks for the info @tomaka! @matklad there may be further cleanup from jettisoning this, haven't looked too closely yet though. If you find anything then feel free to take an axe to it!

@bors
Copy link
Contributor

bors commented Oct 7, 2016

📌 Commit 9f6a8a7 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Oct 7, 2016

⌛ Testing commit 9f6a8a7 with merge 09a955c...

bors added a commit that referenced this pull request Oct 7, 2016
Remove ExecEngine abstraction

Hi! Not sure what was the idea behind exec engine (perhaps to allow swapping it out during tests?), but looks like it does absolutely nothing at the moment, and can be removed.
@bors
Copy link
Contributor

bors commented Oct 7, 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 09a955c to master...

@bors bors merged commit 9f6a8a7 into rust-lang:master Oct 7, 2016
bors added a commit that referenced this pull request Oct 12, 2016
Remove command prototype

A followup of #3177 . I am not sure, but perhaps we can remove/refactor `CommandType` as well: for each command variant, `Compilation` as a public dedicated method, but it also has a generic one (`process`) (haha: https://github.com/rust-lang/cargo/pull/1107/files#r22429844).
@matklad matklad deleted the kill-exec-engine 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.

5 participants