Skip to content

Commit

Permalink
fix(git): Two behavior fixes for Sort. (#20)
Browse files Browse the repository at this point in the history
- Move the default branch up top.
  - Handle branches that point to the same commit correctly.
  • Loading branch information
JeffFaer authored Mar 18, 2024
1 parent 2c3a09a commit 68bb388
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 21 deletions.
53 changes: 35 additions & 18 deletions git/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,13 @@ func (repo *gitRepo) Sort(workUnits []string) error {
return nil
}

workUnitsByHash, err := repo.keyByHash(workUnits)
branchesByHash, err := repo.keyBranchByHash(workUnits)
if err != nil {
return err
}
slog.Debug("Found hashes for branches.", "hashes", branchesByHash)

args := []string{"rev-list", "--topo-order", "--format=format:%H", "--reverse"}
args := []string{"rev-list", "--topo-order", "--reverse"}
// We're reversing the output of rev-list, which will use its command line for
// tie breakers. So reverse the order of our work units so that they'll be
// sorted correctly in the output.
Expand All @@ -182,12 +183,12 @@ func (repo *gitRepo) Sort(workUnits []string) error {
}
var i int
r := bufio.NewReader(stdout)
for i < len(workUnitsByHash) {
for i < len(workUnits) {
hash, err := r.ReadString('\n')
if hash != "" {
hash = strings.TrimSuffix(hash, "\n")
if wu := workUnitsByHash[hash]; wu != "" {
workUnits[i] = wu
for _, b := range branchesByHash[hash] {
workUnits[i] = b
i++
}
}
Expand All @@ -199,38 +200,53 @@ func (repo *gitRepo) Sort(workUnits []string) error {
return err
}
}
if i != len(workUnitsByHash) {
if n := len(workUnits); i != n {
found := make(map[string]bool)
for _, wu := range workUnits[:i] {
found[wu] = true
}
var missing []string
for wu := range workUnitsByHash {
if !found[wu] {
missing = append(missing, wu)
for _, branches := range branchesByHash {
for _, b := range branches {
if !found[b] {
missing = append(missing, b)
}
}
}
return fmt.Errorf("only able to topologically sort %d of %d branches: unsortable branches: %q", i, len(workUnitsByHash), missing)
return fmt.Errorf("only able to topologically sort %d of %d branches: unsortable branches: %q", i, n, missing)
}
if err := cmd.Process.Kill(); err != nil {
slog.Warn("Problem killing rev-list command early.", "error", err)
}

// Move the default branch up top.
defaultBranch, err := repo.defaultBranchName()
if err != nil {
return err
}
isDefault := func(name string) bool { return name == defaultBranch }
slices.SortStableFunc(workUnits, morecmp.ComparingFunc(isDefault, morecmp.TrueFirst()))

return nil
}

func (repo *gitRepo) keyByHash(objects []string) (map[string]string, error) {
if len(objects) == 0 {
func (repo *gitRepo) keyBranchByHash(branches []string) (map[string][]string, error) {
if len(branches) == 0 {
return nil, nil
}
args := []string{"rev-list", "--no-walk=unsorted"}
args = append(args, objects...)
args := []string{"branch", "--list", "--format=%(refname:short) %(objectname)"}
args = append(args, branches...)
stdout, err := repo.Command(args...).RunStdout()
if err != nil {
return nil, fmt.Errorf("could not get object hashes: %w", err)
return nil, fmt.Errorf("could not get branch hashes: %w", err)
}
ret := make(map[string]string)
for i, hash := range strings.Split(stdout, "\n") {
ret[hash] = objects[i]
ret := make(map[string][]string)
for _, line := range strings.Split(stdout, "\n") {
if line == "" {
break
}
sp := strings.Split(line, " ")
ret[sp[1]] = append(ret[sp[1]], sp[0])
}
return ret, nil
}
Expand All @@ -252,6 +268,7 @@ func (repo *gitRepo) defaultBranchName() (string, error) {
}
for _, n := range slices.Compact([]string{def, "master"}) {
if repo.branchExists(n) {
slog.Debug("Found default branch name.", "name", n)
return n, nil
}
}
Expand Down
41 changes: 41 additions & 0 deletions git/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"fmt"
"os"
"path/filepath"
"slices"
"strings"
"testing"

"github.com/JeffFaer/tmux-vcs-sync/api"
"github.com/JeffFaer/tmux-vcs-sync/api/exec"
"github.com/JeffFaer/tmux-vcs-sync/api/exec/exectest"
"github.com/JeffFaer/tmux-vcs-sync/api/repotest"
"github.com/google/go-cmp/cmp"
"github.com/kballard/go-shellquote"
)

Expand Down Expand Up @@ -50,6 +52,7 @@ func (git testGit) newRepo(dir string, name string) (*testGitRepo, error) {
return nil, err
}
gitRepo := &testGitRepo{repo.(*gitRepo)}
gitRepo.gitRepo.git = git.git
if err := gitRepo.addEmptyCommit("Initial commit."); err != nil {
return nil, err
}
Expand Down Expand Up @@ -241,6 +244,31 @@ func TestCurrent(t *testing.T) {
}
}

func TestSort_DuplicateBranch(t *testing.T) {
git := newGit(t)
repo, err := git.newRepo(t.TempDir(), t.Name())
if err != nil {
t.Fatalf("Could not create repo: %v", err)
}
addBranch := func(name string) initStep {
return repoCommand{args: []string{"branch", name}}
}
commit := func(name string) initStep {
return apiCommand{"commit " + name, func(repo *testGitRepo) error { return repo.Commit(name) }}
}

initializeRepo(t, repo, []initStep{addBranch("foo"), commit("bar")})

branches := []string{"foo", defaultBranchName, "bar"}
got := slices.Clone(branches)
if err := repo.Sort(got); err != nil {
t.Errorf("repo.Sort(%q) = %v", branches, err)
}
if diff := cmp.Diff([]string{"main", "foo", "bar"}, got); diff != "" {
t.Errorf("repo.Sort(%q) diff (-want +got)\n%s", branches, diff)
}
}

type initStep interface {
Run(*testGitRepo) error
String() string
Expand Down Expand Up @@ -282,6 +310,19 @@ func (cmd newFile) String() string {
return fmt.Sprintf("echo %q > %q", cmd.content, cmd.path)
}

type apiCommand struct {
name string
cmd func(*testGitRepo) error
}

func (cmd apiCommand) Run(repo *testGitRepo) error {
return cmd.cmd(repo)
}

func (cmd apiCommand) String() string {
return cmd.name
}

func initializeRepo(t *testing.T, repo *testGitRepo, init []initStep) {
t.Helper()
for i, step := range init {
Expand Down
2 changes: 1 addition & 1 deletion git/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ go 1.22.0
require (
github.com/JeffFaer/go-stdlib-ext v0.2.0
github.com/JeffFaer/tmux-vcs-sync/api v0.0.0-20240314045224-4f466c92bafd
github.com/google/go-cmp v0.6.0
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
)

require (
github.com/adrg/xdg v0.4.0 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/magefile/mage v1.15.0 // indirect
golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359 // indirect
)
4 changes: 2 additions & 2 deletions git/go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
github.com/JeffFaer/go-stdlib-ext v0.1.1 h1:M8JFdsiV9Uyi8CFSTZr/TCAoE8bdVYuDlGvudxVkgHw=
github.com/JeffFaer/go-stdlib-ext v0.1.1/go.mod h1:+ipHu7aPnk5LuKfU1dda8+eIEfXNF2m7bC4porFJKXs=
github.com/JeffFaer/go-stdlib-ext v0.2.0 h1:jUNUFurbzFdh1XtENCrb+QIaHeWUe8YneHvUs81hW5M=
github.com/JeffFaer/go-stdlib-ext v0.2.0/go.mod h1:+ipHu7aPnk5LuKfU1dda8+eIEfXNF2m7bC4porFJKXs=
github.com/JeffFaer/tmux-vcs-sync/api v0.0.0-20240314045224-4f466c92bafd h1:WJgnYObOv3BnL+AV+mUCIzGthpypB5xb3bHAvJxvbCs=
github.com/JeffFaer/tmux-vcs-sync/api v0.0.0-20240314045224-4f466c92bafd/go.mod h1:3hqch0mjY8h884063yZLPulFxvxbsoB9Pir02j3ubt4=
github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls=
Expand Down

0 comments on commit 68bb388

Please sign in to comment.