From d983d15ed7078bfe76a4158d9cbb1f19d5585d96 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 00:30:09 +0000 Subject: [PATCH 1/3] chore(deps): update git2 requirement in the cargo-dependencies group Updates the requirements on [git2](https://github.com/rust-lang/git2-rs) to permit the latest version. Updates `git2` to 0.21.0 - [Changelog](https://github.com/rust-lang/git2-rs/blob/main/CHANGELOG.md) - [Commits](https://github.com/rust-lang/git2-rs/commits/git2-0.21.0) --- updated-dependencies: - dependency-name: git2 dependency-version: 0.21.0 dependency-type: direct:production dependency-group: cargo-dependencies ... Signed-off-by: dependabot[bot] --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a5ed13e..662d4d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,7 +51,7 @@ glob = "0.3" dirs = "6.0.0" # git2 library for in-process git operations (Tier 1 credential relay) -git2 = "0.20.4" +git2 = "0.21.0" # Compression for tar.gz archives flate2 = "1.0" From d90d0682e36b315a87b412b4da610eeb3764e298 Mon Sep 17 00:00:00 2001 From: Matej Gomboc Date: Wed, 27 May 2026 07:12:13 +0200 Subject: [PATCH 2/3] fix(deps): adapt code to git2 0.21 breaking API changes git2 0.21 ships two breaking changes that broke the build: 1. `default` features became empty (0.20 enabled "ssh" + "https"). Request them explicitly so `Cred::credential_helper` (now gated behind the new transitive "cred" feature) resolves again. 2. `TreeEntry::name()` and `Commit::message()` now return `Result<&str, Error>` instead of `Option<&str>`. Switch the three tree-walk `let Some(name)` callbacks to `let Ok(name)` (preserving the skip-on-non-UTF-8 behaviour) and one test assertion to `Ok(..)`. Also replace the deprecated `Oid::zero()` with `Oid::ZERO_SHA1` in three submodule tests. Updated CHANGELOG. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 15 +++++++++++++++ Cargo.toml | 7 +++++-- src/git2_ops/push.rs | 3 ++- src/git2_ops/submodule.rs | 10 ++++++---- src/streaming/tar.rs | 8 ++++++-- 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bd6784..f25ff5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **`git2` requirement bumped 0.20.4 → 0.21.0** (cargo-dependencies group). + 0.21 ships two breaking changes that touched this crate: + 1. **`default` features are now empty** (0.20 enabled `ssh` + `https`). + `Cargo.toml` now requests `features = ["https", "ssh"]` explicitly — + the exact set 0.20.4 enabled by default — which also pulls in the new + `cred` feature that gates `Cred::credential_helper`. No change to the + credential or transport behaviour. + 2. **`TreeEntry::name()` and `Commit::message()` now return + `Result<&str, Error>`** (UTF-8 validation) instead of `Option<&str>`. + The three `let Some(name) = entry.name()` walk callbacks in + `streaming/tar.rs` and `git2_ops/submodule.rs` became + `let Ok(name) = …`, preserving the "skip non-UTF-8 names" behaviour; + one `push.rs` test assertion moved from `Some(..)` to `Ok(..)`. + Also replaced the now-deprecated `Oid::zero()` with the `Oid::ZERO_SHA1` + constant in three `submodule.rs` tests. No production behaviour change. - **`download_with_retry` and `download_with_progress` collapsed into a single `download_chunked` helper.** The two had drifted on observability (the progress variant was missing `url` in its `warn!` calls) and the diff --git a/Cargo.toml b/Cargo.toml index 662d4d3..44af6c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,8 +50,11 @@ glob = "0.3" # Platform-specific directories dirs = "6.0.0" -# git2 library for in-process git operations (Tier 1 credential relay) -git2 = "0.21.0" +# git2 library for in-process git operations (Tier 1 credential relay). +# 0.21 changed `default` features to empty (0.20 enabled "ssh" + "https"), +# so they must now be requested explicitly. "https" + "ssh" transitively +# enable the new "cred" feature that gates `Cred::credential_helper`. +git2 = { version = "0.21.0", features = ["https", "ssh"] } # Compression for tar.gz archives flate2 = "1.0" diff --git a/src/git2_ops/push.rs b/src/git2_ops/push.rs index ec53579..21061ef 100644 --- a/src/git2_ops/push.rs +++ b/src/git2_ops/push.rs @@ -534,7 +534,8 @@ mod tests { .find_reference("refs/heads/main") .expect("refs/heads/main must exist after unbundle"); let commit = reference.peel_to_commit().unwrap(); - assert_eq!(commit.message(), Some("seed commit")); + // git2 0.21: `Commit::message()` returns `Result` (UTF-8 check). + assert_eq!(commit.message(), Ok("seed commit")); } #[test] diff --git a/src/git2_ops/submodule.rs b/src/git2_ops/submodule.rs index 4e39e7c..e5c8968 100644 --- a/src/git2_ops/submodule.rs +++ b/src/git2_ops/submodule.rs @@ -260,7 +260,9 @@ pub fn find_submodule_entries( tree.walk(TreeWalkMode::PreOrder, |dir, entry| { // Check if this is a submodule entry (mode 160000) if entry.filemode() == SUBMODULE_MODE && entry.kind() == Some(ObjectType::Commit) { - let Some(name) = entry.name() else { + // git2 0.21: `TreeEntry::name()` returns `Result` (UTF-8 check); + // `Err` means a non-UTF-8 name, which we skip as before. + let Ok(name) = entry.name() else { return TreeWalkResult::Ok; }; @@ -917,7 +919,7 @@ mod tests { fn submodule_entry_struct_fields() { let entry = SubmoduleEntry { path: "vendor/x".to_string(), - commit: Oid::zero(), + commit: Oid::ZERO_SHA1, url: "https://example.com/x.git".to_string(), }; assert_eq!(entry.path, "vendor/x"); @@ -951,7 +953,7 @@ mod tests { fn fetch_submodule_with_invalid_url_fails() { let entry = SubmoduleEntry { path: "vendor/x".to_string(), - commit: Oid::zero(), + commit: Oid::ZERO_SHA1, url: "not-a-valid-url".to_string(), }; let result = fetch_submodule(&entry, None); @@ -962,7 +964,7 @@ mod tests { fn fetch_submodule_with_file_url_rejected() { let entry = SubmoduleEntry { path: "vendor/x".to_string(), - commit: Oid::zero(), + commit: Oid::ZERO_SHA1, url: "file:///etc/passwd".to_string(), }; assert!(fetch_submodule(&entry, None).is_err()); diff --git a/src/streaming/tar.rs b/src/streaming/tar.rs index 6c0e75a..d27a37f 100644 --- a/src/streaming/tar.rs +++ b/src/streaming/tar.rs @@ -342,7 +342,9 @@ pub fn create_tar_from_tree_with_options( // Walk the tree and add each blob to the tar tree.walk(TreeWalkMode::PreOrder, |dir, entry| { - let Some(name) = entry.name() else { + // git2 0.21: `TreeEntry::name()` returns `Result` (UTF-8 check); + // `Err` means a non-UTF-8 name, which we skip as before. + let Ok(name) = entry.name() else { return TreeWalkResult::Skip; }; @@ -623,7 +625,9 @@ fn write_submodules_to_tar( let mut submodule_files = 0usize; let walk_result = submodule_tree.walk(TreeWalkMode::PreOrder, |dir, entry| { - let Some(name) = entry.name() else { + // git2 0.21: `TreeEntry::name()` returns `Result` (UTF-8 check); + // `Err` means a non-UTF-8 name, which we skip as before. + let Ok(name) = entry.name() else { return TreeWalkResult::Skip; }; From 7fb97b22345b40425727d61ba476f8ee4a40f0b0 Mon Sep 17 00:00:00 2001 From: Matej Gomboc Date: Wed, 27 May 2026 07:40:20 +0200 Subject: [PATCH 3/3] test(submodule): cover git2 0.21 migrated walk callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The git2 0.21 `Some`→`Ok` migration touched two submodule walk callbacks (`find_submodule_entries` and `write_submodules_to_tar`) whose `let Ok(name) = entry.name()` lines were only exercised by the Python integration tests. Those don't run in the PR coverage job (ci_pr.yml is unit-tests-only), so Codecov patch coverage flagged them as uncovered changed lines (71.43% < 80% gate). Add three unit tests that close the gap without any network: - find_submodule_entries_detects_gitlink_in_tree / _gitlink_missing_ from_gitmodules_is_skipped: build a tree containing a real gitlink (mode 160000) entry and assert the entry is/ isn't returned. - write_submodules_to_tar_writes_submodule_files: drive the private write_submodules_to_tar against a locally-created bare repo via a new test-only FetchResult::from_parts_for_test constructor. Both previously-missed lines are now unit-covered; full suite is 597 unit tests. Updated CHANGELOG. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 9 +++++ src/git2_ops/clone.rs | 22 +++++++++++ src/git2_ops/submodule.rs | 79 ++++++++++++++++++++++++++++++++++++++ src/streaming/tar.rs | 80 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 190 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f25ff5c..2f0359e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,6 +146,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 one `push.rs` test assertion moved from `Some(..)` to `Ok(..)`. Also replaced the now-deprecated `Oid::zero()` with the `Oid::ZERO_SHA1` constant in three `submodule.rs` tests. No production behaviour change. + Three new regression tests close the unit-coverage gap on the two + migrated submodule walk callbacks (previously only exercised by the + Python integration tests, which don't run in the PR coverage job): + `find_submodule_entries_detects_gitlink_in_tree` and + `find_submodule_entries_gitlink_missing_from_gitmodules_is_skipped` + build a tree with a real gitlink (mode 160000) entry, and + `write_submodules_to_tar_writes_submodule_files` drives + `write_submodules_to_tar` against a locally-created bare repo via a new + test-only `FetchResult::from_parts_for_test` constructor (no network). - **`download_with_retry` and `download_with_progress` collapsed into a single `download_chunked` helper.** The two had drifted on observability (the progress variant was missing `url` in its `warn!` calls) and the diff --git a/src/git2_ops/clone.rs b/src/git2_ops/clone.rs index 13688e5..747b540 100644 --- a/src/git2_ops/clone.rs +++ b/src/git2_ops/clone.rs @@ -40,6 +40,28 @@ pub struct FetchResult { _temp_dir: TempDir, } +#[cfg(test)] +impl FetchResult { + /// Construct a `FetchResult` from already-prepared parts. Test-only: lets + /// unit tests in other modules (e.g. `streaming::tar`) build a fetched-repo + /// handle around a locally-created bare repo without a real network fetch. + /// The private `_temp_dir` field keeps the temp directory alive for the + /// lifetime of the returned value, exactly as the production path does. + pub(crate) fn from_parts_for_test( + repo: Repository, + head_commit: Oid, + branch: String, + temp_dir: TempDir, + ) -> Self { + Self { + repo, + head_commit, + branch, + _temp_dir: temp_dir, + } + } +} + /// Options for fetch operations. #[derive(Debug, Clone, Default)] pub struct FetchOptions2 { diff --git a/src/git2_ops/submodule.rs b/src/git2_ops/submodule.rs index e5c8968..241b053 100644 --- a/src/git2_ops/submodule.rs +++ b/src/git2_ops/submodule.rs @@ -905,6 +905,85 @@ mod tests { assert!(entries.is_empty()); } + #[test] + fn find_submodule_entries_detects_gitlink_in_tree() { + // A tree containing a real gitlink (mode 160000) exercises the + // `let Ok(name) = entry.name()` walk-callback branch that the + // no-submodule test above never reaches. + let temp = tempfile::TempDir::new().unwrap(); + let repo = Repository::init_bare(temp.path()).unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + + // A commit to serve as the gitlink target oid (lives in this same repo + // for the test; `find_submodule_entries` only reads the tree entry's + // id/name, never peels the target). + let blob = repo.blob(b"sub\n").unwrap(); + let mut sub_tb = repo.treebuilder(None).unwrap(); + sub_tb.insert("f.txt", blob, 0o100_644).unwrap(); + let sub_tree = repo.find_tree(sub_tb.write().unwrap()).unwrap(); + let gitlink_target = repo + .commit(None, &sig, &sig, "submodule commit", &sub_tree, &[]) + .unwrap(); + + // Parent commit whose tree has a gitlink at the root path "sub". + let mut root_tb = repo.treebuilder(None).unwrap(); + root_tb + .insert("sub", gitlink_target, SUBMODULE_MODE) + .unwrap(); + let root_tree = repo.find_tree(root_tb.write().unwrap()).unwrap(); + let commit_oid = repo + .commit(None, &sig, &sig, "root commit", &root_tree, &[]) + .unwrap(); + + let mut gitmodules = HashMap::new(); + gitmodules.insert( + "sub".to_string(), + SubmoduleInfo { + name: "sub".to_string(), + path: "sub".to_string(), + url: "https://example.com/sub.git".to_string(), + branch: None, + }, + ); + + let entries = find_submodule_entries(&repo, commit_oid, &gitmodules).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].path, "sub"); + assert_eq!(entries[0].commit, gitlink_target); + assert_eq!(entries[0].url, "https://example.com/sub.git"); + } + + #[test] + fn find_submodule_entries_gitlink_missing_from_gitmodules_is_skipped() { + // Same gitlink tree, but with an empty .gitmodules map: the entry is + // found in the tree (hitting the same walk branch) yet produces no + // `SubmoduleEntry` because there's no URL to fetch from. + let temp = tempfile::TempDir::new().unwrap(); + let repo = Repository::init_bare(temp.path()).unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + + let blob = repo.blob(b"sub\n").unwrap(); + let mut sub_tb = repo.treebuilder(None).unwrap(); + sub_tb.insert("f.txt", blob, 0o100_644).unwrap(); + let sub_tree = repo.find_tree(sub_tb.write().unwrap()).unwrap(); + let gitlink_target = repo + .commit(None, &sig, &sig, "submodule commit", &sub_tree, &[]) + .unwrap(); + + let mut root_tb = repo.treebuilder(None).unwrap(); + root_tb + .insert("sub", gitlink_target, SUBMODULE_MODE) + .unwrap(); + let root_tree = repo.find_tree(root_tb.write().unwrap()).unwrap(); + let commit_oid = repo + .commit(None, &sig, &sig, "root commit", &root_tree, &[]) + .unwrap(); + + let gitmodules = HashMap::new(); + let entries = find_submodule_entries(&repo, commit_oid, &gitmodules).unwrap(); + assert!(entries.is_empty()); + } + #[test] fn find_submodule_entries_invalid_commit_returns_error() { let temp = tempfile::TempDir::new().unwrap(); diff --git a/src/streaming/tar.rs b/src/streaming/tar.rs index d27a37f..7707c75 100644 --- a/src/streaming/tar.rs +++ b/src/streaming/tar.rs @@ -758,6 +758,86 @@ mod tests { assert_eq!(encoded, "aGVsbG8gd29ybGQ="); } + #[test] + fn write_submodules_to_tar_writes_submodule_files() { + // Exercises `write_submodules_to_tar` (including its + // `let Ok(name) = entry.name()` walk branch) without a network fetch, + // by building a `FetchedSubmodule` around a locally-created bare repo. + use crate::git2_ops::clone::FetchResult; + use crate::git2_ops::submodule::SubmoduleEntry; + + let temp = tempfile::TempDir::new().unwrap(); + let commit_oid = { + let repo = Repository::init_bare(temp.path()).unwrap(); + let blob = repo.blob(b"submodule file contents\n").unwrap(); + let mut tb = repo.treebuilder(None).unwrap(); + tb.insert("README.md", blob, 0o100_644).unwrap(); + let tree = repo.find_tree(tb.write().unwrap()).unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + repo.commit(None, &sig, &sig, "submodule commit", &tree, &[]) + .unwrap() + }; + let repo = Repository::open_bare(temp.path()).unwrap(); + + let fetched = FetchedSubmodule { + entry: SubmoduleEntry { + path: "vendor/sub".to_string(), + commit: commit_oid, + url: "https://example.com/sub.git".to_string(), + }, + fetch_result: FetchResult::from_parts_for_test( + repo, + commit_oid, + "main".to_string(), + temp, + ), + children: Vec::new(), + }; + + let mut archive = Vec::new(); + { + let encoder = GzEncoder::new(&mut archive, Compression::fast()); + let mut tar_builder = tar::Builder::new(encoder); + + let mut file_count = 0usize; + let mut uncompressed_size = 0u64; + let mut skipped_by_filter = 0usize; + let mut skipped_binary = 0usize; + let mut skipped_too_large = 0usize; + let mut skipped_path_too_long = 0usize; + let mut submodules_included = 0usize; + let mut submodules_failed = 0usize; + + write_submodules_to_tar( + std::slice::from_ref(&fetched), + "", + None, + false, + None, + &mut tar_builder, + &mut file_count, + &mut uncompressed_size, + &mut skipped_by_filter, + &mut skipped_binary, + &mut skipped_too_large, + &mut skipped_path_too_long, + &mut submodules_included, + &mut submodules_failed, + None, + ); + + tar_builder.finish().unwrap(); + + assert_eq!(file_count, 1, "the submodule's single file should be added"); + assert_eq!(submodules_included, 1); + assert_eq!(submodules_failed, 0); + assert!(uncompressed_size > 0); + } + + // The archive should contain the submodule file under its path prefix. + assert!(!archive.is_empty()); + } + #[test] fn sparse_filter_empty_matches_all() { let filter = SparseFilter::new(&[]);