Path to registry index is not cross-platform stable (i.e. hash part of registry/index/index.crates.io-<hash>)
#14795
Comments
epage
added
Command-fetch
S-triage
|
Thanks for the bug report. Could you attach some additional information, like
cargo/src/cargo/ops/cargo_fetch.rs Lines 35 to 64 in cb088dc
Line 119 in cb088dc Bugs might be there if they exist. |
weihanglo
added
S-needs-info
|
Hi @weihanglo . This the version of cargo running on my container installed through zypper. The commands ran in that service is as follows.
I wouldn't know of a regression as this is the first time that I noticed this and my first bug report about it.
None. I just use whatever is the default.
So shouldn't the help section of
This seems to confuse me more as this was the only assumption I have from the help section but the behaviour for what I expected was different than what was described. Should fetch at least have the same behaviour as vendor that they both fetch everything in the dependency resolution by default? |
|
Hi. I tried using the |
Either this or make the behavior of both commands aligned. Let me try to get a minimal reproduction first. |
I've re-read cargo-fetch logic. While it may fail to fetch packages that have unstable features like It is surprising that in your build log it's crossbeam-channel missing If I read it correctly, crossbeam-channel is not even a platform-specific dependency. I wonder what is inside the |
|
It's not corrupted or missing since doing the following gives me this output The other variant which uses the Extracting it and running So I am not really sure how |
|
Do we have the correct index files under this location? Perhaps you can upload a tarball that failed to build and we can investigate together. |
Hi. It seems I am not able to upload a tarball. One thing you can do to perform a reproduction is by doing the following cargo install --git https://github.com/openSUSE-Rust/obs-service-cargo
git clone --depth 1 --branch v18.0.3 https://github.com/kakoune-lsp/kakoune-lsp/
cd kakoune-lsp
cargo_vendor --src $PWD --outdir /tmp
cd /tmp/
mkdir vendored-sources/
tar xf vendor.tar.zst -C vendored-sources/
cd vendored-sources/by now you can check the inside. It should contain a |
|
@uncomfyhalomacro the current obs-service-cargo openSUSE-Rust/obs-service-cargo@bde0300 generates only vendored source. There is no registry index file or tarball at all. I am not sure which commit I should use to reproduce. BTW, you could also run |
Oh i forgot to add the flag cargo_vendor --src $PWD --method registry --outdir /tmpJust to keep you updated, doing your command does this, |
|
I was so dumb! Just realized the hash of Given that, I believe |
weihanglo
changed the title
cargo fetch does not actually use all target architectures as the default.registry/index/index.crates.io-<hash>)
|
Sure! So the plan now is to create
not sure when that will happen but for now you can close this issue as that literally answered all my questions! |
|
Yes. As I said the stable path is best-effort. We don't guarantee the permanent path of it. I'll propose to the team for a stable hash next week (if I remember) Thanks for your understanding! |
|
Going to reopen and use this for proposing using stable hash for source URLs. tl;dr:
During the course of designing remap rules for The remaining question is the hash algorithm. In #14116 we tried out
Therefore, I favor and propose BLAKE3 as our stable hash algorithm for these reasons,
@rfcbot fcp merge Besides, I'd like to call out some area this stable hash proposal doesn't cover:
|
|
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
@weihanglo whats the cost if we change our mind on which hash we use in the future? The important part is its stable across platforms, right? And we're mainly signing off that the small performance hit for blake3 is fine? |
|
@epage I've clarified it in the FCP. There are two things:
If we don't want BLAKE3 we can continue using SipHash with The cost of changing hash algorithm is it |
Double checking, It doesn't thrash the cache (throw out on each switch, like RUSTFLAGS today for rlibs) but keeps the caches between the old and new version separate (like my PR for RUSTFLAGS for rlibs)? This would be a one-time extra download and extra space taken up until the user cleans it up or we stabilize GC. Not great but not the worst. iirc there are other parts of the project that are seeking out crytograhically secure hashes, making this line up with those, right? |
You're absolutely correct. Not trashing but extra downloads for newer versions.
Yes. A meeting is happening on Dec 6th rust-lang/compiler-team#774. It would be great if both rustc and cargo depend on the same cryptographically secure hash. We can wait for a conclusion from that meeting, though it is not a must. The scenarios are not the same. |
|
Can you say more about where this hash shows up? IIUC, it is Can you say more about the current situation? IIRC, it is using |
|
Sorry I should have copied them from #14116. Here they are:
Cargo currently uses This FCP proposes we use the cross-platform stable hash from |
|
If we're worried about hash collisions, we'll also need to ensure we're not truncating the hash in the directory name. Is there any concern with making the paths longer? |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
@rfcbot concern path-length if cryptographically secure, we need the full hash, which might blow path lengths on windows |
rfcbot
added
proposed-final-comment-period
This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang#14795 (despite they should use `cargo vendor` instead IMO). Some caveats, * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows MSVC is not really covered by the "cross-platform` hash There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also <rust-lang#13171 (comment)>
This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang#14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows is not really covered by the "cross-platform` hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also <rust-lang#13171 (comment)>
This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang#14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows is not really covered by the "cross-platform" hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also <rust-lang#13171 (comment)>
|
Yes, switching fixes that @rfcbot resolve path-length |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang#14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows is not really covered by the "cross-platform" hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also <rust-lang#13171 (comment)>
This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in rust-lang#14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Windows is not really covered by the "cross-platform" hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. See also <rust-lang#13171 (comment)>
### What does this PR try to resolve? This helps `-Ztrim-paths` build a stable cross-platform path for the registry and git sources. Sources files then can be found from the same path when debugging. It also helps cache registry index all at once for all platforms, for example the use case in #14795 (despite they should use `cargo vendor` instead IMO). Some caveats: * Newer cargo will need to re-download files for global caches (index files, git/registry sources). The old cache is still kept and used when running with older cargoes. * Absolute paths on windows iarenot really covered by the "cross-platform" hash, because path prefix components like `C:` are always there. That means hashes of some sources kind, like local registry and local path, are not going to be real cross-platform stable. #### Security concern There might be hash collisions if you have two registries under the same domain. This won't happen to crates.io, as the infra would have to intentionally put another registry on index.crates.io to collide. We don't consider this is an actual threat model, so we are not going to use any cryptographically secure hash algorithm like BLAKE3. At least, the current unstable SipHash isn't in a better situation. We might switch to a cryptographic secure one when needed. See also <#13171 (comment)> ### How should we test and review this PR? We have an FCP in <#14795 (comment)>. This PR implements the proposal, The path-length concern in <#14795 (comment)> is automatically addressed because we don't need cryptographically secure hash for now. ### Additional information See more information and benchmark results in <#14116>.
|
Resolved by #14917 |
Hello. I have an issue with
cargo-fetchbehaving differently thancargo-vendor. In a part of the help section ofcargo fetch, it says for the flag--targetHowever, I believe this is untrue since in some architectures, they cannot find their dependencies. I haven't tried the
--targetflag yet as a workaround but given that some target architectures cannot find their dependencies is weird to me, since it says that the default is to use "all" target architectures.Here is a comparison between
cargo vendorvscargo fetch.You can see here that in https://build.opensuse.org/package/show/home:uncomfyhalomacro:branches:editors:registry/kak-lsp, only aarch64, x86_64, and ppc64le are able to find their dependencies and compile (whether it fails or not). i586, armv7l, and s390x are the architectures that were not able to find their dependencies.
Everything in https://build.opensuse.org/package/show/home:uncomfyhalomacro:branches:editors:vendor/kak-lsp are able to find their dependencies and are able to compile (whether it fails or not).
I would like to know if there is an inaccuracy with the documentation or not.
For context, I've been helping out to build a tool to ease the process of vendoring dependencies in openSUSE/SUSE. https://github.com/openSUSE-Rust/obs-service-cargo.
The text was updated successfully, but these errors were encountered: