From 40226e4a99f40cdc86f9a35b5036fce8fd0ca78a Mon Sep 17 00:00:00 2001 From: Jason LeBrun Date: Thu, 23 May 2024 17:55:44 +0000 Subject: [PATCH] Fix for Bug #1321 - Support symbolic refs in tx lines In git 2.45, reference transactions now support symbolic refs (see: https://github.com/git/git/commit/a8ae923f85da6434c3faf9c39719d6d5e5c77e65) To support this, we add a transaction reference resolver that's capable of looking up the Oid for a named reference. It requires a repository object to do this successfully. To support the functionality, a new `refname_to_id` function is added to the Repo object that calls the similarly named method on the inner repository to get the `git2::Oid` for a given reference name. --- .github/workflows/linux.yml | 4 +- git-branchless-hook/src/lib.rs | 77 +++++++++++++++++++++++++++--- git-branchless-lib/src/git/repo.rs | 12 +++++ 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 6be6d46cf..7158681d4 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -18,7 +18,7 @@ jobs: matrix: # Use a tag from https://github.com/git/git/tags # Make sure to update `git-version` in the `run-tests` step as well. - git-version: ["v2.24.3", "v2.29.2", "v2.33.1", "v2.37.3"] + git-version: ["v2.24.3", "v2.29.2", "v2.33.1", "v2.37.3", "v2.45.1"] steps: - uses: actions/checkout@v4 @@ -58,7 +58,7 @@ jobs: strategy: matrix: - git-version: ["v2.24.3", "v2.29.2", "v2.33.1", "v2.37.3"] + git-version: ["v2.24.3", "v2.29.2", "v2.33.1", "v2.37.3", "v2.45.1"] steps: - uses: actions/checkout@v4 diff --git a/git-branchless-hook/src/lib.rs b/git-branchless-hook/src/lib.rs index cf3cbfff1..196a18b47 100644 --- a/git-branchless-hook/src/lib.rs +++ b/git-branchless-hook/src/lib.rs @@ -42,6 +42,44 @@ pub use lib::core::rewrite::rewrite_hooks::{ hook_skip_upstream_applied_commit, }; +/// A component that will find the hash for a given reference transaction OID. +/// Reference transaction OIDs may be one of: +/// +/// * A 40-hex-digit hash string +/// * A symbolic ref, in which case, it will start with "ref:" +/// +/// See: "refs: support symrefs in 'reference-transaction' hook" +/// https://github.com/git/git/commit/a8ae923f85da6434c3faf9c39719d6d5e5c77e65 +#[derive(Debug)] +struct TransactionReferenceFinder<'a> { + repo: &'a Repo, +} + +impl<'a> TransactionReferenceFinder<'a> { + fn new(repo: &'a Repo) -> Self { + Self { repo } + } + + /// Attempt to resolve the provided string as a transaction reference OID. + /// + /// If name starts with `ref:`, and the implementation has a reference lookup + /// mechanism, then the OID for the provided reference name will be + /// returned, or a failure, if no target was found. + /// + /// Otherwise, an attempt is made to parse `name` as a 40-hex-digit hash, + /// returning the results of that parse as a [MaybeZeroOid]. + #[instrument] + fn oid_for_transaction_reference(&self, name: &str) -> eyre::Result { + match name.strip_prefix("ref:") { + Some(refname) => Ok(self + .repo + .reference_name_to_oid(&ReferenceName::from(refname))?), + + None => Ok(name.parse()?), + } + } +} + /// Handle Git's `post-checkout` hook. /// /// See the man-page for `githooks(5)`. @@ -197,6 +235,8 @@ mod reference_transaction { use lib::git::{MaybeZeroOid, ReferenceName, Repo}; + use crate::TransactionReferenceFinder; + #[instrument] fn parse_packed_refs_line(line: &str) -> Option<(ReferenceName, MaybeZeroOid)> { if line.is_empty() { @@ -289,13 +329,14 @@ mod reference_transaction { #[instrument] pub fn parse_reference_transaction_line( line: &str, + resolver: &TransactionReferenceFinder, ) -> eyre::Result { let fields = line.split(' ').collect_vec(); match fields.as_slice() { [old_value, new_value, ref_name] => Ok(ParsedReferenceTransactionLine { ref_name: ReferenceName::from(*ref_name), - old_oid: MaybeZeroOid::from_str(old_value)?, - new_oid: MaybeZeroOid::from_str(new_value)?, + old_oid: resolver.oid_for_transaction_reference(old_value)?, + new_oid: resolver.oid_for_transaction_reference(new_value)?, }), _ => { eyre::bail!( @@ -309,11 +350,21 @@ mod reference_transaction { #[cfg(test)] #[test] fn test_parse_reference_transaction_line() -> eyre::Result<()> { - use lib::core::eventlog::should_ignore_ref_updates; + use lib::{core::eventlog::should_ignore_ref_updates, testing::make_git}; + + use crate::TransactionReferenceFinder; + + let git = make_git()?; + git.init_repo()?; + let master_head_oid = git.commit_file("README", 1)?; + + // Attempt to use this repository for testing. Maybe not a good idea... + let repo = Repo::from_dir(git.repo_path.as_path())?; + let resolver = TransactionReferenceFinder::new(&repo); let line = "123abc 456def refs/heads/mybranch"; assert_eq!( - parse_reference_transaction_line(line)?, + parse_reference_transaction_line(line, &resolver)?, ParsedReferenceTransactionLine { old_oid: "123abc".parse()?, new_oid: { @@ -324,9 +375,19 @@ mod reference_transaction { } ); + let line = "000000 ref:refs/heads/master HEAD"; + assert_eq!( + parse_reference_transaction_line(line, &resolver)?, + ParsedReferenceTransactionLine { + old_oid: "000000".parse()?, + new_oid: master_head_oid.into(), + ref_name: ReferenceName::from("HEAD"), + } + ); + { let line = "123abc 456def ORIG_HEAD"; - let parsed_line = parse_reference_transaction_line(line)?; + let parsed_line = parse_reference_transaction_line(line, &resolver)?; assert_eq!( parsed_line, ParsedReferenceTransactionLine { @@ -339,7 +400,7 @@ mod reference_transaction { } let line = "there are not three fields here"; - assert!(parse_reference_transaction_line(line).is_err()); + assert!(parse_reference_transaction_line(line, &resolver).is_err()); Ok(()) } @@ -432,6 +493,8 @@ fn hook_reference_transaction(effects: &Effects, transaction_state: &str) -> eyr let packed_references = read_packed_refs_file(&repo)?; + let rf = TransactionReferenceFinder::new(&repo); + let parsed_lines: Vec = stdin() .lock() .split(b'\n') @@ -447,7 +510,7 @@ fn hook_reference_transaction(effects: &Effects, transaction_state: &str) -> eyr return None; } }; - match parse_reference_transaction_line(line) { + match parse_reference_transaction_line(line, &rf) { Ok(line) => Some(line), Err(err) => { error!(?err, ?line, "Could not parse reference-transaction-line"); diff --git a/git-branchless-lib/src/git/repo.rs b/git-branchless-lib/src/git/repo.rs index e6c55dbba..1de1e2e71 100644 --- a/git-branchless-lib/src/git/repo.rs +++ b/git-branchless-lib/src/git/repo.rs @@ -698,6 +698,18 @@ impl Repo { } } + /// Get the OID for a given [ReferenceName] if it exists. + #[instrument] + pub fn reference_name_to_oid(&self, name: &ReferenceName) -> Result { + match self.inner.refname_to_id(name.as_str()) { + Ok(git2_oid) => Ok(MaybeZeroOid::from(git2_oid)), + Err(source) => Err(Error::FindReference { + source, + name: name.clone(), + }), + } + } + /// Set the `HEAD` reference directly to the provided `oid`. Does not touch /// the working copy. #[instrument]