Skip to content

Commit 010f171

Browse files
weihangloweihanglo
authored
fix(): error context for git_fetch refspec not found (#14806)
### What does this PR try to resolve? Fixes Issue : #14621 Adds more error context for fetching a commit that doesn't exists. ### How should we test and review this PR? I've created two tests, one for fast_path and one for libgit2. r? @weihanglo
2 parents 8862183 + 43a5dee commit 010f171

File tree

2 files changed

+264
-149
lines changed

2 files changed

+264
-149
lines changed

‎src/cargo/sources/git/utils.rs

Lines changed: 180 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,9 @@ pub fn fetch(
968968

969969
let shallow = remote_kind.to_shallow_setting(repo.is_shallow(), gctx);
970970

971+
// Flag to keep track if the rev is a full commit hash
972+
let mut fast_path_rev: bool = false;
973+
971974
let oid_to_fetch = match github_fast_path(repo, remote_url, reference, gctx) {
972975
Ok(FastPathRev::UpToDate) => return Ok(()),
973976
Ok(FastPathRev::NeedsFetch(rev)) => Some(rev),
@@ -1008,6 +1011,7 @@ pub fn fetch(
10081011
if rev.starts_with("refs/") {
10091012
refspecs.push(format!("+{0}:{0}", rev));
10101013
} else if let Some(oid_to_fetch) = oid_to_fetch {
1014+
fast_path_rev = true;
10111015
refspecs.push(format!("+{0}:refs/commit/{0}", oid_to_fetch));
10121016
} else if !matches!(shallow, gix::remote::fetch::Shallow::NoChange)
10131017
&& rev.parse::<Oid>().is_ok()
@@ -1030,158 +1034,20 @@ pub fn fetch(
10301034
}
10311035
}
10321036

1033-
if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
1034-
return fetch_with_cli(repo, remote_url, &refspecs, tags, gctx);
1035-
}
1036-
1037-
if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) {
1038-
let git2_repo = repo;
1039-
let config_overrides = cargo_config_to_gitoxide_overrides(gctx)?;
1040-
let repo_reinitialized = AtomicBool::default();
1041-
let res = oxide::with_retry_and_progress(
1042-
&git2_repo.path().to_owned(),
1043-
gctx,
1044-
&|repo_path,
1045-
should_interrupt,
1046-
mut progress,
1047-
url_for_authentication: &mut dyn FnMut(&gix::bstr::BStr)| {
1048-
// The `fetch` operation here may fail spuriously due to a corrupt
1049-
// repository. It could also fail, however, for a whole slew of other
1050-
// reasons (aka network related reasons). We want Cargo to automatically
1051-
// recover from corrupt repositories, but we don't want Cargo to stomp
1052-
// over other legitimate errors.
1053-
//
1054-
// Consequently we save off the error of the `fetch` operation and if it
1055-
// looks like a "corrupt repo" error then we blow away the repo and try
1056-
// again. If it looks like any other kind of error, or if we've already
1057-
// blown away the repository, then we want to return the error as-is.
1058-
loop {
1059-
let res = oxide::open_repo(
1060-
repo_path,
1061-
config_overrides.clone(),
1062-
oxide::OpenMode::ForFetch,
1063-
)
1064-
.map_err(crate::sources::git::fetch::Error::from)
1065-
.and_then(|repo| {
1066-
debug!("initiating fetch of {refspecs:?} from {remote_url}");
1067-
let url_for_authentication = &mut *url_for_authentication;
1068-
let remote = repo
1069-
.remote_at(remote_url)?
1070-
.with_fetch_tags(if tags {
1071-
gix::remote::fetch::Tags::All
1072-
} else {
1073-
gix::remote::fetch::Tags::Included
1074-
})
1075-
.with_refspecs(
1076-
refspecs.iter().map(|s| s.as_str()),
1077-
gix::remote::Direction::Fetch,
1078-
)
1079-
.map_err(crate::sources::git::fetch::Error::Other)?;
1080-
let url = remote
1081-
.url(gix::remote::Direction::Fetch)
1082-
.expect("set at init")
1083-
.to_owned();
1084-
let connection = remote.connect(gix::remote::Direction::Fetch)?;
1085-
let mut authenticate = connection.configured_credentials(url)?;
1086-
let connection = connection.with_credentials(
1087-
move |action: gix::protocol::credentials::helper::Action| {
1088-
if let Some(url) = action.context().and_then(|gctx| {
1089-
gctx.url.as_ref().filter(|url| *url != remote_url)
1090-
}) {
1091-
url_for_authentication(url.as_ref());
1092-
}
1093-
authenticate(action)
1094-
},
1095-
);
1096-
let outcome = connection
1097-
.prepare_fetch(&mut progress, gix::remote::ref_map::Options::default())?
1098-
.with_shallow(shallow.clone().into())
1099-
.receive(&mut progress, should_interrupt)?;
1100-
Ok(outcome)
1101-
});
1102-
let err = match res {
1103-
Ok(_) => break,
1104-
Err(e) => e,
1105-
};
1106-
debug!("fetch failed: {}", err);
1107-
1108-
if !repo_reinitialized.load(Ordering::Relaxed)
1109-
// We check for errors that could occur if the configuration, refs or odb files are corrupted.
1110-
// We don't check for errors related to writing as `gitoxide` is expected to create missing leading
1111-
// folder before writing files into it, or else not even open a directory as git repository (which is
1112-
// also handled here).
1113-
&& err.is_corrupted()
1114-
|| has_shallow_lock_file(&err)
1115-
{
1116-
repo_reinitialized.store(true, Ordering::Relaxed);
1117-
debug!(
1118-
"looks like this is a corrupt repository, reinitializing \
1119-
and trying again"
1120-
);
1121-
if oxide::reinitialize(repo_path).is_ok() {
1122-
continue;
1123-
}
1124-
}
1125-
1126-
return Err(err.into());
1127-
}
1128-
Ok(())
1129-
},
1130-
);
1131-
if repo_reinitialized.load(Ordering::Relaxed) {
1132-
*git2_repo = git2::Repository::open(git2_repo.path())?;
1133-
}
1134-
res
1037+
let result = if let Some(true) = gctx.net_config()?.git_fetch_with_cli {
1038+
fetch_with_cli(repo, remote_url, &refspecs, tags, gctx)
1039+
} else if gctx.cli_unstable().gitoxide.map_or(false, |git| git.fetch) {
1040+
fetch_with_gitoxide(repo, remote_url, refspecs, tags, shallow, gctx)
11351041
} else {
1136-
debug!("doing a fetch for {remote_url}");
1137-
let git_config = git2::Config::open_default()?;
1138-
with_fetch_options(&git_config, remote_url, gctx, &mut |mut opts| {
1139-
if tags {
1140-
opts.download_tags(git2::AutotagOption::All);
1141-
}
1142-
if let gix::remote::fetch::Shallow::DepthAtRemote(depth) = shallow {
1143-
opts.depth(0i32.saturating_add_unsigned(depth.get()));
1144-
}
1145-
// The `fetch` operation here may fail spuriously due to a corrupt
1146-
// repository. It could also fail, however, for a whole slew of other
1147-
// reasons (aka network related reasons). We want Cargo to automatically
1148-
// recover from corrupt repositories, but we don't want Cargo to stomp
1149-
// over other legitimate errors.
1150-
//
1151-
// Consequently we save off the error of the `fetch` operation and if it
1152-
// looks like a "corrupt repo" error then we blow away the repo and try
1153-
// again. If it looks like any other kind of error, or if we've already
1154-
// blown away the repository, then we want to return the error as-is.
1155-
let mut repo_reinitialized = false;
1156-
loop {
1157-
debug!("initiating fetch of {refspecs:?} from {remote_url}");
1158-
let res =
1159-
repo.remote_anonymous(remote_url)?
1160-
.fetch(&refspecs, Some(&mut opts), None);
1161-
let err = match res {
1162-
Ok(()) => break,
1163-
Err(e) => e,
1164-
};
1165-
debug!("fetch failed: {}", err);
1166-
1167-
if !repo_reinitialized
1168-
&& matches!(err.class(), ErrorClass::Reference | ErrorClass::Odb)
1169-
{
1170-
repo_reinitialized = true;
1171-
debug!(
1172-
"looks like this is a corrupt repository, reinitializing \
1173-
and trying again"
1174-
);
1175-
if reinitialize(repo).is_ok() {
1176-
continue;
1177-
}
1178-
}
1042+
fetch_with_libgit2(repo, remote_url, refspecs, tags, shallow, gctx)
1043+
};
11791044

1180-
return Err(err.into());
1181-
}
1182-
Ok(())
1183-
})
1045+
if fast_path_rev {
1046+
if let Some(oid) = oid_to_fetch {
1047+
return result.with_context(|| format!("revision {} not found", oid));
1048+
}
11841049
}
1050+
result
11851051
}
11861052

11871053
/// `gitoxide` uses shallow locks to assure consistency when fetching to and to avoid races, and to write
@@ -1251,6 +1117,171 @@ fn fetch_with_cli(
12511117
Ok(())
12521118
}
12531119

1120+
fn fetch_with_gitoxide(
1121+
repo: &mut git2::Repository,
1122+
remote_url: &str,
1123+
refspecs: Vec<String>,
1124+
tags: bool,
1125+
shallow: gix::remote::fetch::Shallow,
1126+
gctx: &GlobalContext,
1127+
) -> CargoResult<()> {
1128+
let git2_repo = repo;
1129+
let config_overrides = cargo_config_to_gitoxide_overrides(gctx)?;
1130+
let repo_reinitialized = AtomicBool::default();
1131+
let res = oxide::with_retry_and_progress(
1132+
&git2_repo.path().to_owned(),
1133+
gctx,
1134+
&|repo_path,
1135+
should_interrupt,
1136+
mut progress,
1137+
url_for_authentication: &mut dyn FnMut(&gix::bstr::BStr)| {
1138+
// The `fetch` operation here may fail spuriously due to a corrupt
1139+
// repository. It could also fail, however, for a whole slew of other
1140+
// reasons (aka network related reasons). We want Cargo to automatically
1141+
// recover from corrupt repositories, but we don't want Cargo to stomp
1142+
// over other legitimate errors.
1143+
//
1144+
// Consequently we save off the error of the `fetch` operation and if it
1145+
// looks like a "corrupt repo" error then we blow away the repo and try
1146+
// again. If it looks like any other kind of error, or if we've already
1147+
// blown away the repository, then we want to return the error as-is.
1148+
loop {
1149+
let res = oxide::open_repo(
1150+
repo_path,
1151+
config_overrides.clone(),
1152+
oxide::OpenMode::ForFetch,
1153+
)
1154+
.map_err(crate::sources::git::fetch::Error::from)
1155+
.and_then(|repo| {
1156+
debug!("initiating fetch of {refspecs:?} from {remote_url}");
1157+
let url_for_authentication = &mut *url_for_authentication;
1158+
let remote = repo
1159+
.remote_at(remote_url)?
1160+
.with_fetch_tags(if tags {
1161+
gix::remote::fetch::Tags::All
1162+
} else {
1163+
gix::remote::fetch::Tags::Included
1164+
})
1165+
.with_refspecs(
1166+
refspecs.iter().map(|s| s.as_str()),
1167+
gix::remote::Direction::Fetch,
1168+
)
1169+
.map_err(crate::sources::git::fetch::Error::Other)?;
1170+
let url = remote
1171+
.url(gix::remote::Direction::Fetch)
1172+
.expect("set at init")
1173+
.to_owned();
1174+
let connection = remote.connect(gix::remote::Direction::Fetch)?;
1175+
let mut authenticate = connection.configured_credentials(url)?;
1176+
let connection = connection.with_credentials(
1177+
move |action: gix::protocol::credentials::helper::Action| {
1178+
if let Some(url) = action
1179+
.context()
1180+
.and_then(|gctx| gctx.url.as_ref().filter(|url| *url != remote_url))
1181+
{
1182+
url_for_authentication(url.as_ref());
1183+
}
1184+
authenticate(action)
1185+
},
1186+
);
1187+
let outcome = connection
1188+
.prepare_fetch(&mut progress, gix::remote::ref_map::Options::default())?
1189+
.with_shallow(shallow.clone().into())
1190+
.receive(&mut progress, should_interrupt)?;
1191+
Ok(outcome)
1192+
});
1193+
let err = match res {
1194+
Ok(_) => break,
1195+
Err(e) => e,
1196+
};
1197+
debug!("fetch failed: {}", err);
1198+
1199+
if !repo_reinitialized.load(Ordering::Relaxed)
1200+
// We check for errors that could occur if the configuration, refs or odb files are corrupted.
1201+
// We don't check for errors related to writing as `gitoxide` is expected to create missing leading
1202+
// folder before writing files into it, or else not even open a directory as git repository (which is
1203+
// also handled here).
1204+
&& err.is_corrupted()
1205+
|| has_shallow_lock_file(&err)
1206+
{
1207+
repo_reinitialized.store(true, Ordering::Relaxed);
1208+
debug!(
1209+
"looks like this is a corrupt repository, reinitializing \
1210+
and trying again"
1211+
);
1212+
if oxide::reinitialize(repo_path).is_ok() {
1213+
continue;
1214+
}
1215+
}
1216+
1217+
return Err(err.into());
1218+
}
1219+
Ok(())
1220+
},
1221+
);
1222+
if repo_reinitialized.load(Ordering::Relaxed) {
1223+
*git2_repo = git2::Repository::open(git2_repo.path())?;
1224+
}
1225+
res
1226+
}
1227+
1228+
fn fetch_with_libgit2(
1229+
repo: &mut git2::Repository,
1230+
remote_url: &str,
1231+
refspecs: Vec<String>,
1232+
tags: bool,
1233+
shallow: gix::remote::fetch::Shallow,
1234+
gctx: &GlobalContext,
1235+
) -> CargoResult<()> {
1236+
debug!("doing a fetch for {remote_url}");
1237+
let git_config = git2::Config::open_default()?;
1238+
with_fetch_options(&git_config, remote_url, gctx, &mut |mut opts| {
1239+
if tags {
1240+
opts.download_tags(git2::AutotagOption::All);
1241+
}
1242+
if let gix::remote::fetch::Shallow::DepthAtRemote(depth) = shallow {
1243+
opts.depth(0i32.saturating_add_unsigned(depth.get()));
1244+
}
1245+
// The `fetch` operation here may fail spuriously due to a corrupt
1246+
// repository. It could also fail, however, for a whole slew of other
1247+
// reasons (aka network related reasons). We want Cargo to automatically
1248+
// recover from corrupt repositories, but we don't want Cargo to stomp
1249+
// over other legitimate errors.
1250+
//
1251+
// Consequently we save off the error of the `fetch` operation and if it
1252+
// looks like a "corrupt repo" error then we blow away the repo and try
1253+
// again. If it looks like any other kind of error, or if we've already
1254+
// blown away the repository, then we want to return the error as-is.
1255+
let mut repo_reinitialized = false;
1256+
loop {
1257+
debug!("initiating fetch of {refspecs:?} from {remote_url}");
1258+
let res = repo
1259+
.remote_anonymous(remote_url)?
1260+
.fetch(&refspecs, Some(&mut opts), None);
1261+
let err = match res {
1262+
Ok(()) => break,
1263+
Err(e) => e,
1264+
};
1265+
debug!("fetch failed: {}", err);
1266+
1267+
if !repo_reinitialized && matches!(err.class(), ErrorClass::Reference | ErrorClass::Odb)
1268+
{
1269+
repo_reinitialized = true;
1270+
debug!(
1271+
"looks like this is a corrupt repository, reinitializing \
1272+
and trying again"
1273+
);
1274+
if reinitialize(repo).is_ok() {
1275+
continue;
1276+
}
1277+
}
1278+
1279+
return Err(err.into());
1280+
}
1281+
Ok(())
1282+
})
1283+
}
1284+
12541285
/// Attempts to `git gc` a repository.
12551286
///
12561287
/// Cargo has a bunch of long-lived git repositories in its global cache and

0 commit comments

Comments
 (0)