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 specifying global replacements in .cargo/config, not just crate Cargo.toml #3308

Closed
vvuk opened this issue Nov 21, 2016 · 9 comments
Closed

Comments

@vvuk
Copy link

vvuk commented Nov 21, 2016

the .cargo/config paths mechanism seems to be somewhat deprecated, or at least unmaintained -- e.g. if any of those crates have optional dependencies that aren't normally selected (e.g. clippy), a warning is printed that a dependency was added on the optional ones.

By contrast, [replace] works well, but it can only be specified in a crate's Cargo.toml, making it difficult to work with when the upstream is changing (requires rebasing your replacements to Cargo.toml, and making sure they don't get checked in accidentally).

We should be able to specify global replacements in .cargo/config.

@alexcrichton
Copy link
Member

The tl;dr; here is unfortunately this is not possible.

To explain a bit more in detail, this all boils down to how the two features interact with Cargo.lock. When you use [replace] in a manifest it takes this into account when creating Cargo.lock. So much so that replacements are themselves recorded in Cargo.lock. This has many benefits:

  • Everyone using that project will have the same replacement resolution. This is important for larger projects which have lots of [replace] in flight to ensure all builds are still consistent.
  • Future builds will also have the same resolution. This is mostly important locally in providing the guarantee that cargo build followed by cargo build doesn't download anything. Otherwise builds themselves are just consistent because the crate graph is always the same.
  • You can arbitrarily restructure the dependency graph with [replace] as it's all recorded in Cargo.lock. This means that what you [replace] with can change dependencies, add dependencies, etc.

Note that this is all possible because [replace] is located in Cargo.toml, meaning it's able to modify Cargo.lock.

Now paths, on the other hand, is a very historical feature in Cargo. It was basically literally the first thing implemented, and we just never got around to fixing it before Cargo's release. The paths feature is placed in .cargo/config, which crucially means that it cannot affect Cargo.lock resolution. As a result, it has few of the benefits mentioned above.

The main point is that if you can't modify Cargo.lock, then an override cannot restructure the crate graph. If the graph were restructured then there's no way we could restructure it deterministically over time, creating an endless stream of bug reports about spurious recompiles, spurious registry updates, etc. Note that this is precisely the bug in the paths implementation. It allows for restructuring the crate graph, which implies that nondeterministic restructuring happens, causing endless bugs in Cargo.

So with all that in mind, we've got two use case for replacing code with something locally:

  • Recorded in the manifest for everyone to benefit from
  • Recorded globally as you're just working on something temporarily

The first is [replace], stable, and works well (as you've noted). The latter has historically been paths but doesn't work well and the "more powerful" features of it are deprecated. Note, though, that the feature itself is not deprecated nor going away as it has a powerful and strong use case! In the future paths will just start to have a hard constraint that a replacement cannot restructure the dependency graph. In other words, if you don't get a warning today, then you're fine.

Does that all make sense? With that in mind we can't literally add [replace] to global configuration, but paths will always be around for overriding. In that sense this may boil down to #736 where we should provide the ability to perhaps have more targeted path overrides (like [replace]) where you can specify both a source and a version range that needs to be replaced.

@vvuk
Copy link
Author

vvuk commented Nov 22, 2016

The main point is that if you can't modify Cargo.lock

Okay, but -- why can't you modify Cargo.lock? Or rather: if I have some kind of global replace list, why can't it behave as if every single Cargo.toml that cargo read had those replacements specified? It should still certainly record their presence in Cargo.lock, and if they ever go away, the behaviour should be the same as if the Cargo.toml was changed to remove them.

I agree that such global configuration can lead to confusion, but there's already precedent, such as specifying rustflags in a .cargo/config. There are valid use cases for it (for example: I work on the webrender crate. I want to always use my local version of it, assuming versions match, without needing to modify Cargo.toml in anything that might depend on it, and without needing to carry that modification through my git tree. I can't use path overrides because it causes problems if I ever change deps.).

In the future paths will just start to have a hard constraint that a replacement cannot restructure the dependency graph. In other words, if you don't get a warning today, then you're fine.

Hm, then I think there's a bug here. I have a crate that has serde_derive listed as an optional dependency. Default features specify codegen, which does serde_codegen. If I specify a path override to that crate, which is literally exactly the same as the one on crates.io, I get a warning about a dep on serde_derive being either added or modified to not match. This is the same behaviour on other crates on any optional deps (e.g. any crate that has an optional dep on clippy).

@alexcrichton
Copy link
Member

The purpose of Cargo.lock is to provide reproducible builds at the crate graph level, but if we start reading global configuration which modifies Cargo.lock then that starts to erode. You could now forget there's a global replace and commit the Cargo.lock into the repo, and then it can be very difficult to reconstruct how to get the same crate graph.

The confusion part is basically the lynchpin of why this currently isn't allowed, basically.

Hm, then I think there's a bug here.

Oh dear! Can you file a bug?

@vvuk
Copy link
Author

vvuk commented Nov 23, 2016

Yep, the confusion would be bad -- but wouldn't it be mitigated by recording the replacements in Cargo.lock and reporting if they change? e.g.
Cargo.lock was generated with replacements that are now missing! Build may not be what you expect. or somesuch.

Oh dear! Can you file a bug?

Filed #3313 :)

@alexcrichton
Copy link
Member

Yeah we could provide a warning, but it wouldn't solve the problem of figuring out how to regenerate Cargo.lock (which is done on every build). In that sense we'd have to record everything but we also need to balance changes to Cargo.lock as having is oscillate too much can be bad for checking into repositories.

Thanks for filing the issue!

@larsbergstrom
Copy link

@wycats This discussion captures a lot of what we were talking about a few weeks ago around the need to only use [replace] for suitable situations (e.g., handling a CVE) and you identified both some work that should happen around paths and possibly another new feature. Do you have a writeup / RFC for that?

@jethrogb
Copy link
Contributor

jethrogb commented Apr 10, 2017

I was unpleasantly surprised by these warnings popping up after a recent upgrade.

I use git dependencies a lot. For development across git repositories, I have to use path overrides. I don't think I should have to make temporary changes to Cargo.toml that are never going to be checked in.

@carols10cents
Copy link
Member

The tl;dr; here is unfortunately this is not possible.

I'm closing this issue based on this comment; please reopen and update the title and description if I've misunderstood.

@snuk182
Copy link

snuk182 commented Mar 7, 2018

Another usecase for .cargo/config paths is the usage of Qt bindings generator. While pre-generated bindings exist at crates.io, thus can be referenced as a default stub, there is a variety of environments that contain Qt with variety of Qt versions, against which the pre-generated bingings are useless and should be generated in place. And in order to use them the dependant libraries should somehow reach them. Via config paths this is done transparently (I also like the dependency remap warning on cargo build). Via [patch] for the libraries I don't own this is impossible.

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

No branches or pull requests

6 participants