diff --git a/actions/review.go b/actions/review.go index 7bc9e1c..9c41897 100644 --- a/actions/review.go +++ b/actions/review.go @@ -2,13 +2,11 @@ package actions import ( "bufio" + "bytes" "context" "fmt" - "io/ioutil" "net/http" - "os" - "os/user" - "path" + "os/exec" "regexp" "strings" "text/tabwriter" @@ -18,11 +16,9 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" - gitConfig "github.com/go-git/go-git/v5/plumbing/format/config" "github.com/go-git/go-git/v5/plumbing/object" "github.com/google/go-github/v32/github" "github.com/pkg/errors" - "github.com/shibumi/go-pathspec" "github.com/shurcooL/graphql" "github.com/urfave/cli/v2" ) @@ -119,118 +115,25 @@ func Review(c *cli.Context) error { } func checkCleanWorktree(ctx context.Context, gitHubRepo *gitHubRepo) error { - worktree, err := gitHubRepo.GitRepo().Worktree() - if err != nil { - return errors.WithStack(err) - } - status, err := worktree.Status() + // Worktree.Status() is very slow so fall back to the command line instead. + // https://github.com/go-git/go-git/issues/181 + cmd := exec.Command("git", "status", "--porcelain") + var stdout bytes.Buffer + cmd.Stdout = &stdout + err := cmd.Run() if err != nil { + var exitErr exec.ExitError + if errors.As(err, &exitErr) { + return errors.Errorf("git status failed: %s", exitErr.Stderr) + } return errors.WithStack(err) } - if !status.IsClean() { - // go-git's handling of gitignore is not reliable, so we iterate over - // un-clean files again to make sure that files to be ignored are - // properly ignored based on the new ignore patterns. - ignorePatterns, err := getGitIgnorePatterns() - if err != nil { - return err - } - for filePath := range status { - match, err := pathspec.GitIgnore(ignorePatterns, filePath) - if err != nil { - return err - } - if match { - delete(status, filePath) - } - } - if len(status) > 0 { - return errors.Errorf("index is not clean:\n%v", status) - } + if stdout.Len() > 0 { + return errors.Errorf("index is not clean") } return nil } -// getGitIgnorePatterns returns all patterns to ignore for a given project, -// by looking at the project's gitignore, global gitignore, and system gitignore -func getGitIgnorePatterns() ([]string, error) { - var ignorePatterns []string - projectIgnorePatterns, err := getIgnorePatterns("./.gitignore") - if err != nil { - return nil, err - } - ignorePatterns = append(ignorePatterns, projectIgnorePatterns...) - currentUser, err := user.Current() - if err != nil { - return nil, err - } - globalIgnorePath, err := getIgnoreFilePathFromConfig(path.Join(currentUser.HomeDir, ".gitconfig")) - if err != nil && !os.IsNotExist(err) { - return nil, err - } - globalIgnorePatterns, err := getIgnorePatterns(globalIgnorePath) - if err != nil { - return nil, err - } - ignorePatterns = append(ignorePatterns, globalIgnorePatterns...) - systemIgnorePath, err := getIgnoreFilePathFromConfig("/etc/gitconfig") - if err != nil && !os.IsNotExist(err) { - return nil, err - } - systemIgnorePatterns, err := getIgnorePatterns(systemIgnorePath) - if err != nil { - return nil, err - } - ignorePatterns = append(ignorePatterns, systemIgnorePatterns...) - var allPatterns []string - for _, pattern := range ignorePatterns { - allPatterns = append(allPatterns, pattern) - // for any pattern that has no wildcard and slash, add a slash at the end - // to make sure that a new rule is added that every file in a directory - // is ignored. this is to handle go-git's behaviour where certain files - // are not ignored (e.g., node_modules/dist/.gitkeep is not ignored even - // when there is a pattern `node_modules` in gitignore file. - if !strings.Contains(pattern, "**") && - !strings.HasPrefix(pattern, "/") && - !strings.HasSuffix(pattern, "/") { - allPatterns = append(allPatterns, pattern, pattern+"/") - } - } - return allPatterns, nil -} - -// getIgnoreFilePathFromConfig returns path to a gitignore file, -// specified given config file -func getIgnoreFilePathFromConfig(configPath string) (string, error) { - configContent, err := os.Open(configPath) - if err != nil { - return "", err - } - decoder := gitConfig.NewDecoder(configContent) - raw := gitConfig.New() - if err = decoder.Decode(raw); err != nil { - return "", err - } - configSection := raw.Section("core") - return configSection.Options.Get("excludesfile"), nil -} - -// getIgnorePatterns reads a gitignore file and returns its list of patterns -// in a slice of strings -func getIgnorePatterns(filePath string) ([]string, error) { - content, err := ioutil.ReadFile(filePath) - if err != nil && !os.IsNotExist(err) { - return []string{}, err - } - var patterns []string - for _, pattern := range strings.Split(string(content), "\n") { - if pattern != "" { - patterns = append(patterns, pattern) - } - } - return patterns, nil -} - func getReviewInfo( ctx context.Context, gitHubRepo *gitHubRepo, diff --git a/go.mod b/go.mod index c819540..3037f1f 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/go-git/go-git/v5 v5.3.0 github.com/google/go-github/v32 v32.1.0 github.com/pkg/errors v0.9.1 - github.com/shibumi/go-pathspec v1.2.0 github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f github.com/urfave/cli/v2 v2.2.0 github.com/zalando/go-keyring v0.1.1 diff --git a/go.sum b/go.sum index 7039a2b..88b27d0 100644 --- a/go.sum +++ b/go.sum @@ -13,8 +13,6 @@ github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYU github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q= -github.com/cli/oauth v0.8.0 h1:YTFgPXSTvvDUFti3tR4o6q7Oll2SnQ9ztLwCAn4/IOA= -github.com/cli/oauth v0.8.0/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4= github.com/cli/oauth v0.9.0 h1:nxBC0Df4tUzMkqffAB+uZvisOwT3/N9FpkfdTDtafxc= github.com/cli/oauth v0.9.0/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4= github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q= @@ -90,8 +88,6 @@ github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= -github.com/shibumi/go-pathspec v1.2.0 h1:KVKEDHYk7bQolRMs7nfzjT3SBOCgcXFJzccnj9bsGbA= -github.com/shibumi/go-pathspec v1.2.0/go.mod h1:bDxCftD0fST3qXIlHoQ/fChsU4mWMVklXp1yPErQaaY= github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f h1:tygelZueB1EtXkPI6mQ4o9DQ0+FKW41hTbunoXZCTqk= github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f/go.mod h1:AuYgA5Kyo4c7HfUmvRGs/6rGlMMV/6B1bVnB9JxJEEg= github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= @@ -131,7 +127,6 @@ golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210324051608-47abb6519492 h1:Paq34FxTluEPvVyayQqMPgHm+vTOrIifmcYxFBx9TLg= golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71 h1:X/2sJAybVknnUnV7AD2HdT6rm2p5BP6eH2j+igduWgk= golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=