Add target directory parameter --target-dir #5393
Conversation
|
r? @matklad |
src/cargo/util/config.rs
Outdated
| @@ -70,6 +70,8 @@ pub struct Config { | |||
| cache_rustc_info: bool, | |||
| /// Creation time of this config, used to output the total build time | |||
| creation_time: Instant, | |||
| /// Target Directory via resolved Cli parameter | |||
| cli_target_dir: Option<Filesystem>, | |||
There was a problem hiding this comment.
I think that instead of cli_target_dir, we can store computed target_dir. That is, we can change fn target_dir() to
fn target_dir(&self) -> Option<FileSystem> { self.target_dir.clone() }
and instead move all logic from it to configure method. I think that would be slightly more clear, because we'll only store something, instead of both storing and computing on the fly. What do you think?
There was a problem hiding this comment.
Agreed!
I had some problems having the logic in configure as it would change some error messages with bad configs. But I was able to minimize it to just a bad_config::invalid_global_config situation, so it should be good.
tests/testsuite/build.rs
Outdated
| let exe_name = format!("foo{}", env::consts::EXE_SUFFIX); | ||
|
|
||
| assert_that( | ||
| p.cargo("build").arg("--target-dir").arg("foo/target"), |
There was a problem hiding this comment.
We've recently added a nicer API for this: p.cargo("build --target-dir foo/target").
|
☔ The latest upstream changes (presumably #5404) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r+ Thanks! |
|
📌 Commit 0b530c3 has been approved by |
Add target directory parameter --target-dir Implements: #5308 Adds a target directory parameter, that acts in the same manner as the environment variable `CARGO_TARGET_DIR`, to the following subcommands: - `bench` - `build` - `check` - `clean` - `doc` - `package` - `publish` - `run` - `rustc` - `rustdoc` - `test`
|
☀️ Test successful - status-appveyor, status-travis |
|
@matklad Thanks for the mentoring, it was a great learning experience! |
Commit 0b530c3 (which this commit mostly reverts) did some refactoring around the `target_dir` function. However, it introduced a bug because it meant that where `CARGO_TARGET_DIR` was specified but `--target-dir` was not, the value from `CARGO_TARGET_DIR` was ignored.
|
was |
|
I miss commandline option for cargo too. |
|
I think |
Implements: #5308
Adds a target directory parameter, that acts in the same manner as the environment variable
CARGO_TARGET_DIR, to the following subcommands:benchbuildcheckcleandocpackagepublishrunrustcrustdoctest