Skip to content

Conversation

@matklad
Copy link
Contributor

@matklad matklad commented Jul 24, 2016

Target dir is now an API of Workspace. It is still initialized eagerly, just later.

I also had to errors in the config file when retrieving verbose and color config. Seems fishy, but is probably OK.

matklad added 3 commits July 23, 2016 04:51
While it is generally terrible to silently ignore errors in the
configuration files, it is acceptable in this case. Verbosity and color
config have reasonable defaults, and we don't want to fail for simple
commands like `cargo --version` just because of the garbage in the
config file.

fixed rust-lang#2848
@rust-highfive
Copy link

r? @alexcrichton

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

@alexcrichton
Copy link
Member

@bors: r+ 886c878

Thanks @matklad!

@bors
Copy link
Contributor

bors commented Jul 25, 2016

⌛ Testing commit 886c878 with merge 43d4259...

bors added a commit that referenced this pull request Jul 25, 2016
Move `target_dir` to Workspace and fix #2848

Target dir is now an API of Workspace. It is still initialized eagerly, just later.

I also had to errors in the config file when retrieving `verbose` and `color` config. Seems fishy, but is probably OK.
@bors
Copy link
Contributor

bors commented Jul 25, 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 43d4259 to master...

@bors bors merged commit 886c878 into rust-lang:master Jul 25, 2016
@matklad matklad deleted the move-target-dir branch August 19, 2016 22:45
github-merge-queue bot pushed a commit that referenced this pull request Aug 12, 2025
### What does this PR try to resolve?

While preparing to stabilize `build-dir`, [it was
discovered](#14125 (comment))
that `cargo package` will sometimes use the build cache for dependency
crates if `CARGO_TARGET_DIR` is explicitly set.

If target-dir is not set, the build will be preformed in an "inner
target dir" at `target/package/{name}-{version}/target` and rebuild
everything.

This PR changes the default behavior to always use the build cache,
matching the behavior of having `target-dir` set.

### How to test and review this PR?

Added a dedicated test to verify the previous behavior (cargo recompiled
non-workspace crates) in the first commit.
Then updated it to verify the new behavior.

### Related links/notes

* I did some Git archaeology and I _think_ this behavior was introduced
in #2912
* While searching through the commit/pr history was never able to find
any explicit reason why the behavior is the way it is.
* Even if it was intentional, it has been 9 years since and Cargo's
fingerprinting/rebuild detection is much more mature now so I believe
its worth revisiting if this is still the desired behavior.
* Would help mitigate #14941
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