Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync subcommand with --pull does not support negative refspecs #1444

Open
ryan-ph opened this issue Nov 12, 2024 · 3 comments
Open

sync subcommand with --pull does not support negative refspecs #1444

ryan-ph opened this issue Nov 12, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@ryan-ph
Copy link

ryan-ph commented Nov 12, 2024

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/_*

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 ryan-ph added the bug Something isn't working label Nov 12, 2024
@ryan-ph ryan-ph changed the title sync subcommand does not support negative refspecs sync subcommand with --pull does not support negative refspecs Nov 12, 2024
@ryan-ph
Copy link
Author

ryan-ph commented Nov 12, 2024

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

@arxanas
Copy link
Owner

arxanas commented Nov 19, 2024

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:

/// 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:

/// 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:

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
Copy link
Author

ryan-ph commented Nov 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants