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

cargo-install: build directory should be in $TEMPDIR #2606

Closed
japaric opened this issue Apr 22, 2016 · 1 comment · Fixed by #2610
Closed

cargo-install: build directory should be in $TEMPDIR #2606

japaric opened this issue Apr 22, 2016 · 1 comment · Fixed by #2610

Comments

@japaric
Copy link
Member

japaric commented Apr 22, 2016

and each instance of cargo-install should have its own build directory.

Right now cargo-install uses a target-install directory in $PWD. So you run into issues like these:

You don't have permission to write to $PWD:

$ cd /
$ cargo install cargo-graph
    Updating registry `https://github.com/rust-lang/crates.io-index`
 Downloading cargo-graph v0.2.2
error: failed to compile `cargo-graph v0.2.2`, intermediate artifacts can be found at `/target-install`

Caused by:
  failed to open: /target-install/release/.cargo-lock

To learn more, run the command again with --verbose.

Calling cargo install multiple times from the same directory will build each crate sequentially instead of in parallel (because the build directory gets locked -- without the lock things would be worse, so removing the lock is not a solution)

$ cargo install cargo-graph & cargo install cargo-count
[1] 28438
    Updating registry `https://github.com/rust-lang/crates.io-index`
    Blocking waiting for file lock on the registry index
    Updating registry `https://github.com/rust-lang/crates.io-index`
    Blocking waiting for file lock on the registry index
    Blocking waiting for file lock on crate metadata
    Blocking waiting for file lock on build directory
   Compiling rustc-serialize v0.3.19
(...)

If the build directory was $(mktemp -d) then you wouldn't have problem calling cargo install from a directory you don't permission to write to, and you could spawn several cargo install (from the same directory) that actually run in parallel.

Thoughts?

@alexcrichton
Copy link
Member

The problem with tempdirs is that if you ctrl-c they end up lying around and they're hard to find to remove. I wonder though if Cargo could just try a few locations? For example if $PWD isn't workable because it's not writable, it could switch to a deterministic directory in /tmp. Otherwise if it fails it could switch to cargo-install2 or something like that.

Although I guess it's just amounting to a tempdir that's slightly more readable...

Meh, I guess just using a tempdir seems fine to me!

japaric pushed a commit to japaric/cargo that referenced this issue Apr 22, 2016
…ctory

and each cargo-install instance creates and uses its own build directory. This
allows running several cargo-install instances in parallel.

If we fail to create a temporary directory for whatever reason fallback to
creating and using a target-install directory in the current directory.

closes rust-lang#2606
japaric pushed a commit to japaric/cargo that referenced this issue Apr 23, 2016
bors added a commit that referenced this issue Apr 24, 2016
cargo-install: prefer building artifacts in the system temporary directory

and each cargo-install instance creates and uses its own build directory. This
allows running several cargo-install instances in parallel.

If we fail to create a temporary directory for whatever reason fallback to
creating and using a target-install directory in the current directory.

closes #2606

---

r? @alexcrichton

Qs:
- Should we preserve the current behavior (`target-install` in `cwd`) as a fallback or remove it and error if we can't create a `TempDir` in `env::temp_dir()`? (we currently error if we can't create `target-install` directory in `cwd`)

- Should I add tests for the issues I raised at #2606? If yes, how can I test `cargo-install` parallelism? Lack of "Blocking waiting for file lock on build directory" in the output of the `cargo` commands? or something else?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants