From 2ed1053ff9d440ec0e405e2f25157e25926633dd Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 13:45:06 +0100 Subject: [PATCH 01/14] Stop using deprecated function `ioutil.TempDir()` --- git_sizer_test.go | 3 +-- internal/testutils/repoutils.go | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/git_sizer_test.go b/git_sizer_test.go index 16d58c9..fbf470d 100644 --- a/git_sizer_test.go +++ b/git_sizer_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -760,7 +759,7 @@ func TestSubmodule(t *testing.T) { ctx := context.Background() - tmp, err := ioutil.TempDir("", "submodule") + tmp, err := os.MkdirTemp("", "submodule") require.NoError(t, err, "creating temporary directory") defer func() { diff --git a/internal/testutils/repoutils.go b/internal/testutils/repoutils.go index 60a2f9b..954cff4 100644 --- a/internal/testutils/repoutils.go +++ b/internal/testutils/repoutils.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -29,7 +28,7 @@ type TestRepo struct { func NewTestRepo(t *testing.T, bare bool, pattern string) *TestRepo { t.Helper() - path, err := ioutil.TempDir("", pattern) + path, err := os.MkdirTemp("", pattern) require.NoError(t, err) repo := TestRepo{Path: path} @@ -73,7 +72,7 @@ func (repo *TestRepo) Remove(t *testing.T) { func (repo *TestRepo) Clone(t *testing.T, pattern string) *TestRepo { t.Helper() - path, err := ioutil.TempDir("", pattern) + path, err := os.MkdirTemp("", pattern) require.NoError(t, err) err = repo.GitCommand( From c20cbb8693f82594b73d4a279390a1c7aa2b7644 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 12:45:42 +0100 Subject: [PATCH 02/14] Repository.gitDir: rename member from `path` The name `gitDir` is less ambiguous. Also rename method `Path()` to `GitDir()`. --- git/git.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/git/git.go b/git/git.go index f451c54..5531a6f 100644 --- a/git/git.go +++ b/git/git.go @@ -15,7 +15,10 @@ type ObjectType string // Repository represents a Git repository on disk. type Repository struct { - path string + // gitDir is the path to the `GIT_DIR` for this repository. It + // might be absolute or it might be relative to the current + // directory. + gitDir string // gitBin is the path of the `git` executable that should be used // when running commands in this repository. @@ -79,7 +82,7 @@ func NewRepository(path string) (*Repository, error) { } return &Repository{ - path: gitDir, + gitDir: gitDir, gitBin: gitBin, }, nil } @@ -103,7 +106,7 @@ func (repo *Repository) GitCommand(callerArgs ...string) *exec.Cmd { cmd.Env = append( os.Environ(), - "GIT_DIR="+repo.path, + "GIT_DIR="+repo.gitDir, // Disable grafts when running our commands: "GIT_GRAFT_FILE="+os.DevNull, ) @@ -111,7 +114,8 @@ func (repo *Repository) GitCommand(callerArgs ...string) *exec.Cmd { return cmd } -// Path returns the path to `repo`. -func (repo *Repository) Path() string { - return repo.path +// GitDir returns the path to `repo`'s `GIT_DIR`. It might be absolute +// or it might be relative to the current directory. +func (repo *Repository) GitDir() string { + return repo.gitDir } From 29fc88208a3a38f54fda3e7e555469bd6c8fff29 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 12:58:23 +0100 Subject: [PATCH 03/14] Repository.GitPath(): new method, extracted from `NewRepository()` Add a method `Repository.GitPath(relPath)`, which invokes `git rev-parse --git-path $relPath` to find the path to a file within the Git repository. In `NewRepository()`, instantiate the `Repository` object earlier so that the new method can be used to find the path to `shallow`. --- git/git.go | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/git/git.go b/git/git.go index 5531a6f..cba262d 100644 --- a/git/git.go +++ b/git/git.go @@ -66,25 +66,22 @@ func NewRepository(path string) (*Repository, error) { } gitDir := smartJoin(path, string(bytes.TrimSpace(out))) - //nolint:gosec // `gitBin` is chosen carefully. - cmd = exec.Command(gitBin, "rev-parse", "--git-path", "shallow") - cmd.Dir = gitDir - out, err = cmd.Output() + repo := Repository{ + gitDir: gitDir, + gitBin: gitBin, + } + + shallow, err := repo.GitPath("shallow") if err != nil { - return nil, fmt.Errorf( - "could not run 'git rev-parse --git-path shallow': %w", err, - ) + return nil, err } - shallow := smartJoin(gitDir, string(bytes.TrimSpace(out))) + _, err = os.Lstat(shallow) if err == nil { return nil, errors.New("this appears to be a shallow clone; full clone required") } - return &Repository{ - gitDir: gitDir, - gitBin: gitBin, - }, nil + return &repo, nil } func (repo *Repository) GitCommand(callerArgs ...string) *exec.Cmd { @@ -119,3 +116,20 @@ func (repo *Repository) GitCommand(callerArgs ...string) *exec.Cmd { func (repo *Repository) GitDir() string { return repo.gitDir } + +// GitPath returns that path of a file within the git repository, by +// calling `git rev-parse --git-path $relPath`. The returned path is +// relative to the current directory. +func (repo *Repository) GitPath(relPath string) (string, error) { + cmd := repo.GitCommand("rev-parse", "--git-path", relPath) + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf( + "running 'git rev-parse --git-path %s': %w", relPath, err, + ) + } + // `git rev-parse --git-path` is documented to return the path + // relative to the current directory. Since we haven't changed the + // current directory, we can use it as-is: + return string(bytes.TrimSpace(out)), nil +} From 1d75c744e2ed1ad45f469a356897b0e07ba9b7a2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 13:15:08 +0100 Subject: [PATCH 04/14] Repository.IsFull(): new method Extract a method to determine whether the repository seems to be a full clone. Call it from `NewRepository()`. --- git/git.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/git/git.go b/git/git.go index cba262d..a82d14c 100644 --- a/git/git.go +++ b/git/git.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "io/fs" "os" "os/exec" "path/filepath" @@ -71,17 +72,36 @@ func NewRepository(path string) (*Repository, error) { gitBin: gitBin, } + full, err := repo.IsFull() + if err != nil { + return nil, fmt.Errorf("determining whether the repository is a full clone: %w", err) + } + if !full { + return nil, errors.New("this appears to be a shallow clone; full clone required") + } + + return &repo, nil +} + +// IsFull returns `true` iff `repo` appears to be a full clone. +func (repo *Repository) IsFull() (bool, error) { shallow, err := repo.GitPath("shallow") if err != nil { - return nil, err + return false, err } _, err = os.Lstat(shallow) if err == nil { - return nil, errors.New("this appears to be a shallow clone; full clone required") + return false, nil } - return &repo, nil + if !errors.Is(err, fs.ErrNotExist) { + return false, err + } + + // The `shallow` file is absent, which is what we expect + // for a full clone. + return true, nil } func (repo *Repository) GitCommand(callerArgs ...string) *exec.Cmd { From 39102dfaa3c2fc57e53c9a909042bee382af1d11 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 13:27:01 +0100 Subject: [PATCH 05/14] findGitBin(): memoize the result --- git/git_bin.go | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/git/git_bin.go b/git/git_bin.go index fc03435..526e9bb 100644 --- a/git/git_bin.go +++ b/git/git_bin.go @@ -2,10 +2,20 @@ package git import ( "path/filepath" + "sync" "github.com/cli/safeexec" ) +// This variable will be used to memoize the result of `findGitBin()`, +// since its return value only depends on the environment. +var gitBinMemo struct { + once sync.Once + + gitBin string + err error +} + // findGitBin finds the `git` binary in PATH that should be used by // the rest of `git-sizer`. It uses `safeexec` to find the executable, // because on Windows, `exec.Cmd` looks not only in PATH, but also in @@ -13,15 +23,20 @@ import ( // being scanned is hostile and non-bare because it might possibly // contain an executable file named `git`. func findGitBin() (string, error) { - gitBin, err := safeexec.LookPath("git") - if err != nil { - return "", err - } + gitBinMemo.once.Do(func() { + p, err := safeexec.LookPath("git") + if err != nil { + gitBinMemo.err = err + return + } - gitBin, err = filepath.Abs(gitBin) - if err != nil { - return "", err - } + p, err = filepath.Abs(p) + if err != nil { + gitBinMemo.err = err + return + } - return gitBin, nil + gitBinMemo.gitBin = p + }) + return gitBinMemo.gitBin, gitBinMemo.err } From 51cf26bdfd5f80d278cc274427d91d593b585235 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 17:53:15 +0100 Subject: [PATCH 06/14] smartJoin(): improve docstring --- git/git.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/git/git.go b/git/git.go index a82d14c..3f51c53 100644 --- a/git/git.go +++ b/git/git.go @@ -26,9 +26,10 @@ type Repository struct { gitBin string } -// smartJoin returns the path that can be described as `relPath` -// relative to `path`, given that `path` is either absolute or is -// relative to the current directory. +// smartJoin returns `relPath` if it is an absolute path. If not, it +// assumes that `relPath` is relative to `path`, so it joins them +// together and returns the result. In that case, if `path` itself is +// relative, then the return value is also relative. func smartJoin(path, relPath string) string { if filepath.IsAbs(relPath) { return relPath From 02928f10bf9a42654333abc5d288c3e36b405477 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 13:35:19 +0100 Subject: [PATCH 07/14] NewRepositoryFromGitDir(): new function If you already have the desired `GIT_DIR`, there's no need to determine it from the current path. --- git/git.go | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/git/git.go b/git/git.go index 3f51c53..60a468a 100644 --- a/git/git.go +++ b/git/git.go @@ -37,8 +37,36 @@ func smartJoin(path, relPath string) string { return filepath.Join(path, relPath) } -// NewRepository creates a new repository object that can be used for -// running `git` commands within that repository. +// NewRepositoryFromGitDir creates a new `Repository` object that can +// be used for running `git` commands, given the value of `GIT_DIR` +// for the repository. +func NewRepositoryFromGitDir(gitDir string) (*Repository, error) { + // Find the `git` executable to be used: + gitBin, err := findGitBin() + if err != nil { + return nil, fmt.Errorf( + "could not find 'git' executable (is it in your PATH?): %w", err, + ) + } + + repo := Repository{ + gitDir: gitDir, + gitBin: gitBin, + } + + full, err := repo.IsFull() + if err != nil { + return nil, fmt.Errorf("determining whether the repository is a full clone: %w", err) + } + if !full { + return nil, errors.New("this appears to be a shallow clone; full clone required") + } + + return &repo, nil +} + +// NewRepository creates a new `Repository` object that can be used +// for running `git` commands within `path`. func NewRepository(path string) (*Repository, error) { // Find the `git` executable to be used: gitBin, err := findGitBin() @@ -68,20 +96,7 @@ func NewRepository(path string) (*Repository, error) { } gitDir := smartJoin(path, string(bytes.TrimSpace(out))) - repo := Repository{ - gitDir: gitDir, - gitBin: gitBin, - } - - full, err := repo.IsFull() - if err != nil { - return nil, fmt.Errorf("determining whether the repository is a full clone: %w", err) - } - if !full { - return nil, errors.New("this appears to be a shallow clone; full clone required") - } - - return &repo, nil + return NewRepositoryFromGitDir(gitDir) } // IsFull returns `true` iff `repo` appears to be a full clone. From f9aec5023a77e9336b6ec2f29bad7804caca57a6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 17:56:00 +0100 Subject: [PATCH 08/14] NewRepositoryFromPath(): function renamed from `NewRepository()` --- git-sizer.go | 2 +- git/git.go | 9 +++++---- internal/testutils/repoutils.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/git-sizer.go b/git-sizer.go index 0888d78..1ef9812 100644 --- a/git-sizer.go +++ b/git-sizer.go @@ -134,7 +134,7 @@ func mainImplementation(ctx context.Context, stdout, stderr io.Writer, args []st // Try to open the repository, but it's not an error yet if this // fails, because the user might only be asking for `--help`. - repo, repoErr := git.NewRepository(".") + repo, repoErr := git.NewRepositoryFromPath(".") flags := pflag.NewFlagSet("git-sizer", pflag.ContinueOnError) flags.Usage = func() { diff --git a/git/git.go b/git/git.go index 60a468a..096ce81 100644 --- a/git/git.go +++ b/git/git.go @@ -65,10 +65,11 @@ func NewRepositoryFromGitDir(gitDir string) (*Repository, error) { return &repo, nil } -// NewRepository creates a new `Repository` object that can be used -// for running `git` commands within `path`. -func NewRepository(path string) (*Repository, error) { - // Find the `git` executable to be used: +// NewRepositoryFromPath creates a new `Repository` object that can be +// used for running `git` commands within `path`. It does so by asking +// `git` what `GIT_DIR` to use. Git, in turn, bases its decision on +// the path and the environment. +func NewRepositoryFromPath(path string) (*Repository, error) { gitBin, err := findGitBin() if err != nil { return nil, fmt.Errorf( diff --git a/internal/testutils/repoutils.go b/internal/testutils/repoutils.go index 954cff4..e530925 100644 --- a/internal/testutils/repoutils.go +++ b/internal/testutils/repoutils.go @@ -89,7 +89,7 @@ func (repo *TestRepo) Clone(t *testing.T, pattern string) *TestRepo { func (repo *TestRepo) Repository(t *testing.T) *git.Repository { t.Helper() - r, err := git.NewRepository(repo.Path) + r, err := git.NewRepositoryFromPath(repo.Path) require.NoError(t, err) return r } From d605cdb7c5e61f2d24cc29445f30255488a046c0 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 13 Dec 2023 17:56:31 +0100 Subject: [PATCH 09/14] TestRepo: for bare repositories, use `NewRepositoryFromGitDir()` There's no need to deduce the `GIT_DIR` for a bare repository. --- internal/testutils/repoutils.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/testutils/repoutils.go b/internal/testutils/repoutils.go index e530925..48a8759 100644 --- a/internal/testutils/repoutils.go +++ b/internal/testutils/repoutils.go @@ -20,6 +20,7 @@ import ( // TestRepo represents a git repository used for tests. type TestRepo struct { Path string + bare bool } // NewTestRepo creates and initializes a test repository in a @@ -37,6 +38,7 @@ func NewTestRepo(t *testing.T, bare bool, pattern string) *TestRepo { return &TestRepo{ Path: path, + bare: bare, } } @@ -89,9 +91,15 @@ func (repo *TestRepo) Clone(t *testing.T, pattern string) *TestRepo { func (repo *TestRepo) Repository(t *testing.T) *git.Repository { t.Helper() - r, err := git.NewRepositoryFromPath(repo.Path) - require.NoError(t, err) - return r + if repo.bare { + r, err := git.NewRepositoryFromGitDir(repo.Path) + require.NoError(t, err) + return r + } else { + r, err := git.NewRepositoryFromPath(repo.Path) + require.NoError(t, err) + return r + } } // localEnvVars is a list of the variable names that should be cleared From fb78b414e22c5c95dfb4c4847b6e7cee58b1b1af Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 12 Dec 2023 11:27:10 +0100 Subject: [PATCH 10/14] Be mindful of `safe.bareRepository` in the tests As of Git v2.38.0, there is an option to prevent Git from accessing bare repositories unless asked for explicitly (via `--git-dir` or `GIT_DIR`): `safe.bareRepository`. The tests of `git sizer`, however, assume that Git will access a bare repository when the current directory points inside that repository. This only works if `safe.bareRepository` indicates that this is safe. If that is not the case, i.e. if `safe.bareRepository` is set to `explicit`, Git demands that the environment variable `GIT_DIR` is set (either explicitly, or via `--git-dir`) when accessing bare repositories. So let's set `GIT_DIR` for the test cases that work on bare repositories. Signed-off-by: Johannes Schindelin --- git_sizer_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/git_sizer_test.go b/git_sizer_test.go index fbf470d..8a7a2d2 100644 --- a/git_sizer_test.go +++ b/git_sizer_test.go @@ -272,7 +272,10 @@ func TestRefSelections(t *testing.T) { args := []string{"--show-refs", "--no-progress", "--json", "--json-version=2"} args = append(args, p.args...) cmd := exec.Command(executable, args...) - cmd.Dir = repo.Path + cmd.Env = append( + os.Environ(), + "GIT_DIR="+repo.Path, + ) var stdout bytes.Buffer cmd.Stdout = &stdout var stderr bytes.Buffer @@ -519,7 +522,10 @@ References (included references marked with '+'): args := append([]string{"--show-refs", "-v", "--no-progress"}, p.args...) cmd := exec.Command(executable, args...) - cmd.Dir = repo.Path + cmd.Env = append( + os.Environ(), + "GIT_DIR="+repo.Path, + ) var stdout bytes.Buffer cmd.Stdout = &stdout var stderr bytes.Buffer From 69418a954c18494a0d594930fd1b6ed7e6f420bb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 14 Dec 2023 09:56:42 +0100 Subject: [PATCH 11/14] Re-format some comments This is the result of running `go fmt -mod=readonly ./...` --- git/gitconfig.go | 8 ++++---- git/reference.go | 2 +- internal/refopts/filter_value.go | 6 +++--- sizes/output.go | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/git/gitconfig.go b/git/gitconfig.go index 76b8422..33a66ba 100644 --- a/git/gitconfig.go +++ b/git/gitconfig.go @@ -90,10 +90,10 @@ func (config *Config) FullKey(key string) string { // a component boundary (i.e., at a '.'). If yes, it returns `true` // and the part of the key after the prefix; e.g.: // -// configKeyMatchesPrefix("foo.bar", "foo") → true, "bar" -// configKeyMatchesPrefix("foo.bar", "foo.") → true, "bar" -// configKeyMatchesPrefix("foo.bar", "foo.bar") → true, "" -// configKeyMatchesPrefix("foo.bar", "foo.bar.") → false, "" +// configKeyMatchesPrefix("foo.bar", "foo") → true, "bar" +// configKeyMatchesPrefix("foo.bar", "foo.") → true, "bar" +// configKeyMatchesPrefix("foo.bar", "foo.bar") → true, "" +// configKeyMatchesPrefix("foo.bar", "foo.bar.") → false, "" func configKeyMatchesPrefix(key, prefix string) (bool, string) { if prefix == "" { return true, key diff --git a/git/reference.go b/git/reference.go index e8a1aaf..fc16371 100644 --- a/git/reference.go +++ b/git/reference.go @@ -27,7 +27,7 @@ type Reference struct { // `Reference`. It is assumed that `line` is formatted like the output // of // -// git for-each-ref --format='%(objectname) %(objecttype) %(objectsize) %(refname)' +// git for-each-ref --format='%(objectname) %(objecttype) %(objectsize) %(refname)' func ParseReference(line string) (Reference, error) { words := strings.Split(line, " ") if len(words) != 4 { diff --git a/internal/refopts/filter_value.go b/internal/refopts/filter_value.go index 5dec209..5ec1c91 100644 --- a/internal/refopts/filter_value.go +++ b/internal/refopts/filter_value.go @@ -76,9 +76,9 @@ func (v *filterValue) Set(s string) error { // // * If it is bracketed with `/` characters, treat it as a regexp. // -// * If it starts with `@`, then consider it a refgroup name. That -// refgroup must already be defined. Use its filter. This construct -// is only allowed at the top level. +// - If it starts with `@`, then consider it a refgroup name. That +// refgroup must already be defined. Use its filter. This construct +// is only allowed at the top level. // // * Otherwise treat it as a prefix. func (v *filterValue) interpretFlexibly(s string) (git.ReferenceFilter, error) { diff --git a/sizes/output.go b/sizes/output.go index 933cc05..83f7e59 100644 --- a/sizes/output.go +++ b/sizes/output.go @@ -279,10 +279,10 @@ func (t *Threshold) Type() string { // A `pflag.Value` that can be used as a boolean option that sets a // `Threshold` variable to a fixed value. For example, // -// pflag.Var( -// sizes.NewThresholdFlagValue(&threshold, 30), -// "critical", "only report critical statistics", -// ) +// pflag.Var( +// sizes.NewThresholdFlagValue(&threshold, 30), +// "critical", "only report critical statistics", +// ) // // adds a `--critical` flag that sets `threshold` to 30. type thresholdFlagValue struct { From 16b2fa2e0297fabb4273f3e36b563e78e47a139a Mon Sep 17 00:00:00 2001 From: Force Charlie Date: Mon, 17 Apr 2023 21:34:04 +0800 Subject: [PATCH 12/14] Update dependencies --- go.mod | 15 +++++++-------- go.sum | 20 ++++++++++++++------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 9db294d..7693cd4 100644 --- a/go.mod +++ b/go.mod @@ -3,18 +3,17 @@ module github.com/github/git-sizer go 1.17 require ( - github.com/cli/safeexec v1.0.0 - github.com/davecgh/go-spew v1.1.1 // indirect + github.com/cli/safeexec v1.0.1 + github.com/github/go-pipe v1.0.2 github.com/spf13/pflag v1.0.5 - github.com/stretchr/testify v1.8.1 - golang.org/x/sync v0.1.0 // indirect + github.com/stretchr/testify v1.8.4 ) -require github.com/github/go-pipe v1.0.2 - require ( - github.com/kr/pretty v0.1.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/kr/pretty v0.3.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect + golang.org/x/sync v0.5.0 // indirect + gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 5c5d0a9..82bcc40 100644 --- a/go.sum +++ b/go.sum @@ -1,19 +1,24 @@ -github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI= -github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q= +github.com/cli/safeexec v1.0.1 h1:e/C79PbXF4yYTN/wauC4tviMxEV13BwljGj0N9j+N00= +github.com/cli/safeexec v1.0.1/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/github/go-pipe v1.0.2 h1:befTXflsc6ir/h9f6Q7QCDmfojoBswD1MfQrPhmmSoA= github.com/github/go-pipe v1.0.2/go.mod h1:/GvNLA516QlfGGMtfv4PC/5/CdzL9X4af/AJYhmLD54= -github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -21,8 +26,9 @@ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSS github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo= @@ -36,8 +42,9 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= +golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -54,8 +61,9 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 69ac7d8e1d71d9dab8b2e3fdb2ec75b1e113b4bc Mon Sep 17 00:00:00 2001 From: Force Charlie Date: Mon, 17 Apr 2023 21:34:04 +0800 Subject: [PATCH 13/14] Introduce sha256 support for git-sizer --- git/git.go | 25 +++++++++++-- git/obj_iter.go | 6 ++-- git/obj_resolver.go | 4 +-- git/oid.go | 63 ++++++++++++++++++++++++++++----- git/tree.go | 19 +++++----- internal/testutils/repoutils.go | 2 +- sizes/graph.go | 3 +- sizes/output.go | 4 +-- 8 files changed, 97 insertions(+), 29 deletions(-) diff --git a/git/git.go b/git/git.go index 096ce81..d2b2d1e 100644 --- a/git/git.go +++ b/git/git.go @@ -23,7 +23,8 @@ type Repository struct { // gitBin is the path of the `git` executable that should be used // when running commands in this repository. - gitBin string + gitBin string + hashAlgo HashAlgo } // smartJoin returns `relPath` if it is an absolute path. If not, it @@ -49,9 +50,19 @@ func NewRepositoryFromGitDir(gitDir string) (*Repository, error) { ) } + hashAlgo := HashSHA1 + //nolint:gosec // `gitDir` is the path we need Git to access. + cmd := exec.Command(gitBin, "--git-dir", gitDir, "rev-parse", "--show-object-format") + if out, err := cmd.Output(); err == nil { + if string(bytes.TrimSpace(out)) == "sha256" { + hashAlgo = HashSHA256 + } + } + repo := Repository{ - gitDir: gitDir, - gitBin: gitBin, + gitDir: gitDir, + gitBin: gitBin, + hashAlgo: hashAlgo, } full, err := repo.IsFull() @@ -170,3 +181,11 @@ func (repo *Repository) GitPath(relPath string) (string, error) { // current directory, we can use it as-is: return string(bytes.TrimSpace(out)), nil } + +func (repo *Repository) HashAlgo() HashAlgo { + return repo.hashAlgo +} + +func (repo *Repository) HashSize() int { + return repo.hashAlgo.HashSize() +} diff --git a/git/obj_iter.go b/git/obj_iter.go index cecdc2a..c367f11 100644 --- a/git/obj_iter.go +++ b/git/obj_iter.go @@ -30,7 +30,7 @@ func (repo *Repository) NewObjectIter(ctx context.Context) (*ObjectIter, error) errCh: make(chan error), headerCh: make(chan BatchHeader), } - + hashHexSize := repo.HashSize() * 2 iter.p.Add( // Read OIDs from `iter.oidCh` and write them to `git // rev-list`: @@ -68,10 +68,10 @@ func (repo *Repository) NewObjectIter(ctx context.Context) (*ObjectIter, error) pipe.LinewiseFunction( "copy-oids", func(_ context.Context, _ pipe.Env, line []byte, stdout *bufio.Writer) error { - if len(line) < 40 { + if len(line) < hashHexSize { return fmt.Errorf("line too short: '%s'", line) } - if _, err := stdout.Write(line[:40]); err != nil { + if _, err := stdout.Write(line[:hashHexSize]); err != nil { return fmt.Errorf("writing OID to 'git cat-file': %w", err) } if err := stdout.WriteByte('\n'); err != nil { diff --git a/git/obj_resolver.go b/git/obj_resolver.go index 418e293..c2b8aa5 100644 --- a/git/obj_resolver.go +++ b/git/obj_resolver.go @@ -9,12 +9,12 @@ func (repo *Repository) ResolveObject(name string) (OID, error) { cmd := repo.GitCommand("rev-parse", "--verify", "--end-of-options", name) output, err := cmd.Output() if err != nil { - return NullOID, fmt.Errorf("resolving object %q: %w", name, err) + return repo.HashAlgo().NullOID(), fmt.Errorf("resolving object %q: %w", name, err) } oidString := string(bytes.TrimSpace(output)) oid, err := NewOID(oidString) if err != nil { - return NullOID, fmt.Errorf("parsing output %q from 'rev-parse': %w", oidString, err) + return repo.HashAlgo().NullOID(), fmt.Errorf("parsing output %q from 'rev-parse': %w", oidString, err) } return oid, nil } diff --git a/git/oid.go b/git/oid.go index 2aefbcb..c57ceae 100644 --- a/git/oid.go +++ b/git/oid.go @@ -1,31 +1,76 @@ package git import ( + "bytes" + //nolint:gosec // Git indeed does use SHA-1, still + "crypto/sha1" + "crypto/sha256" "encoding/hex" "errors" ) +const ( + HashSizeSHA256 = sha256.Size + HashSizeSHA1 = sha1.Size + HashSizeMax = HashSizeSHA256 +) + +type HashAlgo int + +const ( + HashUnknown HashAlgo = iota + HashSHA1 + HashSHA256 +) + // OID represents the SHA-1 object ID of a Git object, in binary // format. type OID struct { - v [20]byte + v [HashSizeMax]byte + hashSize int } -// NullOID is the null object ID; i.e., all zeros. -var NullOID OID +func (h HashAlgo) NullOID() OID { + switch h { + case HashSHA1: + return OID{hashSize: HashSizeSHA1} + case HashSHA256: + return OID{hashSize: HashSizeSHA256} + } + return OID{} +} + +func (h HashAlgo) HashSize() int { + switch h { + case HashSHA1: + return HashSizeSHA1 + case HashSHA256: + return HashSizeSHA256 + } + return 0 +} + +// defaultNullOID is the null object ID; i.e., all zeros. +var defaultNullOID OID + +func IsNullOID(o OID) bool { + return bytes.Equal(o.v[:], defaultNullOID.v[:]) +} // OIDFromBytes converts a byte slice containing an object ID in // binary format into an `OID`. func OIDFromBytes(oidBytes []byte) (OID, error) { var oid OID - if len(oidBytes) != len(oid.v) { + oidSize := len(oidBytes) + if oidSize != HashSizeSHA1 && oidSize != HashSizeSHA256 { return OID{}, errors.New("bytes oid has the wrong length") } - copy(oid.v[0:20], oidBytes) + oid.hashSize = oidSize + copy(oid.v[0:oidSize], oidBytes) return oid, nil } -// NewOID converts an object ID in hex format (i.e., `[0-9a-f]{40}`) +// NewOID converts an object ID in hex format (i.e., `[0-9a-f]{40,64}`) // into an `OID`. func NewOID(s string) (OID, error) { oidBytes, err := hex.DecodeString(s) @@ -37,18 +82,18 @@ func NewOID(s string) (OID, error) { // String formats `oid` as a string in hex format. func (oid OID) String() string { - return hex.EncodeToString(oid.v[:]) + return hex.EncodeToString(oid.v[:oid.hashSize]) } // Bytes returns a byte slice view of `oid`, in binary format. func (oid OID) Bytes() []byte { - return oid.v[:] + return oid.v[:oid.hashSize] } // MarshalJSON expresses `oid` as a JSON string with its enclosing // quotation marks. func (oid OID) MarshalJSON() ([]byte, error) { - src := oid.v[:] + src := oid.v[:oid.hashSize] dst := make([]byte, hex.EncodedLen(len(src))+2) dst[0] = '"' dst[len(dst)-1] = '"' diff --git a/git/tree.go b/git/tree.go index c31fa78..18cb3ee 100644 --- a/git/tree.go +++ b/git/tree.go @@ -10,13 +10,14 @@ import ( // Tree represents a Git tree object. type Tree struct { - data string + data string + hashSize int } // ParseTree parses the tree object whose contents are contained in // `data`. `oid` is currently unused. func ParseTree(oid OID, data []byte) (*Tree, error) { - return &Tree{string(data)}, nil + return &Tree{string(data), oid.hashSize}, nil } // Size returns the size of the tree object. @@ -36,13 +37,15 @@ type TreeEntry struct { // TreeIter is an iterator over the entries in a Git tree object. type TreeIter struct { // The as-yet-unread part of the tree's data. - data string + data string + hashSize int } // Iter returns an iterator over the entries in `tree`. func (tree *Tree) Iter() *TreeIter { return &TreeIter{ - data: tree.data, + data: tree.data, + hashSize: tree.hashSize, } } @@ -74,12 +77,12 @@ func (iter *TreeIter) NextEntry() (TreeEntry, bool, error) { entry.Name = iter.data[:nulAt] iter.data = iter.data[nulAt+1:] - if len(iter.data) < 20 { + if len(iter.data) < iter.hashSize { return TreeEntry{}, false, errors.New("tree entry ends unexpectedly") } - - copy(entry.OID.v[0:20], iter.data[0:20]) - iter.data = iter.data[20:] + entry.OID.hashSize = iter.hashSize + copy(entry.OID.v[0:iter.hashSize], iter.data[0:iter.hashSize]) + iter.data = iter.data[iter.hashSize:] return entry, true, nil } diff --git a/internal/testutils/repoutils.go b/internal/testutils/repoutils.go index 48a8759..e14e487 100644 --- a/internal/testutils/repoutils.go +++ b/internal/testutils/repoutils.go @@ -165,7 +165,7 @@ func (repo *TestRepo) UpdateRef(t *testing.T, refname string, oid git.OID) { var cmd *exec.Cmd - if oid == git.NullOID { + if git.IsNullOID(oid) { cmd = repo.GitCommand(t, "update-ref", "-d", refname) } else { cmd = repo.GitCommand(t, "update-ref", refname, oid.String()) diff --git a/sizes/graph.go b/sizes/graph.go index 0fb1c8a..888e793 100644 --- a/sizes/graph.go +++ b/sizes/graph.go @@ -37,6 +37,7 @@ func ScanRepositoryUsingGraph( nameStyle NameStyle, progressMeter meter.Progress, ) (HistorySize, error) { + nullOID := repo.HashAlgo().NullOID() graph := NewGraph(nameStyle) objIter, err := repo.NewObjectIter(ctx) @@ -134,7 +135,7 @@ func ScanRepositoryUsingGraph( case "tree": trees = append(trees, ObjectHeader{obj.OID, obj.ObjectSize}) case "commit": - commits = append(commits, CommitHeader{ObjectHeader{obj.OID, obj.ObjectSize}, git.NullOID}) + commits = append(commits, CommitHeader{ObjectHeader{obj.OID, obj.ObjectSize}, nullOID}) case "tag": tags = append(tags, ObjectHeader{obj.OID, obj.ObjectSize}) default: diff --git a/sizes/output.go b/sizes/output.go index 83f7e59..28ed130 100644 --- a/sizes/output.go +++ b/sizes/output.go @@ -155,7 +155,7 @@ func (i *item) Emit(t *table) { } func (i *item) Footnote(nameStyle NameStyle) string { - if i.path == nil || i.path.OID == git.NullOID { + if i.path == nil || git.IsNullOID(i.path.OID) { return "" } switch nameStyle { @@ -214,7 +214,7 @@ func (i *item) MarshalJSON() ([]byte, error) { LevelOfConcern: float64(value) / i.scale, } - if i.path != nil && i.path.OID != git.NullOID { + if i.path != nil && !git.IsNullOID(i.path.OID) { stat.ObjectName = i.path.OID.String() stat.ObjectDescription = i.path.Path() } From 0fc1aa334ff2ca4df1968c496fd0a8af00d21cff Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 14 Dec 2023 09:42:38 +0100 Subject: [PATCH 14/14] Add a test case for SHA-256 support Signed-off-by: Johannes Schindelin --- git_sizer_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/git_sizer_test.go b/git_sizer_test.go index 8a7a2d2..c74b459 100644 --- a/git_sizer_test.go +++ b/git_sizer_test.go @@ -849,3 +849,40 @@ func TestSubmodule(t *testing.T) { assert.Equal(t, counts.Count32(2), h.UniqueBlobCount, "unique blob count") assert.Equal(t, counts.Count32(3), h.MaxExpandedBlobCount, "max expanded blob count") } + +func TestSHA256(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + t.Helper() + + path, err := os.MkdirTemp("", "sha256") + require.NoError(t, err) + + testRepo := testutils.TestRepo{Path: path} + defer testRepo.Remove(t) + + // Don't use `GitCommand()` because the directory might not + // exist yet: + cmd := exec.Command("git", "init", "--object-format", "sha256", testRepo.Path) + cmd.Env = testutils.CleanGitEnv() + err = cmd.Run() + require.NoError(t, err) + + timestamp := time.Unix(1112911993, 0) + + testRepo.AddFile(t, "hello.txt", "Hello, world!\n") + cmd = testRepo.GitCommand(t, "commit", "-m", "initial") + testutils.AddAuthorInfo(cmd, ×tamp) + require.NoError(t, cmd.Run(), "creating initial commit") + + cmd = testRepo.GitCommand(t, "commit", "-m", "initial", "--allow-empty") + testutils.AddAuthorInfo(cmd, ×tamp) + require.NoError(t, cmd.Run(), "creating commit") + + repo := testRepo.Repository(t) + + _, err = sizes.CollectReferences(ctx, repo, refGrouper{}) + require.NoError(t, err) +}