diff --git a/config/config.go b/config/config.go index e4b71fce..379e9d3b 100644 --- a/config/config.go +++ b/config/config.go @@ -62,7 +62,7 @@ type Refs struct { } type GlobalConfig struct { - // GithubReportType is the format of the report, will be used if linterConfig.GithubReportFormat is empty. + // GithubReportType/GitlabReportType is the format of the report, will be used if linterConfig.ReportFormat is empty. // e.g. "github_checks", "github_pr_review" GithubReportType ReportType `json:"githubReportType,omitempty"` GitlabReportType ReportType `json:"gitlabReportType,omitempty"` @@ -173,8 +173,7 @@ type Linter struct { // github_pr_review: https://developer.github.com/v3/pulls/reviews/#create-a-pull-request-review // Note: // * github_check_run only support on Github Apps, not support on Github OAuth Apps or authenticated users. - GithubReportFormat ReportType `json:"githubReportType,omitempty"` - GitlabReportFormat ReportType `json:"gitlabReportType,omitempty"` + ReportFormat ReportType `json:"githubReportType,omitempty"` // ConfigPath is the path of the linter config file. // If not empty, use the config to run the linter. @@ -187,7 +186,7 @@ type Linter struct { func (l Linter) String() string { return fmt.Sprintf( "Linter{Enable: %v, DockerAsRunner: %v, Workspace: %v, WorkDir: %v, Command: %v, Args: %v, ReportFormat: %v, ConfigPath: %v}", - *l.Enable, l.DockerAsRunner, l.Workspace, l.WorkDir, l.Command, l.Args, l.GithubReportFormat, l.ConfigPath) + *l.Enable, l.DockerAsRunner, l.Workspace, l.WorkDir, l.Command, l.Args, l.ReportFormat, l.ConfigPath) } var ( @@ -256,15 +255,19 @@ func NewConfig(conf string) (Config, error) { return c, nil } -func (c Config) GetLinterConfig(org, repo, ln string) Linter { +func (c Config) GetLinterConfig(org, repo, ln string, repotype RepoType) Linter { linter := Linter{ - Enable: boolPtr(true), - GithubReportFormat: c.GlobalDefaultConfig.GithubReportType, - GitlabReportFormat: c.GlobalDefaultConfig.GitlabReportType, - Modifier: NewBaseModifier(), - Name: ln, - Org: org, - Repo: repo, + Enable: boolPtr(true), + Modifier: NewBaseModifier(), + Name: ln, + Org: org, + Repo: repo, + } + if repotype == GitLab { + linter.ReportFormat = c.GlobalDefaultConfig.GithubReportType + } + if repotype == GitHub { + linter.ReportFormat = c.GlobalDefaultConfig.GithubReportType } // set golangci-lint config path if exists @@ -332,11 +335,8 @@ func applyCustomConfig(legacy, custom Linter) Linter { legacy.Args = custom.Args } - if custom.GithubReportFormat != "" { - legacy.GithubReportFormat = custom.GithubReportFormat - } - if custom.GitlabReportFormat != "" { - legacy.GitlabReportFormat = custom.GitlabReportFormat + if custom.ReportFormat != "" { + legacy.ReportFormat = custom.ReportFormat } if custom.ConfigPath != "" { @@ -392,6 +392,13 @@ const ( Quiet ReportType = "quiet" ) +type RepoType string + +const ( + GitLab RepoType = "gitlab" + GitHub RepoType = "github" +) + func boolPtr(b bool) *bool { return &b } diff --git a/config/config_test.go b/config/config_test.go index ad22b7ad..23ef0440 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -487,7 +487,7 @@ customConfig: # custom config for specific orgs or repos for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - got := c.GetLinterConfig(tc.org, tc.repo, tc.linter) + got := c.GetLinterConfig(tc.org, tc.repo, tc.linter, GitHub) if *got.Enable != *tc.want.Enable { t.Errorf("expected %v, got %v", *tc.want.Enable, *got.Enable) } diff --git a/internal/linters/agent.go b/internal/linters/agent.go index 1a801b3c..11b412d9 100644 --- a/internal/linters/agent.go +++ b/internal/linters/agent.go @@ -59,7 +59,7 @@ type Agent struct { func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string][]LinterOutput) { log := lintersutil.FromContext(ctx) var msgFormat string - format := a.LinterConfig.GithubReportFormat + format := a.LinterConfig.ReportFormat switch format { case config.GithubCheckRuns: msgFormat = "%s\nmore info: %s" diff --git a/internal/linters/go/gofmt/gofmt.go b/internal/linters/go/gofmt/gofmt.go index 32ecc382..8081cb2c 100644 --- a/internal/linters/go/gofmt/gofmt.go +++ b/internal/linters/go/gofmt/gofmt.go @@ -47,7 +47,7 @@ func gofmtHandler(ctx context.Context, a linters.Agent) error { // Since GitHub's check run feature does not have the suggestion functionality, GitHub PR review is fixed used to display gofmt reports. // Details: https://github.com/qiniu/reviewbot/issues/166 - a.LinterConfig.GithubReportFormat = config.GithubPRReview + a.LinterConfig.ReportFormat = config.GithubPRReview executor, err := NewgofmtExecutor(a.LinterConfig.WorkDir) if err != nil { diff --git a/internal/linters/linters_test.go b/internal/linters/linters_test.go index 734c238d..7b7d7591 100644 --- a/internal/linters/linters_test.go +++ b/internal/linters/linters_test.go @@ -189,10 +189,10 @@ func TestExecRun(t *testing.T) { id: "case1 - without ARTIFACT", input: Agent{ LinterConfig: config.Linter{ - Enable: &tp, - Command: []string{"/bin/bash", "-c", "--"}, - Args: []string{"echo file:line:column:message"}, - GithubReportFormat: config.Quiet, + Enable: &tp, + Command: []string{"/bin/bash", "-c", "--"}, + Args: []string{"echo file:line:column:message"}, + ReportFormat: config.Quiet, }, }, output: []byte("file:line:column:message\n"), @@ -202,10 +202,10 @@ func TestExecRun(t *testing.T) { id: "case2 - with ARTIFACT", input: Agent{ LinterConfig: config.Linter{ - Enable: &tp, - Command: []string{"/bin/bash", "-c", "--"}, - Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"}, - GithubReportFormat: config.Quiet, + Enable: &tp, + Command: []string{"/bin/bash", "-c", "--"}, + Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"}, + ReportFormat: config.Quiet, }, }, output: []byte("file2:6:7:message\n"), @@ -215,10 +215,10 @@ func TestExecRun(t *testing.T) { id: "case2 - with multi files under ARTIFACT", input: Agent{ LinterConfig: config.Linter{ - Enable: &tp, - Command: []string{"/bin/bash", "-c", "--"}, - Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1 ;echo file3:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"}, - GithubReportFormat: config.Quiet, + Enable: &tp, + Command: []string{"/bin/bash", "-c", "--"}, + Args: []string{"echo file2:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1 ;echo file3:6:7:message >> $ARTIFACT/golangci-lint.log 2>&1"}, + ReportFormat: config.Quiet, }, }, output: []byte("file2:6:7:message\nfile3:6:7:message\n"), diff --git a/internal/linters/providergithub.go b/internal/linters/providergithub.go index f414e734..b104c9a3 100644 --- a/internal/linters/providergithub.go +++ b/internal/linters/providergithub.go @@ -255,7 +255,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st num := a.Provider.GetCodeReviewInfo().Number orgRepo := fmt.Sprintf("%s/%s", org, repo) - switch a.LinterConfig.GithubReportFormat { + switch a.LinterConfig.ReportFormat { case config.GithubCheckRuns: ch, err := g.CreateGithubChecks(ctx, a, lintResults) if err != nil { @@ -319,7 +319,7 @@ func (g *GithubProvider) Report(ctx context.Context, a Agent, lintResults map[st case config.Quiet: return nil default: - log.Errorf("unsupported report format: %v", a.LinterConfig.GithubReportFormat) + log.Errorf("unsupported report format: %v", a.LinterConfig.ReportFormat) } return nil diff --git a/internal/linters/providergitlab.go b/internal/linters/providergitlab.go index c84efa0c..7a93d42b 100644 --- a/internal/linters/providergitlab.go +++ b/internal/linters/providergitlab.go @@ -195,6 +195,8 @@ func NewGitlabProvider(gitlabClient *gitlab.Client, gitClient gitv2.ClientFactor func ReportFormMatCheck(gc *gitlab.Client, reportFormat config.ReportType) (reportType config.ReportType) { // gitlab verion below 10.8 not support discussion resource api + // gitlab reportformart not config: version=>10.8: GitlabCommentAndDiscussion, version<10.8: GitlabComment + // gitlab reportformart config: version=>10.8: use config, version<10.8: GitlabComment v, r, e := gc.Version.GetVersion() if e != nil { log.Fatalf("Failed to get version: %v,response is %v", e, r) @@ -202,10 +204,16 @@ func ReportFormMatCheck(gc *gitlab.Client, reportFormat config.ReportType) (repo } v1, _ := version.NewVersion(v.Version) v2, _ := version.NewVersion("10.8") + if reportFormat != "" { + if v1.LessThan(v2) { + return config.GitlabComment + } + return reportFormat + } if v1.LessThan(v2) { return config.GitlabComment } - return reportFormat + return config.GitlabCommentAndDiscussion } func (g *GitlabProvider) HandleComments(ctx context.Context, outputs map[string][]LinterOutput) error { @@ -222,7 +230,7 @@ func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[st repo := a.Provider.GetCodeReviewInfo().Repo num := a.Provider.GetCodeReviewInfo().Number orgRepo := fmt.Sprintf("%s/%s", org, repo) - reportformat := ReportFormMatCheck(g.GitLabClient, a.LinterConfig.GitlabReportFormat) + reportformat := ReportFormMatCheck(g.GitLabClient, a.LinterConfig.ReportFormat) switch reportformat { case config.GitlabCommentAndDiscussion: // list MR comments @@ -317,7 +325,7 @@ func (g *GitlabProvider) Report(ctx context.Context, a Agent, lintResults map[st case config.Quiet: return nil default: - log.Errorf("unsupported report format: %v", a.LinterConfig.GitlabReportFormat) + log.Errorf("unsupported report format: %v", a.LinterConfig.ReportFormat) } return nil } @@ -335,9 +343,8 @@ func CreateGitLabCommentsReport(ctx context.Context, gc *gitlab.Client, outputs var errormessage string var tabletop10 string tabletop10 = "\n| FilePath | Line |ErrorMessage |\n" - tabletop10 = tabletop10 + "| -------- | -------- | -------- |\n" - var top10 int64 - top10 = 0 + tabletop10 += "| -------- | -------- | -------- |\n" + var top10 = 0 var totalerrorscount int totalerrorscount = 0 // for combine the linter result @@ -349,9 +356,8 @@ func CreateGitLabCommentsReport(ctx context.Context, gc *gitlab.Client, outputs if top10 < 10 { tabletop10 += "|" + outputmessage.File + "|" + strconv.Itoa(outputmessage.Line) + "|" + outputmessage.Message + "|\n" } - top10 += 1 + top10++ } - } message = fmt.Sprintf("[**%s**] check failed❌ , %v files exist errors,%v errors found. This is [the detailed log](%s).
Top 10 errors as below:\r\n"+tabletop10+"\n%s", lintername, len(outputs), totalerrorscount, logurl, comentDetailHeader+errormessage+"
"+commentDetail) } else { diff --git a/main.go b/main.go index 47378ab9..1bac8587 100644 --- a/main.go +++ b/main.go @@ -213,12 +213,7 @@ func main() { opt := gitv2.ClientFactoryOpts{ CacheDirBase: github.String(o.codeCacheDir), Persist: github.Bool(true), - UseSSH: github.Bool(false), - Host: o.gitLabHost, - Username: func() (string, error) { return "oauth2", nil }, - Token: func(v string) (string, error) { - return o.gitLabAccessToken, nil - }, + UseSSH: github.Bool(true), } v2, err := gitv2.NewClientFactory(opt.Apply) if err != nil { diff --git a/server.go b/server.go index 18649a62..6fa67f50 100644 --- a/server.go +++ b/server.go @@ -465,20 +465,37 @@ func (s *Server) gitlabHandle(ctx context.Context, event *gitlab.MergeEvent) err } }() - r, err := s.gitClientFactory.ClientForWithRepoOpts(org, repo, gitv2.RepoOpts{ + opt := gitv2.ClientFactoryOpts{ + CacheDirBase: github.String(s.repoCacheDir), + Persist: github.Bool(true), + UseSSH: github.Bool(false), + Host: s.gitLabHost, + Username: func() (string, error) { return "oauth2", nil }, + Token: func(v string) (string, error) { + return s.gitLabAccessToken, nil + }, + } + v2, errapp := gitv2.NewClientFactory(opt.Apply) + s.gitClientFactory = v2 + + r, errclone := s.gitClientFactory.ClientForWithRepoOpts(org, repo, gitv2.RepoOpts{ CopyTo: defaultWorkDir + "/" + repo, }) - if err != nil { + if errapp != nil { log.Errorf("failed to create git client: %v", err) - return err + return errapp } - if err := r.Checkout(latestCommit); err != nil { - log.Errorf("failed to checkout pull request %d: %v", num, err) - return err + if errclone != nil { + log.Errorf("failed to clone code repo : %v", err) + return errclone + } + if errcheckout := r.Checkout(latestCommit); err != nil { + log.Errorf("failed to checkout pull request %d: %v", num, errcheckout) + return errcheckout } gitModulesFile := path.Join(r.Directory(), ".gitmodules") - _, err = os.Stat(gitModulesFile) - if err == nil { + _, errpro := os.Stat(gitModulesFile) + if errpro == nil { log.Info("git pull submodule in progress") cmd := exec.Command("git", "submodule", "update", "--init", "--recursive") cmd.Dir = r.Directory() @@ -490,9 +507,9 @@ func (s *Server) gitlabHandle(ctx context.Context, event *gitlab.MergeEvent) err } else { log.Infof("no .gitmodules file in repo %s", repo) } - for name, fn := range linters.TotalPullRequestHandlers() { - linterConfig := s.config.GetLinterConfig(org, repo, name) + + linterConfig := s.config.GetLinterConfig(org, repo, name, config.GitLab) // skip linter if it is disabled if linterConfig.Enable != nil && !*linterConfig.Enable { @@ -506,21 +523,8 @@ func (s *Server) gitlabHandle(ctx context.Context, event *gitlab.MergeEvent) err } log.Infof("[%s] config on repo %v: %+v", name, orgRepo, linterConfig) - opt := gitv2.ClientFactoryOpts{ - CacheDirBase: github.String(s.repoCacheDir), - Persist: github.Bool(true), - UseSSH: github.Bool(false), - Host: s.gitLabHost, - Username: func() (string, error) { return "oauth2", nil }, - Token: func(v string) (string, error) { - return s.gitLabAccessToken, nil - }, - } - v2new, err := gitv2.NewClientFactory(opt.Apply) - if err != nil { - log.Fatalf("failed to create git client factory: %v", err) - } - gitlabProvider, err := linters.NewGitlabProvider(s.GitLabClient(), v2new, mergeRequestAffectedFiles, *event) + + gitlabProvider, err := linters.NewGitlabProvider(s.GitLabClient(), v2, mergeRequestAffectedFiles, *event) if err != nil { log.Errorf("failed to create provider: %v", err) @@ -634,7 +638,7 @@ func (s *Server) handle(ctx context.Context, event *github.PullRequestEvent) err }() for name, fn := range linters.TotalPullRequestHandlers() { - linterConfig := s.config.GetLinterConfig(org, repo, name) + linterConfig := s.config.GetLinterConfig(org, repo, name, config.GitHub) linterConfig.Number = num linterConfig.Workspace = workspace // skip linter if it is disabled