gitext-rs / git-stack

Stacked branch management for Git
Apache License 2.0
491 stars 19 forks source link

Crash in repository with symbolic reference branches #359

Open UrGuardian4ngel opened 1 week ago

UrGuardian4ngel commented 1 week ago

Please complete the following tasks

Description

I'm using the git symbolic-ref command to create "fake" branch references. It looks like git-stack (and additionally git-branch-stack) doesn't play well with those yet.

report-bb3a8cec-dc97-4362-956e-6def55fe6d1c.toml.zip

Background

Symbolic references allow me to quickly define some kind of short branch aliases for moving targets, e.g. version development branches. It boils down to something like this:

$ git symbolic-ref refs/heads/prev refs/remotes/origin/release-11
$ git symbolic-ref refs/heads/current refs/remotes/origin/release-12
$ git symbolic-ref refs/heads/next refs/remotes/origin/release-13

After which I can do things like this:

$ git checkout -d current
# I'm on detached commit of latest upstream "release-12"

$ git checkout -d prev
# I'm on detached commit of latest upstream "release-11"

$ git checkout -b hotfix/my-bugfix-branch -t prev
branch 'hotfix/my-bugfix-branch' set up to track 'origin/release-11' by rebasing.
Switched to new branch 'hotfix/my-bugfix-branch'

$ git checkout -b feature/my-feature-branch -t next
branch 'feature/my-feature-branch ' set up to track 'origin/release-13' by rebasing.
Switched to new branch 'feature/my-feature-branch'

... without having to clutter my local branches with tons of release-* branches that always lag behind their upstream.

When versions change, I just update the symbolic references. From now on, my "aliases" start pointing to the new versions.
No additional mental overhead required in fast-paced release cycles :ok_hand:!

Workaround

For now, I've made some local bandaid patch to silently ignore branches in local_branches() that are having kind: ReferenceType::Symbolic.

--- a/src/legacy/git/repo.rs
+++ b/src/legacy/git/repo.rs
@@ -527,6 +527,13 @@ impl GitRepo {
                     );
                     return None;
                 };
+                if branch.get().kind().unwrap() == git2::ReferenceType::Symbolic {
+                    log::debug!(
+                        "Ignoring symbolic ref {:?}",
+                        branch.name_bytes().unwrap().as_bstr()
+                    );
+                    return None;
+                }
                 self.load_local_branch(&branch, name).ok()
             })
     }

Would be nice if we could handle these symbolic references in load_local_branch(), and resolve them to their target's id?

Potential issues

:one: In my case, these symbolic refs mostly point to remote refs.
I guess this could also tie into discussion #93?

:two: These references could potentially be recursive: sym-a -> sym-b -> some-branch.

Version

git-stack 0.10.17

Steps to reproduce

$ git init ./reproducer && cd ./reproducer
$ echo '1' > ./file_a.txt && git add -A && git commit -m 'commit 1'
$ echo '2' > ./file_b.txt && git add -A && git commit -m 'commit 2'

$ git stack
master (no remote) commit 2

$ git symbolic-ref refs/heads/next refs/heads/master
$ git stack
# crashes

Actual Behaviour

git-stack crashes in a repository that has one or more symbolic ref branches.

Expected Behaviour

git-stack should not crash in a repository that has one or more symbolic ref branches.

Either by:

Debug Output

$ git stack -vv
[TRACE git_stack::stack] Initializing
[TRACE git_stack::config] Loading gitconfig
[TRACE git_stack::config] Loading /path/to/app/gitconfig
[TRACE git_stack::config] Loading /path/to/app/git/config
[TRACE git_stack::stack] Ignoring `auto-fixup=move` without an explicit `--rebase`
[TRACE git_stack::stack] Ignoring `auto-repair=true` without an explicit `--rebase`
[DEBUG globset] built glob set; 0 literals, 15 basenames, 0 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 0 regexes
[TRACE git_stack::legacy::git::repo] Loading local branches
[TRACE git_stack::legacy::git::protect] master: ignored "master"
[TRACE git_stack::stack] Branch master is protected
Well, this is embarrassing.

git-stack had a problem and crashed. To help us diagnose the problem you can send us a crash report.

We have generated a report file at "/tmp/report-4e0c7119-0aa6-4537-a04d-ec097ddd7223.toml". Submit an issue or email with the subject of "git-stack Crash Report" and include the report as an attachment.

We take privacy seriously, and do not perform any automated error collection. In order to improve the software, we rely on people to submit reports.

Thank you kindly!
UrGuardian4ngel commented 1 week ago

💡 FYI, about that resolution:

I also tried playing by replacing this: https://github.com/gitext-rs/git-stack/blob/bd68887f0dc690166599eaa6d32fd0f353edfd86/src/legacy/git/repo.rs#L593

with something along these lines:

let id = match branch.get().kind().unwrap() {
    git2::ReferenceType::Direct => branch.get().target().unwrap(),
    git2::ReferenceType::Symbolic => {
        let target_name = branch.get().symbolic_target().unwrap();
        let target_branch = self
            .repo
            .find_branch(target_name, git2::BranchType::Local)
            .or_else(|_| self.repo.find_branch(target_name, git2::BranchType::Remote))
            .unwrap();
        target_branch.get().target().unwrap()
    },
};

Caution: As a non-Rust dev myself, this probably looks ugly to others... 🫣