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 should not download the full code repository on which it depends. #14689

Closed
silence-coding opened this issue Oct 15, 2024 · 4 comments
Closed
Labels
A-git Area: anything dealing with git C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Performance Gotta go fast! S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@silence-coding
Copy link

silence-coding commented Oct 15, 2024

Problem

like #14687

Some of the problems are solved in the previous issue. However, when the Cargo.lock file is carried and is not a GitHub code repository, the cargo will go to the GitReference::Rev branch due to the lock file. As a result, the cargo will download all tags.

GitReference::Rev(rev) => {
if rev.starts_with("refs/") {
refspecs.push(format!("+{0}:{0}", rev));
} else if let Some(oid_to_fetch) = oid_to_fetch {
refspecs.push(format!("+{0}:refs/commit/{0}", oid_to_fetch));
} else if !matches!(shallow, gix::remote::fetch::Shallow::NoChange)
&& rev.parse::<Oid>().is_ok()
{
// There is a specific commit to fetch and we will do so in shallow-mode only
// to not disturb the previous logic.
// Note that with typical settings for shallowing, we will just fetch a single `rev`
// as single commit.
// The reason we write to `refs/remotes/origin/HEAD` is that it's of special significance
// when during `GitReference::resolve()`, but otherwise it shouldn't matter.
refspecs.push(format!("+{0}:refs/remotes/origin/HEAD", rev));
} else {
// We don't know what the rev will point to. To handle this
// situation we fetch all branches and tags, and then we pray
// it's somewhere in there.
refspecs.push(String::from("+refs/heads/*:refs/remotes/origin/*"));
refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD"));
tags = true;
}
}
}
if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
return fetch_with_cli(repo, remote_url, &refspecs, tags, gctx);

Proposed Solution

In the fetchall tag scenario, if the content in the source is a tag, the tag branch is used.

[[package]]
name = "hyper"
version = "0.14.30"
source = "git+https://open.codehub.com/hyperium/hyper.git?tag=v0.14.30#618a18580ae14776c05ab6789a82b5c43281fe8b"

Notes

No response

@silence-coding silence-coding added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Oct 15, 2024
@weihanglo
Copy link
Member

Please provide a reproduction. Thank you.

@weihanglo
Copy link
Member

Okay I see what you mean. rev could be an arbitrary git ref. Cargo cannot understand which revision it is looking for without downloading everything.

We have a specialized branch for GitHub because

  • GitHub is one of the major Git hosting services that is worth a special optimization.
  • GitHub's Git Server has the allow-reachable-sha1-in-want and provides API for that. Other Git hosting service must have such ability so we can investigate

See what we did for GitHub:

Hence please provide the platform you want to speed up with their docs about Git capabilities and API support. We will evaluate whether the case is worth a dedicated solution.

Worth noting that tags fetch is not the major contributing factor of fetching Git dependencies, shallow clones is the game changer which we already have an unstable feature waiting for feedback.

@weihanglo
Copy link
Member

There might be another approach of tackling this: if the locked dependency has a tag information, try fetching the tag first. If the tag has no commit we want, fall back to full fetch.

I would say this may add too much complexity and most users on GitHub already get benefits from GitHub fast path and won't gain any value from this. To me this solution may not be worth hitting our heads hard.

@weihanglo weihanglo added A-git Area: anything dealing with git Performance Gotta go fast! S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Oct 15, 2024
@silence-coding
Copy link
Author

You're right. The current cost of pulling full tags is not high, and the community should not pay too much for customization. We can wait until the shallow clones is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Performance Gotta go fast! S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

2 participants