Skip to content

Commit

Permalink
internal/git: implement CommonDir in-process
Browse files Browse the repository at this point in the history
Turns out, older versions of Git have a version of --git-common-dir that
returns incorrect results when run from a subdirectory. I've implemented
the algorithm described in the gitrepository-layout man page. I wrote
the tests and they pass on a non-buggy Git using rev-parse, then
implemented the new algorithm and got the same results.

Added a regression test case to `gg gerrithook`.

Fixes #105
  • Loading branch information
zombiezen committed Jul 23, 2019
1 parent e05b4ee commit 5c33638
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 11 deletions.
34 changes: 34 additions & 0 deletions cmd/gg/gerrithook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,40 @@ func TestGerritHook(t *testing.T) {
t.Errorf(".git/hooks/commit-msg content = %q; want %q", got, wantContent)
}
})
t.Run("Subdir", func(t *testing.T) {
// Regression test for https://github.com/zombiezen/gg/issues/105

env, err := newTestEnv(ctx, t)
if err != nil {
t.Fatal(err)
}
defer env.cleanup()
if err := env.git.Init(ctx, "."); err != nil {
t.Fatal(err)
}
if err := env.root.Apply(filesystem.Mkdir("foo")); err != nil {
t.Fatal(err)
}
const wantContent = "#!/bin/bash\necho Hello World\n"
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if want := "/foo/commit-msg"; r.URL.Path != want {
t.Errorf("HTTP path = %q; want %q", r.URL.Path, want)
}
w.Header().Set("Content-Length", strconv.Itoa(len(wantContent)))
io.WriteString(w, wantContent)
}))
defer srv.Close()
env.roundTripper = srv.Client().Transport

