Skip to content

Commit

Permalink
Add target directory parameter: address suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
smithsps committed Apr 24, 2018
1 parent dd0b7a2 commit 0b530c3
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 27 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl<'cfg> Workspace<'cfg> {
/// root and all member packages. It will then validate the workspace
/// before returning it, so `Ok` is only returned for valid workspaces.
pub fn new(manifest_path: &Path, config: &'cfg Config) -> CargoResult<Workspace<'cfg>> {
let target_dir = config.target_dir()?;
let target_dir = config.target_dir();

let mut ws = Workspace {
config,
Expand Down Expand Up @@ -191,7 +191,7 @@ impl<'cfg> Workspace<'cfg> {
ws.target_dir = if let Some(dir) = target_dir {
Some(dir)
} else {
ws.config.target_dir()?
ws.config.target_dir()
};
ws.members.push(ws.current_manifest.clone());
ws.default_members.push(ws.current_manifest.clone());
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn install_one(
let mut needs_cleanup = false;
let overidden_target_dir = if source_id.is_path() {
None
} else if let Some(dir) = config.target_dir()? {
} else if let Some(dir) = config.target_dir() {
Some(dir)
} else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() {
let p = td.path().to_owned();
Expand Down
33 changes: 15 additions & 18 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +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>,
/// Target Directory via resolved Cli parameter
target_dir: Option<Filesystem>,
}

impl Config {
Expand Down Expand Up @@ -115,7 +115,7 @@ impl Config {
crates_io_source_id: LazyCell::new(),
cache_rustc_info,
creation_time: Instant::now(),
cli_target_dir: None,
target_dir: None,
}
}

Expand Down Expand Up @@ -242,17 +242,8 @@ impl Config {
&self.cwd
}

pub fn target_dir(&self) -> CargoResult<Option<Filesystem>> {
if let Some(ref dir) = self.cli_target_dir {
Ok(Some(dir.clone()))
} else if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
Ok(Some(Filesystem::new(self.cwd.join(dir))))
} else if let Some(val) = self.get_path("build.target-dir")? {
let val = self.cwd.join(val.val);
Ok(Some(Filesystem::new(val)))
} else {
Ok(None)
}
pub fn target_dir(&self) -> Option<Filesystem> {
self.target_dir.clone()
}

fn get(&self, key: &str) -> CargoResult<Option<ConfigValue>> {
Expand Down Expand Up @@ -500,17 +491,23 @@ impl Config {
| (None, None, None) => Verbosity::Normal,
};

let cli_target_dir = match target_dir.as_ref() {
Some(dir) => Some(Filesystem::new(dir.clone())),
None => None,
let target_dir = if let Some(dir) = target_dir.as_ref() {
Some(Filesystem::new(self.cwd.join(dir)))
} else if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
Some(Filesystem::new(self.cwd.join(dir)))
} else if let Ok(Some(val)) = self.get_path("build.target-dir") {
let val = self.cwd.join(val.val);
Some(Filesystem::new(val))
} else {
None
};

self.shell().set_verbosity(verbosity);
self.shell().set_color_choice(color.map(|s| &s[..]))?;
self.extra_verbose = extra_verbose;
self.frozen = frozen;
self.locked = locked;
self.cli_target_dir = cli_target_dir;
self.target_dir = target_dir;
self.cli_flags.parse(unstable_flags)?;

Ok(())
Expand Down
5 changes: 4 additions & 1 deletion tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,10 @@ fn invalid_global_config() {
p.cargo("build").arg("-v"),
execs().with_status(101).with_stderr(
"\
[ERROR] Couldn't load Cargo configuration
error: failed to parse manifest at `[..]`
Caused by:
Couldn't load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]`
Expand Down
8 changes: 3 additions & 5 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3688,7 +3688,7 @@ fn custom_target_dir_line_parameter() {
let exe_name = format!("foo{}", env::consts::EXE_SUFFIX);

assert_that(
p.cargo("build").arg("--target-dir").arg("foo/target"),
p.cargo("build --target-dir foo/target"),
execs().with_status(0),
);
assert_that(
Expand Down Expand Up @@ -3721,7 +3721,7 @@ fn custom_target_dir_line_parameter() {
)
.unwrap();
assert_that(
p.cargo("build").arg("--target-dir").arg("bar/target"),
p.cargo("build --target-dir bar/target"),
execs().with_status(0),
);
assert_that(
Expand All @@ -3738,9 +3738,7 @@ fn custom_target_dir_line_parameter() {
);

assert_that(
p.cargo("build")
.arg("--target-dir")
.arg("foobar/target")
p.cargo("build --target-dir foobar/target")
.env("CARGO_TARGET_DIR", "bar/target"),
execs().with_status(0),
);
Expand Down

0 comments on commit 0b530c3

Please sign in to comment.