Skip to content

Support shallow fetch through net.git-fetch-with-cli #15315

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Support shallow fetch through net.git-fetch-with-cli #15315

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
280073f
Add shallow fetch via GitCli
Ygg01 Mar 15, 2025
3dd9df9
Add processing for Shallow
Ygg01 Mar 15, 2025
353bdef
Ignore other Shallow options
Ygg01 Mar 16, 2025
167f58d
WIP test
Ygg01 Mar 16, 2025
3a66b67
Works with Complete
Ygg01 Mar 17, 2025
e9833f9
Add shallow test
Ygg01 Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/cargo/sources/git/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::process::Command;
use std::str;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::{Duration, Instant};
use gix::protocol::fetch::Shallow;
use tracing::{debug, info};
use url::Url;

Expand Down Expand Up @@ -1031,7 +1032,7 @@ pub fn fetch(
}

let result = if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
fetch_with_cli(repo, remote_url, &refspecs, tags, gctx)
fetch_with_cli(repo, remote_url, &refspecs, tags, shallow, gctx)
} else if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) {
fetch_with_gitoxide(repo, remote_url, refspecs, tags, shallow, gctx)
} else {
Expand Down Expand Up @@ -1075,6 +1076,7 @@ fn fetch_with_cli(
url: &str,
refspecs: &[String],
tags: bool,
shallow: Shallow,
gctx: &GlobalContext,
) -> CargoResult<()> {
let mut cmd = ProcessBuilder::new("git");
Expand All @@ -1084,6 +1086,28 @@ fn fetch_with_cli(
} else {
cmd.arg("--no-tags");
}

match shallow {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we are using variants other than NoChange/DepthAtRemote. We might just leave then unimplmented!(…).

cargo/src/cargo/sources/git/mod.rs

Lines 35 to 57 in 611b7c4

pub(crate) fn to_shallow_setting(
&self,
repo_is_shallow: bool,
gctx: &GlobalContext,
) -> gix::remote::fetch::Shallow {
let has_feature = |cb: &dyn Fn(GitFeatures) -> bool| {
gctx.cli_unstable()
.git
.map_or(false, |features| cb(features))
};
// maintain shallow-ness and keep downloading single commits, or see if we can do shallow clones
if !repo_is_shallow {
match self {
RemoteKind::GitDependency if has_feature(&|features| features.shallow_deps) => {
}
RemoteKind::Registry if has_feature(&|features| features.shallow_index) => {}
_ => return gix::remote::fetch::Shallow::NoChange,
}
};
gix::remote::fetch::Shallow::DepthAtRemote(1.try_into().expect("non-zero"))
}

Shallow::NoChange => {},
Shallow::Deepen(deepen) => {
cmd.arg(format!("--deepen={deepen}"));
},
Shallow::DepthAtRemote(depth) => {
cmd.arg(format!("--depth={depth}"));
},
Shallow::Exclude { remote_refs, since_cutoff } => {
for remote_ref in remote_refs {
cmd.arg(format!("--shalow-exclude={remote_ref:?}"));
}
if let Some(cutoff) = since_cutoff {
cmd.arg(format!("--shallow-since={cutoff}"));
}
},
Shallow::Since { cutoff } => {
cmd.arg(format!("--shallow-since={cutoff}"));
},
}

match gctx.shell().verbosity() {
Verbosity::Normal => {}
Verbosity::Verbose => {
Expand Down Expand Up @@ -1119,7 +1143,7 @@ fn fetch_with_gitoxide(
remote_url: &str,
refspecs: Vec<String>,
tags: bool,
shallow: gix::remote::fetch::Shallow,
shallow: Shallow,
gctx: &GlobalContext,
) -> CargoResult<()> {
let git2_repo = repo;
Expand Down Expand Up @@ -1227,7 +1251,7 @@ fn fetch_with_libgit2(
remote_url: &str,
refspecs: Vec<String>,
tags: bool,
shallow: gix::remote::fetch::Shallow,
shallow: Shallow,
gctx: &GlobalContext,
) -> CargoResult<()> {
debug!("doing a fetch for {remote_url}");
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/git_shallow.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For test, adding one simple backend test should be good.

cargo/tests/testsuite/git_shallow.rs

Lines 40 to 43 in 3e96f1a

#[cargo_test]
fn git2_fetch_complete_dep_two_revs() {
fetch_dep_two_revs(Backend::Git2, RepoMode::Complete)
}

I would also encourage to try if we can have cover more gitoxide tests for git CLI with the same switch backend trick.

Also, I'd like to see a test like:

  1. First using gix to fetch
  2. Compiles.
  3. Add a new commit
  4. Use git cli to fetch
  5. Can still compile.

Copy link
Author

@Ygg01 Ygg01 Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I tried that with:

https://github.com/rust-lang/cargo/pull/15315/files#diff-bbd634ceb4c39b4119029427e558942d8eeefb1a24f033332bb0a36d73aca890R47-R51

It does not work.

test git_shallow::git_cli_fetch_complete_dep_two_revs ... FAILED

successes:

successes:

failures:

---- git_shallow::git_cli_fetch_complete_dep_two_revs stdout ----
running `C:\projects\cargo\target\debug\cargo.exe check -v -Zgit=shallow-deps`

thread 'git_shallow::git_cli_fetch_complete_dep_two_revs' panicked at tests\testsuite\git_shallow.rs:135:10:

test failed running `C:\projects\cargo\target\debug\cargo.exe check -v -Zgit=shallow-deps`
error: process exited with code 101 (expected 0)
--- stdout

--- stderr
warning: no edition set: defaulting to the 2015 edition while the latest is 2024
    Updating git repository `file:///C:/projects/cargo/target/tmp/cit/t0/meta-dep`
warning: spurious network error (3 tries remaining): shallow fetch is not supported by the local transport; class=Net (12)
warning: spurious network error (2 tries remaining): shallow fetch is not supported by the local transport; class=Net (12)
warning: spurious network error (1 tries remaining): shallow fetch is not supported by the local transport; class=Net (12)
error: failed to get `bar` as a dependency of package `foo v0.0.0 (C:\projects\cargo\target\tmp\cit\t0\foo)`

Caused by:
  failed to load source for dependency `bar`

Caused by:
  Unable to update file:///C:/projects/cargo/target/tmp/cit/t0/meta-dep?rev=a3e80bc72d40dc21a921742d9f808e7be2b3302f

Caused by:
  failed to clone into: C:\projects\cargo\target\tmp\cit\t0\home\.cargo\git\db\meta-dep-356a1674513a4a27-shallow

Caused by:
  network failure seems to have happened
  if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
  shallow fetch is not supported by the local transport; class=Net (12)

stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library\std\src\panicking.rs:692
   1: core::panicking::panic_fmt
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library\core\src\panicking.rs:75
   2: cargo_test_support::panic_error::pe
             at .\crates\cargo-test-support\src\lib.rs:99
   3: cargo_test_support::panic_error<anyhow::Error>
             at .\crates\cargo-test-support\src\lib.rs:91
   4: cargo_test_support::Execs::run
             at .\crates\cargo-test-support\src\lib.rs:1061
   5: testsuite::git_shallow::fetch_dep_two_revs
             at .\tests\testsuite\git_shallow.rs:130
   6: testsuite::git_shallow::git_cli_fetch_complete_dep_two_revs
             at .\tests\testsuite\git_shallow.rs:49
   7: testsuite::git_shallow::git_cli_fetch_complete_dep_two_revs::closure$0
             at .\tests\testsuite\git_shallow.rs:48
   8: core::ops::function::FnOnce::call_once<testsuite::git_shallow::git_cli_fetch_complete_dep_two_revs::closure_env$0,tuple$<> >
             at C:\Users\~~~~~~\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250
   9: core::ops::function::FnOnce::call_once
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library\core\src\ops\function.rs:250     
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Should I set the netgit-fetch-with-cli parameter somewhere as well. I'm not sure how the tests work. Is there a guide or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the https://doc.crates.io/contrib/tests/index.html and cargo-test-support doc.

You can read how other test works (not limited to git-related tests).
You can also check how switching backend works in git tests. --config override should be able to help set the net.git-fetch-with-cli option.

Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ use crate::git_gc::find_index;
enum Backend {
Git2,
Gitoxide,
GitCli,
}

impl Backend {
fn to_arg(&self) -> &'static str {
match self {
Backend::Git2 => "",
Backend::Gitoxide => "-Zgitoxide=fetch",
Backend::GitCli => "--git=git",
}
}
}
Expand Down
Loading