if _, err := env.gg(ctx, env.root.FromSlash("foo"), "gerrithook", "--url="+srv.URL+"/foo/commit-msg", "on"); err != nil {
t.Errorf("%+v", err)
}
if got, err := env.root.ReadFile(".git/hooks/commit-msg"); err != nil {
t.Error(err)
} else if got != wantContent {
t.Errorf(".git/hooks/commit-msg content = %q; want %q", got, wantContent)
}
})
t.Run("Cached", func(t *testing.T) {
env, err := newTestEnv(ctx, t)
if err != nil {
Expand Down
34 changes: 23 additions & 11 deletions internal/git/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -69,7 +70,7 @@ func (g *Git) GitDir(ctx context.Context) (string, error) {
// TODO(someday): Use --absolute-git-dir when minimum supported
// Git version >= 2.13.2.
const errPrefix = "find .git directory"
out, err := g.output(ctx, errPrefix, []string{g.exe, "rev-parse", "--git-common-dir"})
out, err := g.output(ctx, errPrefix, []string{g.exe, "rev-parse", "--git-dir"})
if err != nil {
return "", err
}
Expand All @@ -94,22 +95,33 @@ func (g *Git) GitDir(ctx context.Context) (string, error) {
// shared among different working trees, given the configuration. Any
// symlinks are resolved.
func (g *Git) CommonDir(ctx context.Context) (string, error) {
// TODO(someday): Use --git-common-dir when minimum supported
// Git version > 2.12.5. See https://github.com/zombiezen/gg/issues/105.
const errPrefix = "find .git directory"
out, err := g.output(ctx, errPrefix, []string{g.exe, "rev-parse", "--git-common-dir"})
for i := len(g.env) - 1; i >= 0; i-- {
e := g.env[i]
const envVar = "GIT_COMMON_DIR="
if len(e) > len(envVar) && strings.HasPrefix(e, envVar) {
return e[len(envVar):], nil
}
}
dir, err := g.GitDir(ctx)
if err != nil {
return "", err
}
line, err := oneLine(out)
if err != nil {
commonDirData, err := ioutil.ReadFile(filepath.Join(dir, "commondir"))
commonDir := dir
if err == nil {
commonDir = strings.TrimSuffix(string(commonDirData), "\n")
if filepath.IsAbs(commonDir) {
commonDir = filepath.Clean(commonDir)
} else {
commonDir = filepath.Join(dir, commonDir)
}
} else if !os.IsNotExist(err) {
return "", xerrors.Errorf("%s: %w", errPrefix, err)
}
var path string
if filepath.IsAbs(line) {
path = filepath.Clean(line)
} else {
path = filepath.Join(g.dir, line)
}
evaled, err := filepath.EvalSymlinks(path)
evaled, err := filepath.EvalSymlinks(commonDir)
if err != nil {
return "", xerrors.Errorf("%s: %w", errPrefix, err)
}
Expand Down
107 changes: 107 additions & 0 deletions internal/git/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
package git

import (
"context"
"os/exec"
"testing"

"gg-scm.io/pkg/internal/filesystem"
"golang.org/x/xerrors"
)

Expand Down Expand Up @@ -92,3 +94,108 @@ func TestCommandError(t *testing.T) {
}
}
}

func TestDirs(t *testing.T) {
gitPath, err := findGit()
if err != nil {
t.Skip("git not found:", err)
}
ctx := context.Background()

t.Run("SingleRepo", func(t *testing.T) {
env, err := newTestEnv(ctx, gitPath)
if err != nil {
t.Fatal(err)
}
defer env.cleanup()
if err := env.g.Init(ctx, "."); err != nil {
t.Fatal(err)
}
if err := env.root.Apply(filesystem.Mkdir("foo")); err != nil {
t.Fatal(err)
}

if got, err := env.g.GitDir(ctx); err != nil {
t.Error("env.g.GitDir(ctx):", err)
} else if want := env.root.FromSlash(".git"); got != want {
t.Errorf("env.g.GitDir(ctx) = %q; want %q", got, want)
}
if got, err := env.g.CommonDir(ctx); err != nil {
t.Error("env.g.CommonDir(ctx):", err)
} else if want := env.root.FromSlash(".git"); got != want {
t.Errorf("env.g.CommonDir(ctx) = %q; want %q", got, want)
}
t.Run("Subdir", func(t *testing.T) {
// Regression test for https://github.com/zombiezen/gg/issues/105

g := env.g.WithDir(env.root.FromSlash("foo"))
if got, err := g.GitDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo\").GitDir(ctx):", err)
} else if want := env.root.FromSlash(".git"); got != want {
t.Errorf("env.g.WithDir(\"foo\").GitDir(ctx) = %q; want %q", got, want)
}
if got, err := g.CommonDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo\").CommonDir(ctx):", err)
} else if want := env.root.FromSlash(".git"); got != want {
t.Errorf("env.g.WithDir(\"foo\").CommonDir(ctx) = %q; want %q", got, want)
}
})
})
t.Run("WorkTree", func(t *testing.T) {
env, err := newTestEnv(ctx, gitPath)
if err != nil {
t.Fatal(err)
}
defer env.cleanup()
// Create a repository with a single commit.
sharedDir := env.root.FromSlash("shared")
if err := env.g.Init(ctx, sharedDir); err != nil {
t.Fatal(err)
}
if err := env.root.Apply(filesystem.Write("shared/file.txt", dummyContent)); err != nil {
t.Fatal(err)
}
sharedGit := env.g.WithDir(sharedDir)
if err := sharedGit.Add(ctx, []Pathspec{"file.txt"}, AddOptions{}); err != nil {
t.Fatal(err)
}
if err := sharedGit.Commit(ctx, "first", CommitOptions{}); err != nil {
t.Fatal(err)
}
// Create linked worktree "foo".
linkedDir := env.root.FromSlash("foo")
if err := env.g.WithDir(sharedDir).Run(ctx, "worktree", "add", linkedDir); err != nil {
t.Fatal(err)
}
if err := env.root.Apply(filesystem.Mkdir("foo/bar")); err != nil {
t.Fatal(err)
}

linkedGit := env.g.WithDir(env.root.FromSlash("foo"))
if got, err := linkedGit.GitDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo\").GitDir(ctx):", err)
} else if want := env.root.FromSlash("shared/.git/worktrees/foo"); got != want {
t.Errorf("env.g.WithDir(\"foo\").GitDir(ctx) = %q; want %q", got, want)
}
if got, err := linkedGit.CommonDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo\").CommonDir(ctx):", err)
} else if want := env.root.FromSlash("shared/.git"); got != want {
t.Errorf("env.g.WithDir(\"foo\").CommonDir(ctx) = %q; want %q", got, want)
}
t.Run("Subdir", func(t *testing.T) {
// Regression test for https://github.com/zombiezen/gg/issues/105

subdirGit := env.g.WithDir(env.root.FromSlash("foo/bar"))
if got, err := subdirGit.GitDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo/bar\").GitDir(ctx):", err)
} else if want := env.root.FromSlash("shared/.git/worktrees/foo"); got != want {
t.Errorf("env.g.WithDir(\"foo/bar\").GitDir(ctx) = %q; want %q", got, want)
}
if got, err := subdirGit.CommonDir(ctx); err != nil {
t.Error("env.g.WithDir(\"foo/bar\").CommonDir(ctx):", err)
} else if want := env.root.FromSlash("shared/.git"); got != want {
t.Errorf("env.g.WithDir(\"foo/bar\").CommonDir(ctx) = %q; want %q", got, want)
}
})
})
}

0 comments on commit 5c33638

Please sign in to comment.