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

Allow specifying a custom output directory #1657

Merged
merged 2 commits into from Jun 4, 2015

Conversation

Projects
None yet
9 participants
@alexcrichton
Member

alexcrichton commented May 28, 2015

This commit adds support to allow specifying a custom output directory to Cargo.
First, the build.target-dir configuration key is checked, and failing that the
CARGO_TARGET_DIR environment variable is checked, and failing that the root
package's directory joined with the directory name "target" is used.

There are a few caveats to switching target directories, however:

  • If the target directory is in the current source tree, and the folder name is
    not called "target", then Cargo may walk the output directory when determining
    whether a tree is fresh.
  • If the target directory is not called "target", then Cargo may look inside it
    currently for Cargo.toml files to learn about local packages.
  • Concurrent usage of Cargo will still result in badness (#354), and this is now
    exascerbated because many Cargo projects can share the same output directory.
  • The top-level crate is not cached for future compilations, so if a crate is
    built into directory foo and then that crate is later used as a dependency,
    it will be recompiled.

The naming limitations can be overcome in time, but for now it greatly
simplifies the crawling routines and shouldn't have much of a negative impact
other than some Cargo runtimes (which can in turn be negated by following the
"target" name convention).

Closes #482

alexcrichton added some commits May 28, 2015

Allow specifying a custom output directory
This commit adds support to allow specifying a custom output directory to Cargo.
First, the `build.target-dir` configuration key is checked, and failing that the
`CARGO_TARGET_DIR` environment variable is checked, and failing that the root
package's directory joined with the directory name "target" is used.

There are a few caveats to switching target directories, however:

* If the target directory is in the current source tree, and the folder name is
  not called "target", then Cargo may walk the output directory when determining
  whether a tree is fresh.
* If the target directory is not called "target", then Cargo may look inside it
  currently for `Cargo.toml` files to learn about local packages.
* Concurrent usage of Cargo will still result in badness (#354), and this is now
  exascerbated because many Cargo projects can share the same output directory.
* The top-level crate is not cached for future compilations, so if a crate is
  built into directory `foo` and then that crate is later used as a dependency,
  it will be recompiled.

The naming limitations can be overcome in time, but for now it greatly
simplifies the crawling routines and shouldn't have much of a negative impact
other than some Cargo runtimes (which can in turn be negated by following the
"target" name convention).

Closes #482
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive May 28, 2015

r? @huonw

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

rust-highfive commented May 28, 2015

r? @huonw

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

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 28, 2015

Member

r? @wycats

cc @larsbergstrom, @metajack, I believe you guys have wanted this in Servo for some time now

Member

alexcrichton commented May 28, 2015

r? @wycats

cc @larsbergstrom, @metajack, I believe you guys have wanted this in Servo for some time now

@rust-highfive rust-highfive assigned wycats and unassigned huonw May 28, 2015

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack May 28, 2015

Am I correct that this is a solution for target directory sharing?

metajack commented May 28, 2015

Am I correct that this is a solution for target directory sharing?

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 28, 2015

Member

@metajack yeah, I would expect Servo to check in a top-level .cargo/config which specifies the target directory, and then all sub-projects will share that target directory.

Alternatively Servo's build system would set the CARGO_TARGET_DIR environment variable for all Cargo invocations.

Member

alexcrichton commented May 28, 2015

@metajack yeah, I would expect Servo to check in a top-level .cargo/config which specifies the target directory, and then all sub-projects will share that target directory.

Alternatively Servo's build system would set the CARGO_TARGET_DIR environment variable for all Cargo invocations.

@larsbergstrom

This comment has been minimized.

Show comment
Hide comment
@larsbergstrom

larsbergstrom commented May 28, 2015

@alexcrichton thank you very much!

@huonw

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 29, 2015

Member

yeah, I would expect Servo to check in a top-level .cargo/config which specifies the target directory, and then all sub-projects will share that target directory.

Hm, doesn't checking in .cargo/config play slightly badly with overrides? (i.e. one would very rarely want to check in/publish a local override.)

Member

huonw commented May 29, 2015

yeah, I would expect Servo to check in a top-level .cargo/config which specifies the target directory, and then all sub-projects will share that target directory.

Hm, doesn't checking in .cargo/config play slightly badly with overrides? (i.e. one would very rarely want to check in/publish a local override.)

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 29, 2015

Member

Right, but .cargo/config can be located at any point in a project hierarchy, so I would expect that overrides wouldn't be placed in servo/.cargo/config but rather a more specific location.

Member

alexcrichton commented May 29, 2015

Right, but .cargo/config can be located at any point in a project hierarchy, so I would expect that overrides wouldn't be placed in servo/.cargo/config but rather a more specific location.

@huonw

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 29, 2015

Member

For servo it's probably OK, I'm more thinking about this as a general footgun.

Member

huonw commented May 29, 2015

For servo it's probably OK, I'm more thinking about this as a general footgun.

@huonw

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 29, 2015

Member

(I don't really have anything concrete to offer to fix/mitigate it, and have no idea if it'll be a problem in practice.)

Member

huonw commented May 29, 2015

(I don't really have anything concrete to offer to fix/mitigate it, and have no idea if it'll be a problem in practice.)

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Jun 4, 2015

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned wycats Jun 4, 2015

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jun 4, 2015

Contributor

@bors r+

Contributor

brson commented Jun 4, 2015

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 4, 2015

Contributor

📌 Commit 014765f has been approved by brson

Contributor

bors commented Jun 4, 2015

📌 Commit 014765f has been approved by brson

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 4, 2015

Contributor

⌛️ Testing commit 014765f with merge 3b16446...

Contributor

bors commented Jun 4, 2015

⌛️ Testing commit 014765f with merge 3b16446...

bors added a commit that referenced this pull request Jun 4, 2015

Auto merge of #1657 - alexcrichton:share-target-dirs, r=brson
This commit adds support to allow specifying a custom output directory to Cargo.
First, the `build.target-dir` configuration key is checked, and failing that the
`CARGO_TARGET_DIR` environment variable is checked, and failing that the root
package's directory joined with the directory name "target" is used.

There are a few caveats to switching target directories, however:

* If the target directory is in the current source tree, and the folder name is
  not called "target", then Cargo may walk the output directory when determining
  whether a tree is fresh.
* If the target directory is not called "target", then Cargo may look inside it
  currently for `Cargo.toml` files to learn about local packages.
* Concurrent usage of Cargo will still result in badness (#354), and this is now
  exascerbated because many Cargo projects can share the same output directory.
* The top-level crate is not cached for future compilations, so if a crate is
  built into directory `foo` and then that crate is later used as a dependency,
  it will be recompiled.

The naming limitations can be overcome in time, but for now it greatly
simplifies the crawling routines and shouldn't have much of a negative impact
other than some Cargo runtimes (which can in turn be negated by following the
"target" name convention).

Closes #482
@bors

This comment has been minimized.

Show comment
Hide comment
@bors
Contributor

bors commented Jun 4, 2015

@bors bors merged commit 014765f into rust-lang:master Jun 4, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@gentoo90

This comment has been minimized.

Show comment
Hide comment
@gentoo90

gentoo90 Jun 4, 2015

@alexcrichton, could you add 0.3.0 tag to this commit in git?

gentoo90 commented on 655c40b Jun 4, 2015

@alexcrichton, could you add 0.3.0 tag to this commit in git?

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 4, 2015

Owner

Ah this wasn't actually the release of 0.3.0, we're more on the "0.3.0 nightly" branch right now. When this bumps to 0.4.0 I'll tag the 0.3.0 release.

Owner

alexcrichton replied Jun 4, 2015

Ah this wasn't actually the release of 0.3.0, we're more on the "0.3.0 nightly" branch right now. When this bumps to 0.4.0 I'll tag the 0.3.0 release.

@alexcrichton alexcrichton deleted the alexcrichton:share-target-dirs branch Jun 15, 2015

@Ryman Ryman referenced this pull request May 16, 2016

Merged

Cargo integration #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment