-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 needs network access if a library's build script calls cargo metadata
since 1.84
#15272
Comments
Yes, the obvious answer is "this already affected any dependency that did this and had examples" (bins not being relevant because artifact deps aren't stable). There is a less obvious case: without a |
For me, the question would be "what is it they are trying to accomplish by doing this and what is the right solution?" |
Another problem with using |
@epage valid question about what we're trying to accomplish. Here's our usage: https://github.com/aya-rs/rustc-llvm-proxy/blob/d0cbf35567ced6e62533135c65e5c208b1838003/build.rs This crate generates code based on the code in another crate. We use |
I had assumed this was being done because the source crate was optional, but it isn't. Is there a reason you don't just embed the source in your own crate? |
I'm not sure I follow. The source crate version is specified by the end user, we don't want to ride the treadmill of that crate's release schedule. We just look at its sources, whatever the end user decided they should be, and generate code from them. |
So the motivation here is to avoid re-releasing every time the source releases? In either case, this feels like a type of artifact dependencies for the raw content. Its something we could explore adding, dependent on analysis of how blessed this workflow should be. It does feel weird to have an artifact type like this when the only use for it is accessing data. We'd be blessing the concept of a accessing a data artifact but not creating one. |
On second examination, it does look like we explicitly depend on the source crate, so we do end up releasing whenever they do (more or less). If there's an ergonomic (read: not manual, and without violating licensing) way to embed upstream's source, we'd be open to that. |
Problem
This is another unintentional behavior change with #13447 dd698ff.
Because in 1.84 Cargo also includes
Cargo.lock
for library only crate.If a dependency has a build script that invokes
cargo metadata
, and there is a lockfile at the package root of the dependency, Cargo respects the lockfile and download necessary crate versions it needs.And it causes a problem that the lockfile might have an outdated/unused dependency than our local package never used. For example, our local package may resolves to
[email protected]
, but the lockfile stays in[email protected]
. Then thecargo metadata
call with the package will have extra downloads of useless crates, as they never shown up in our dependency graph.It is usually not a concern if people have network access, but not when without.
We already have an issue about
-Zavoid-dev-deps
and #14794 thatcargo metadata
are downloading dev-dependencies, which is not necessary if the call is from some tools or a build script of a dependency.This failure mode not only makes me concern the offline usage. If
cargo metadata
from a build script respects the lockfile from it, and resolves to different dependency versions, does that mean the result of callingcargo metadata
from a dependency's build script is inaccurate? Of course if they don't want any dependency information, they should callcargo metadata --no-deps
instead. However, the aya-rustc-llvm-proxy does want to know about dependencies, and might resolve to wrong versions.Surely this is already a problem before #13447, but with #13447 it might affect more people.
Steps
And you'll see
Possible Solution(s)
I don't know if it is considered a bug. Given it is pretty common in a build script
cargo metadata
is called. This will affect more and more people working in a network-contained environment.Notes
No response
Version
The text was updated successfully, but these errors were encountered: