Ensure that init always fetches base from upstream

Author: nightkr

rust/patchable/src/main.rs

@@ -174,12 +174,6 @@ pub enum Error {
 
     #[snafu(display("failed to open product repository at {path:?}"))]
     OpenProductRepo { source: git2::Error, path: PathBuf },
-    #[snafu(display("failed to find base commit {commit:?} in repository {repo}"))]
-    FindBaseCommit {
-        source: git2::Error,
-        repo: error::RepoPath,
-        commit: String,
-    },
     #[snafu(display("failed to find head commit in repository {repo}"))]
     FindHeadCommit {
         source: git2::Error,
@@ -245,7 +239,7 @@ fn main() -> Result<()> {
             .in_scope(|| repo::ensure_bare_repo(&product_repo_root))
             .context(OpenProductRepoForCheckoutSnafu)?;
 
-            let base_commit = repo::resolve_commitish_or_fetch(
+            let base_commit = repo::resolve_and_fetch_commitish(
                 &product_repo,
                 &config.base.to_string(),
                 &config.upstream,
@@ -341,7 +335,7 @@ fn main() -> Result<()> {
             // --base can be a reference, but patchable.toml should always have a resolved commit id,
             // so that it cannot be changed under our feet (without us knowing so, anyway...).
             tracing::info!(?base, "resolving base commit-ish");
-            let base_commit = repo::resolve_commitish_or_fetch(&product_repo, &base, &upstream)
+            let base_commit = repo::resolve_and_fetch_commitish(&product_repo, &base, &upstream)
                 .context(FetchBaseCommitSnafu)?;
             tracing::info!(?base, base.commit = ?base_commit, "resolved base commit");
 

rust/patchable/src/repo.rs

@@ -105,24 +105,29 @@ pub fn ensure_bare_repo(path: &Path) -> Result<Repository> {
     }
 }
 
-/// Try to resolve `commitish` locally. If it doesn't exist, try to fetch it from `upstream_url`.
+/// Try to resolve and fetch `commitish` from `upstream_url`.
+///
+/// As an optimization, it can skip fetching if `commitish` is a literal commit ID that exists locally.
 ///
 /// Returns the resolved commit ID.
 #[tracing::instrument(skip(repo))]
-pub fn resolve_commitish_or_fetch(
+pub fn resolve_and_fetch_commitish(
     repo: &Repository,
     commitish: &str,
     upstream_url: &str,
 ) -> Result<Oid> {
-    let commit = match repo.revparse_single(commitish) {
+    let oid = Oid::from_str(commitish);
+    let commitish_is_oid = oid.is_ok();
+    let local_commit = oid.and_then(|oid| repo.find_commit(oid));
+    let commit = match local_commit {
         Ok(commit_obj) => {
-            tracing::info!("base commit exists, reusing");
+            tracing::info!("literal commit exists locally, reusing");
             Ok(commit_obj)
         }
-        Err(err) if err.code() == git2::ErrorCode::NotFound => {
+        Err(err) if !commitish_is_oid || err.code() == git2::ErrorCode::NotFound => {
             tracing::info!(
                 error = &err as &dyn std::error::Error,
-                "base commit not found, fetching from upstream"
+                "base commit not found locally, fetching from upstream"
             );
             repo.remote_anonymous(upstream_url)
                 .context(CreateRemoteSnafu {
@@ -147,17 +152,15 @@ pub fn resolve_commitish_or_fetch(
             tracing::info!("fetched base commit");
             // FETCH_HEAD is written by Remote::fetch to be the last reference fetched
             repo.revparse_single("FETCH_HEAD")
+                .and_then(|obj| obj.peel_to_commit())
         }
         Err(err) => Err(err),
     }
     .context(FindCommitSnafu {
         repo,
         commit: commitish,
     })?;
-    Ok(commit
-        .peel_to_commit()
-        .context(FindCommitSnafu { repo, commit })?
-        .id())
+    Ok(commit.id())
 }
 
 /// Ensure that the worktree at `worktree_root` exists and is checked out at `branch`.