`sync` subcommand with `--pull` does not support negative refspecs

[ryan-ph]

Description of the bug

Git v2.29.0 supports negative refspecs to allow filtering which refspecs to ignore.

It seems like git branchless sync --pull does not currently respect this and will cause panics if a negative refspec is specified in the git config.

Example git config specifying a negative refspec for any branches with an underscore prefix

[remote "origin"]
    fetch = ^refs/heads/_*

[2024/11/26] edit: seems like this also occurs with the hide subcommand if the target is a branch name that hasn't yet been pushed to the remote

Expected behavior

When running git branchless sync --pull, I expect all refs to be fetched from the remote following all git refspec rules (i.e. all refs which match postive refspecs and do not match any negative refspecs), and all local refs to be updated.

Actual behavior

A panic:

$ git sync --pull
branchless: running command: git fetch --all
branchless: processing 1 update: remote branch origin/master
From github.com:**********
   1cbda20c5530..7aebefa5462d  master        -> origin/master
branchless: processing 1 update: remote branch origin/fix-inventory
 * [new branch]                fix-inventory -> origin/fix-inventory
The application panicked (crashed).
Message:  A fatal error occurred:
   0: could not find upstream branch for branch with name 'master': '^refs/heads/_*' is not a valid refspec.; class=Invalid (3)
   1: '^refs/heads/_*' is not a valid refspec.; class=Invalid (3)

Version of rustc

No response

Automated bug report

No response

Version of git-branchless

git-branchless-opts 0.10.0

Version of git

git version 2.44.0

[ryan-ph]

Seems like this is an upstream issue: libgit2/libgit2#6741

[arxanas]

Thanks for reporting. It's unfortunate because, as far as I know, we don't actually need or care about the fetch refspec for the main branch. I think it's hitting this code path and asking for the upstream branch:

git-branchless/git-branchless-lib/src/git/reference.rs

Lines 277 to 296 in 2be0fd0

	/// If this branch tracks a remote ("upstream") branch, return that branch.
	#[instrument]
	pub fn get_upstream_branch(&self) -> Result<Option<Branch<'repo>>> {
	match self.inner.upstream() {
	Ok(upstream) => Ok(Some(Branch {
	repo: self.repo,
	inner: upstream,
	})),
	Err(err) if err.code() == git2::ErrorCode::NotFound => Ok(None),
	Err(err) => {
	let branch_name = self.inner.name_bytes().map_err(|_err| Error::DecodeUtf8 {
	item: "branch name",
	})?;
	Err(Error::FindUpstreamBranch {
	source: err,
	name: String::from_utf8_lossy(branch_name).into_owned(),
	})
	}
	}
	}

And then I guess we're making a Branch object, which probably involves parsing all the information, not just the upstream branch name.

---

I only glanced through it, but it's possible that we only need the OID of upstream branch, and not the full branch information. Namely, we might be able to call this function instead:

git-branchless/git-branchless-lib/src/git/reference.rs

Lines 298 to 308 in 2be0fd0

	/// If this branch tracks a remote ("upstream") branch, return the OID of the commit which that
	/// branch points to.
	#[instrument]
	pub fn get_upstream_branch_target(&self) -> eyre::Result<Option<NonZeroOid>> {
	let upstream_branch = match self.get_upstream_branch()? {
	Some(upstream_branch) => upstream_branch,
	None => return Ok(None),
	};
	let target_oid = upstream_branch.get_oid()?;
	Ok(target_oid)
	}

where we're doing this in sync:

git-branchless/git-branchless/src/commands/sync.rs

Lines 170 to 183 in 2be0fd0

	let upstream_main_branch = match local_main_branch.get_upstream_branch()? {
	Some(upstream_main_branch) => upstream_main_branch,
	None => {
	writeln!(
	effects.get_output_stream(),
	"{local_main_branch_description} does not track an upstream branch, so not pulling."
	)?;
	return Ok(Ok(()));
	}
	};
	let upstream_main_branch_oid = match upstream_main_branch.get_oid()? {
	Some(upstream_main_branch_oid) => upstream_main_branch_oid,
	None => return Ok(Ok(())),
	};

and then maybe we would avoid hitting the unimplemented libgit2 code path? If you're interested, you could try it to see if it fixes your error.

[ryan-ph]

Thanks for all the code pointers! Will give it a shot when I get some free time (hopefully in the next week)

[ryan-ph]

I only glanced through it, but it's possible that we only need the OID of upstream branch, and not the full branch information. Namely, we might be able to call this function instead:

Seems like this doesn't work as the get_upstream_branch_target() calls get_upstream_branch() under the hood. It seems like gitoxide does support negative refspecs, but I guess migrating to gitoxide is probably a larger effort than waiting for a fix upstream 😢

[ryan-ph]

Once libgit2/libgit2#6962 is merged and a new release is made, I think we should be able to just update the library and close this issue out.

If a patch version is released, I think we should be able to just statically link the latest patch version when building git2-rs to pull in the fix. Otherwise I think we would need to create a PR in git2-rs to bump the major or minor version.

[bryango]

The awesome work of @ryan-ph is now released in libgit2 1.9.0:

• refs: Handle normalizing negative refspecs libgit2/libgit2#6951
• remote: Handle fetching negative refspecs libgit2/libgit2#6962

... and it's also released in git2-rs 0.20.0:

• Update libgit2 to 1.9 rust-lang/git2-rs#1111

... so we should be able to simply get the fix from upstream by bumping the git2 crate to 0.20.0 🎉

Update: I made PR #1485 but haven't actually tested the ^ref spec yet.