Skip to content

Commit c6dd6ae

Browse files
authored
fix(git): make checkout names independent of git config (#17289)
### What does this PR try to resolve? Previously the length followed user's `core.abbrev`. Cargo cache layout should be stable and independent from user settings, hence this fix. Downside of this is that if people already had a system/user-wide git config `core.abbrev` set to number other than 7, it may lead to re-downloads. ### How to test and review this PR? Commit by commit. This was found during `-Ztrim-paths` implementation, which we would like to embed revision in the remapped object files, and we want it to be stable. (but probably we don't need to embed)
2 parents 8ea62e9 + dd6a53d commit c6dd6ae

2 files changed

Lines changed: 63 additions & 16 deletions

File tree

src/sources/git/utils.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,6 @@ use std::time::{Duration, Instant};
3434
/// checkout is ready to go. See [`GitCheckout::reset`] for why we need this.
3535
const CHECKOUT_READY_LOCK: &str = ".cargo-ok";
3636

37-
/// A short abbreviated OID.
38-
///
39-
/// Exists for avoiding extra allocations in [`GitDatabase::to_short_id`].
40-
pub struct GitShortID(git2::Buf);
41-
42-
impl GitShortID {
43-
/// Views the short ID as a `str`.
44-
pub fn as_str(&self) -> &str {
45-
self.0.as_str().unwrap()
46-
}
47-
}
48-
4937
/// A remote repository. It gets cloned into a local [`GitDatabase`].
5038
#[derive(PartialEq, Clone, Debug)]
5139
pub struct GitRemote {
@@ -205,10 +193,26 @@ impl GitDatabase {
205193
Ok(checkout)
206194
}
207195

208-
/// Get a short OID for a `revision`, usually 7 chars or more if ambiguous.
209-
pub fn to_short_id(&self, revision: git2::Oid) -> CargoResult<GitShortID> {
210-
let obj = self.repo.find_object(revision, None)?;
211-
Ok(GitShortID(obj.short_id()?))
196+
/// Get a short OID for a `revision`, 7 chars or more if ambiguous.
197+
///
198+
/// Like [`git2::Object::short_id`]
199+
/// but ignores the user's `core.abbrev` git config.
200+
pub fn to_short_id(&self, rev: git2::Oid) -> CargoResult<String> {
201+
const MIN_ABBREV_LEN: usize = 7; // this is git/libgit2's default
202+
let odb = self.repo.odb()?;
203+
let mut len = MIN_ABBREV_LEN;
204+
let mut hex = rev.to_string();
205+
// quasi- re-implementation of
206+
// https://github.com/libgit2/libgit2/blob/26055f5af74ab/src/libgit2/object.c#L523-L573
207+
while len < hex.len() {
208+
match odb.exists_prefix(rev, len) {
209+
Ok(_) => break,
210+
Err(err) if err.code() == git2::ErrorCode::Ambiguous => len += 1,
211+
Err(err) => return Err(err.into()),
212+
}
213+
}
214+
hex.truncate(len);
215+
Ok(hex)
212216
}
213217

214218
/// Checks if the database contains the object of this `oid`..

tests/testsuite/git.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3163,6 +3163,49 @@ fn templatedir_doesnt_cause_problems() {
31633163
p.cargo("check").run();
31643164
}
31653165

3166+
#[cargo_test]
3167+
fn checkout_name_with_core_abbrev_config() {
3168+
let git_project = git::new("dep1", |project| {
3169+
project
3170+
.file("Cargo.toml", &basic_manifest("dep1", "0.5.0"))
3171+
.file("src/lib.rs", "")
3172+
});
3173+
3174+
let p = project()
3175+
.file(
3176+
"Cargo.toml",
3177+
&format!(
3178+
r#"
3179+
[package]
3180+
name = "foo"
3181+
version = "0.1.0"
3182+
edition = "2015"
3183+
3184+
[dependencies]
3185+
dep1 = {{ git = "{}" }}
3186+
"#,
3187+
git_project.url()
3188+
),
3189+
)
3190+
.file("src/lib.rs", "")
3191+
.build();
3192+
3193+
fs::write(paths::home().join(".gitconfig"), "[core]\n\tabbrev = 4\n").unwrap();
3194+
3195+
p.cargo("fetch").run();
3196+
3197+
let mut co_paths = t!(glob::glob(
3198+
paths::home()
3199+
.join(".cargo/git/checkouts/dep1-*/*")
3200+
.to_str()
3201+
.unwrap()
3202+
));
3203+
let co_path = co_paths.next().unwrap().unwrap();
3204+
let rev = co_path.file_name().unwrap().to_str().unwrap();
3205+
// The checkout directory name ignores the user's `core.abbrev`.
3206+
assert_eq!(rev.len(), 7);
3207+
}
3208+
31663209
#[cargo_test(requires = "git")]
31673210
fn git_with_cli_force() {
31683211
// Supports a force-pushed repo.

0 commit comments

Comments
 (0)