Skip to content

Commit

Permalink
Fix for Bug #1321 - Support symbolic refs in tx lines
Browse files Browse the repository at this point in the history
In git 2.45, reference transactions now support symbolic refs (see:
git/git@a8ae923)

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.
  • Loading branch information
jblebrun committed May 23, 2024
1 parent ce1aa49 commit 40226e4
Showing 3 changed files with 84 additions and 9 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
@@ -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
77 changes: 70 additions & 7 deletions git-branchless-hook/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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<MaybeZeroOid> {
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<ParsedReferenceTransactionLine> {
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<ParsedReferenceTransactionLine> = 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");
12 changes: 12 additions & 0 deletions git-branchless-lib/src/git/repo.rs
Original file line number Diff line number Diff line change
@@ -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<MaybeZeroOid> {
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]

0 comments on commit 40226e4

Please sign in to comment.