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

Allow setting cargo home from .cargo/home #9154

Closed
wants to merge 1 commit into from

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Feb 9, 2021

This patch allows users to set the Cargo home directory through the file
system using a pointer file in .cargo/home. As with .cargo/config,
the file is searched for recursively up the tree from the current
working directory. Unlike with .cargo/config, the search terminates
when a matching file is found.

The search for .cargo/home happens before any .cargo/config files
are read, as that may be affected by the choice of Cargo home directory.

The CARGO_HOME environment variable is preferred over .cargo/home.

Fixes #6452.

This patch allows users to set the Cargo home directory through the file
system using a pointer file in `.cargo/home`. As with `.cargo/config`,
the file is searched for recursively up the tree from the current
working directory. Unlike with `.cargo/config`, the search terminates
when a matching file is found.

The search for `.cargo/home` happens before any `.cargo/config` files
are read, as that may be affected by the choice of Cargo home directory.

The `CARGO_HOME` environment variable is preferred over `.cargo/home`.

Fixes rust-lang#6452.
@rust-highfive
Copy link

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2021
@jonhoo jonhoo marked this pull request as ready for review February 9, 2021 01:31
@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 9, 2021

I'm guessing this will also have to go through the RFC process before being stabilized. Unfortunately, given that this a change to code that runs before parsing experimental flags, I'm not quite sure how to avoid it being insta-stable?

@matthiaskrgr
Copy link
Member

I'm the author of a crate that uses the home crate to get the location of the CARGO_HOME, I'm wondering if something will break if the home crate is not updated to support .cargo/home in cases this lands.

@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 9, 2021

@matthiaskrgr That's a good point — this change should probably be made to home rather than land here directly. I figured this was a better place to get the discussion started though, and to have a way to test it without making it "official" quite yet :)

@Eh2406
Copy link
Contributor

Eh2406 commented Feb 9, 2021

This will have to be tested on windows. Lookin up the .cargo/config can be a significant part of total runtime. If this 2x that overhead when the feature is not used, it could be a problem.

@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 9, 2021

It may be that .cargo/home should not recurse, and only ever be project-local. Though that does get weird if a user runs cargo in a subdirectory of their project..

@joshtriplett
Copy link
Member

joshtriplett commented Feb 9, 2021

CARGO_HOME controls many different things in Cargo. Do you need the ability to change the location of everything in ~/.cargo, or are you primarily looking to move Cargo's caches and similar? Would a variable or configuration option to set the latter suffice for your use case?

We talked about this in the Cargo team meeting today, and we were interested in trying to address the underlying use case, rather than jumping to solution space right away. We had a number of concerns about this kind of "double lookup" approach.

I'm also curious if symlinks might help for your use case.

@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 9, 2021

Ah, sure, let me give some context.

In my case, I have a separate build system that builds Rust packages using cargo. That build system runs in a build jail on a build fleet, but is also invoked by developers in their local development environments.

The build system sets CARGO_HOME primarily for two reasons. The first is the registry cache — the build system generates a dedicated local registry for each package that replaces crates.io, and if we used the user-shared CARGO_HOME, we'd end up polluting it with lots of registry caches that may have long-since been deleted. The other reason is that, on the build fleet, the home directory is read-only, so if we didn't set it, builds would fail.

There is a third reason too that is somewhat more minor, and that is that it's considered a feature of the build system that everything is contained inside the build directory. If we read configuration files outside of the build directory, it's harder to debug failing builds as they may fail due to "unmanaged configuration" in the user's setup. And if we write outside of the build directory, then it may no longer be sufficient to blow away the build directory to get a fresh build, or worse yet, we may end up corrupting some of the user's non-build files (though that's less of a risk with cargo).

The reason why I'd like to be able to set CARGO_HOME through a file is so that developers who run cargo directly, or indirectly through rust-analyzer and friends, end up using the same Cargo directories as the build system does. This is primarily to ensure that they don't end up building dependencies multiple times, but also to avoid keeping multiple copies of each registry cache and such.

@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 16, 2021

A different solution for this is rust-lang/rustup#2458 (comment), which would let the build system inject a "wrapper" toolchain that sets CARGO_HOME before the real cargo is invoked. Not sure if that's better (it requires rustup for one), but anyone interested in this may want to watch that issue as well.

@alexcrichton
Copy link
Member

Could this perhaps be altered for your use case to only change the cache directories? For example the registry directory or git checkout directory. It sounds like that would still solve the duplicate-checkouts or duplicate-caches problem that you'd like to prevent, and it wouldn't require a new feature for changing the home directory.

Cargo reading configuration hierarchically up to the root of the filesystem and unconditionally in the home directory is sort of a known issue and bug at this point. I would like to get to a world where Cargo doesn't hierarchically look up for configuration really all that often and there are at least CLI flags to disable configuration detection if desired (e.g. in your build system integration use case).

A downside of changing just the cache directory, however, is that this starts to get caught up in design with features like #9178 which has languished for quite a long time at this point. It seems reasonable to at least add unstable support for a feature like this for the time being though since we'll inevitably always want the ability to configure where the cache directories are located somehow.

@jonhoo
Copy link
Contributor Author

jonhoo commented Feb 19, 2021

Yes, I think being able to just override the cache directories would also get us pretty far. If that happens though, I'm not sure what else remains in CARGO_HOME? But seems reasonable to me.

Between that and the rustup solution, I'm going to close this, since either of those will be better than the large change that this CR proposes.

@jonhoo jonhoo closed this Feb 19, 2021
@clzoc
Copy link

clzoc commented Dec 29, 2021

CARGO_HOME controls many different things in Cargo. Do you need the ability to change the location of everything in ~/.cargo, or are you primarily looking to move Cargo's caches and similar? Would a variable or configuration option to set the latter suffice for your use case?

We talked about this in the Cargo team meeting today, and we were interested in trying to address the underlying use case, rather than jumping to solution space right away. We had a number of concerns about this kind of "double lookup" approach.

I'm also curious if symlinks might help for your use case.

#9021 (comment).
I am searching the method to change the location of git/crates cache by /ProjectName/Cargo.toml or others without disturbing the existing CARGO_HOME. I have tried to specify [env] in /ProjectName/.cargo/config.toml but it didn't work. Besides, cargo vendor is not convenient for modifying and developing. Would you mind tell me the solution or the progress of it. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to specify CARGO_HOME via file
7 participants