Merge pull request #14 from bvobart/git-improvements

Git Improvements

api/project.go

@@ -11,6 +11,8 @@ import (
 type Project struct {
 	// The project's assumed root directory, absolute path.
 	Dir string
+	// Information about the project's Git repository.
+	Git GitInfo
 	// mllint's configuration for this project
 	Config config.Config
 	// Type of mllint configuration
@@ -23,6 +25,18 @@ type Project struct {
 	PythonFiles utils.Filenames
 }
 
+// GitInfo describes some info about the Git repository that a project is in.
+type GitInfo struct {
+	// the URL of the Git remote, e.g. `[email protected]:bvobart/mllint.git`
+	RemoteURL string
+	// the hash of the current commit.
+	Commit string
+	// the name of the current branch.
+	Branch string
+	// whether the repository is currently in a dirty state (i.e. files added / removed / changed)
+	Dirty bool
+}
+
 // ProjectReport is what you end up with after mllint finishes analysing a project.
 type ProjectReport struct {
 	Project

commands/describe.go

@@ -2,7 +2,6 @@ package commands
 
 import (
 	"fmt"
-	"io/ioutil"
 	"strings"
 
 	"github.com/fatih/color"
@@ -11,7 +10,6 @@ import (
 	"github.com/bvobart/mllint/api"
 	"github.com/bvobart/mllint/categories"
 	"github.com/bvobart/mllint/linters"
-	"github.com/bvobart/mllint/utils"
 	"github.com/bvobart/mllint/utils/markdown"
 	"github.com/bvobart/mllint/utils/markdowngen"
 )
@@ -64,13 +62,7 @@ func describe(cmd *cobra.Command, args []string) error {
 	}
 
 	if outputToFile() {
-		if err := ioutil.WriteFile(outputFile, []byte(output.String()), 0644); err != nil {
-			return fmt.Errorf("failed to write output file: %w", err)
-		}
-		bold := color.New(color.Bold)
-		shush(func() { bold.Println("Your report is complete, see", formatInlineCode(utils.AbsolutePath(outputFile))) })
-		shush(func() { bold.Println() })
-		return nil
+		return writeToOutputFile(output.String())
 	}
 
 	if outputToStdout() {

commands/list.go

@@ -2,14 +2,12 @@ package commands
 
 import (
 	"fmt"
-	"io/ioutil"
 
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 
 	"github.com/bvobart/mllint/api"
 	"github.com/bvobart/mllint/linters"
-	"github.com/bvobart/mllint/utils"
 	"github.com/bvobart/mllint/utils/markdowngen"
 )
 
@@ -100,13 +98,7 @@ func listLinters(linters map[api.Category]api.Linter) error {
 		md := markdowngen.LintersOverview(linters)
 
 		if outputToFile() {
-			if err := ioutil.WriteFile(outputFile, []byte(md), 0644); err != nil {
-				return fmt.Errorf("failed to write output file: %w", err)
-			}
-			bold := color.New(color.Bold)
-			shush(func() { bold.Println("Your report is complete, see", formatInlineCode(utils.AbsolutePath(outputFile))) })
-			shush(func() { bold.Println() })
-			return nil
+			return writeToOutputFile(md)
 		}
 
 		if outputToStdout() {

commands/run.go

@@ -3,7 +3,6 @@ package commands
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 
 	"github.com/fatih/color"
 	"github.com/hashicorp/go-multierror"
@@ -16,6 +15,7 @@ import (
 	"github.com/bvobart/mllint/linters"
 	"github.com/bvobart/mllint/setools/cqlinters"
 	"github.com/bvobart/mllint/setools/depmanagers"
+	"github.com/bvobart/mllint/setools/git"
 	"github.com/bvobart/mllint/utils"
 	"github.com/bvobart/mllint/utils/markdown"
 )
@@ -47,10 +47,12 @@ type runCommand struct {
 }
 
 // Runs pre-analysis checks:
+// - Retrieve some info about project's Git state
 // - Detect dependency managers used in the project
 // - Detect code quality linters used in the project
 // - Detect the Python files in the project repository.
 func (rc *runCommand) runPreAnalysisChecks() error {
+	rc.ProjectR.Git = git.MakeGitInfo(rc.ProjectR.Dir)
 	rc.ProjectR.DepManagers = depmanagers.Detect(rc.ProjectR.Project)
 	rc.ProjectR.CQLinters = cqlinters.Detect(rc.ProjectR.Project)
 
@@ -115,12 +117,7 @@ func (rc *runCommand) RunLint(cmd *cobra.Command, args []string) error {
 	}
 
 	if outputToFile() {
-		if err := ioutil.WriteFile(outputFile, []byte(output), 0644); err != nil {
-			return fmt.Errorf("failed to write output file: %w", err)
-		}
-		bold := color.New(color.Bold)
-		shush(func() { bold.Println("Your report is complete, see", formatInlineCode(utils.AbsolutePath(outputFile))) })
-		shush(func() { bold.Println() })
+		return writeToOutputFile(output)
 	} else {
 		fmt.Println(markdown.Render(output))
 	}

commands/util.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path"
 	"strings"
@@ -36,6 +37,24 @@ func formatInlineCode(text string) string {
 	return color.New(color.Reset, color.Italic, color.FgYellow).Sprint(text)
 }
 
+func writeToOutputFile(output string) error {
+	// create output folder if it doesn't exist
+	if !utils.FolderExists(path.Dir(outputFile)) {
+		if err := os.MkdirAll(path.Dir(outputFile), 0755); err != nil {
+			return fmt.Errorf("failed to create output directory: %w", err)
+		}
+	}
+
+	if err := ioutil.WriteFile(outputFile, []byte(output), 0644); err != nil {
+		return fmt.Errorf("failed to write output file: %w", err)
+	}
+
+	bold := color.New(color.Bold)
+	shush(func() { bold.Println("Your report is complete, see", formatInlineCode(utils.AbsolutePath(outputFile))) })
+	shush(func() { bold.Println() })
+	return nil
+}
+
 func prettyPrintLinters(linters map[api.Category]api.Linter) {
 	if len(linters) == 0 {
 		color.Red("Oh no! Your mllint configuration has disabled ALL rules!")

linters/ci/linter.go

@@ -32,7 +32,7 @@ func (l *CILinter) LintProject(project api.Project) (api.Report, error) {
 	for _, provider := range providers {
 		// if the repo is not tracking the CI config file, then they're not really using CI,
 		// they're merely trying to define it, which is at least a step in the right direction.
-		if !git.IsTracking(project.Dir, provider.ConfigFile()) {
+		if !git.IsTracking(project.Dir, provider.ConfigFile(project.Dir)) {
 			report.Scores[RuleUseCI] = 25
 		}
 	}

linters/ci/linter_test.go

@@ -11,15 +11,29 @@ import (
 	"github.com/bvobart/mllint/api"
 	"github.com/bvobart/mllint/linters/ci"
 	"github.com/bvobart/mllint/setools/ciproviders"
+	"github.com/bvobart/mllint/utils/exec"
 )
 
+const tmpkey = "mllint-tests-ci"
+
+func createTestGitDir(t *testing.T, name string) string {
+	dir, err := ioutil.TempDir(os.TempDir(), tmpkey+"-"+name)
+	require.NoError(t, err)
+	_, err = exec.CommandOutput(dir, "git", "init")
+	require.NoError(t, err)
+	return dir
+}
+
 func TestCILinter(t *testing.T) {
 	linter := ci.NewLinter()
 	require.Equal(t, "Continuous Integration (CI)", linter.Name())
 	require.Equal(t, []*api.Rule{&ci.RuleUseCI}, linter.Rules())
 
 	t.Run("None", func(t *testing.T) {
-		project := api.Project{Dir: "test-resources/none"}
+		dir := createTestGitDir(t, "")
+		project := api.Project{Dir: dir}
+		defer os.RemoveAll(dir)
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 
@@ -29,40 +43,69 @@ func TestCILinter(t *testing.T) {
 	})
 
 	t.Run("Azure", func(t *testing.T) {
-		project := api.Project{Dir: "test-resources/azure"}
+		dir := createTestGitDir(t, "azure")
+		project := api.Project{Dir: dir}
+		defer os.RemoveAll(dir)
+
+		require.NoError(t, ioutil.WriteFile(ciproviders.Azure{}.ConfigFile(dir), []byte("\n"), 0644))
+		_, err := exec.CommandOutput(dir, "git", "add", ".")
+		require.NoError(t, err)
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 		require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
 	})
 
 	t.Run("GitHubActions", func(t *testing.T) {
-		project := api.Project{Dir: "test-resources/ghactions"}
+		dir := createTestGitDir(t, "ghactions")
+		project := api.Project{Dir: dir}
+		defer os.RemoveAll(dir)
+
+		require.NoError(t, os.MkdirAll(ciproviders.GHActions{}.ConfigFile(dir), 0755))
+		require.NoError(t, ioutil.WriteFile(path.Join(ciproviders.GHActions{}.ConfigFile(dir), "workflow.yml"), []byte("\n"), 0644))
+		_, err := exec.CommandOutput(dir, "git", "add", ".")
+		require.NoError(t, err)
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 		require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
 	})
 
 	t.Run("GitlabCI", func(t *testing.T) {
-		project := api.Project{Dir: "test-resources/gitlab"}
+		dir := createTestGitDir(t, "gitlab")
+		project := api.Project{Dir: dir}
+		defer os.RemoveAll(dir)
+
+		require.NoError(t, ioutil.WriteFile(ciproviders.Gitlab{}.ConfigFile(dir), []byte("\n"), 0644))
+		_, err := exec.CommandOutput(dir, "git", "add", ".")
+		require.NoError(t, err)
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 		require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
 	})
 
 	t.Run("Travis", func(t *testing.T) {
-		project := api.Project{Dir: "test-resources/travis"}
+		dir := createTestGitDir(t, "travis")
+		project := api.Project{Dir: dir}
+		defer os.RemoveAll(dir)
+
+		require.NoError(t, ioutil.WriteFile(ciproviders.Travis{}.ConfigFile(dir), []byte("\n"), 0644))
+		_, err := exec.CommandOutput(dir, "git", "add", ".")
+		require.NoError(t, err)
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 		require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
 	})
 
 	t.Run("GitlabUntracked", func(t *testing.T) {
-		dir, err := ioutil.TempDir("test-resources", "gitlab-untracked")
-		require.NoError(t, err)
-		require.NoError(t, ioutil.WriteFile(path.Join(dir, ciproviders.Gitlab{}.ConfigFile()), []byte("\n"), 0644))
+		dir := createTestGitDir(t, "gitlab-untracked")
+		project := api.Project{Dir: dir}
 		defer os.RemoveAll(dir)
 
-		project := api.Project{Dir: dir}
+		require.NoError(t, ioutil.WriteFile(ciproviders.Gitlab{}.ConfigFile(dir), []byte("\n"), 0644))
+
 		report, err := linter.LintProject(project)
 		require.NoError(t, err)
 		require.EqualValues(t, 25, report.Scores[ci.RuleUseCI])

linters/versioncontrol/git.go

@@ -40,7 +40,7 @@ func (l *GitLinter) LintProject(project api.Project) (api.Report, error) {
 		return report, nil
 	}
 
-	largeFiles, err := git.FindLargeFiles(project.Dir, l.MaxFileSize)
+	largeFiles, err := git.FindLargeFilesInHistory(project.Dir, l.MaxFileSize)
 	if err != nil {
 		return api.Report{}, err
 	}
@@ -55,9 +55,14 @@ func (l *GitLinter) LintProject(project api.Project) (api.Report, error) {
 
 func (l *GitLinter) buildDetails(largeFiles []git.FileSize) string {
 	details := strings.Builder{}
-	details.WriteString(fmt.Sprintf("Your project is tracking the following files that are larger than %s:\n", humanize.Bytes(l.MaxFileSize)))
+	details.WriteString(fmt.Sprintf("Your project's Git history contains the following files that are larger than %s:\n", humanize.Bytes(l.MaxFileSize)))
 	for _, file := range largeFiles {
-		details.WriteString(fmt.Sprintf("- **%s**  %s\n", humanize.Bytes(file.Size), file.Path))
+		details.WriteString(fmt.Sprintf("- **%s** - commit `%s` - `%s`\n", humanize.Bytes(file.Size), file.CommitHash, file.Path))
 	}
+	details.WriteString(`
+These files may not necessarily be in your project right now, but they are still stored inside your project's Git history.
+Thus, whenever your project is downloaded (with ` + "`git clone`" + `), all these unnecessary files have to be downloaded as well.
+
+See [this StackOverflow answer](https://stackoverflow.com/a/46615578/8059181) to learn how to remove these files from your project's Git history.`)
 	return details.String()
 }

linters/versioncontrol/rules.go

@@ -24,7 +24,7 @@ To start using Git, run %s in a terminal at the root of your project. See also [
 // See https://docs.github.com/en/github/managing-large-files/what-is-my-disk-quota
 var RuleGitNoBigFiles = api.Rule{
 	Slug: "version-control/code/git-no-big-files",
-	Name: "Project should not use Git to track large files",
+	Name: "Project should not have any large files in its Git history",
 	Details: `Git is great for version controlling small, textual files, but not for binary or large files.
 Tracking large files directly with Git adds bloat to your repository's Git history, which needs to be downloaded every time your project is cloned.
 Large files should instead be version controlled as Data, e.g. using Git LFS or DVC.

setools/ciproviders/azure.go

@@ -3,19 +3,20 @@ package ciproviders
 import (
 	"path"
 
+	"github.com/bvobart/mllint/setools/git"
 	"github.com/bvobart/mllint/utils"
 )
 
 const azureFile = "azure-pipelines.yml"
 
 type Azure struct{}
 
-func (_ Azure) ConfigFile() string {
-	return azureFile
+func (_ Azure) ConfigFile(projectdir string) string {
+	return path.Join(git.GetGitRoot(projectdir), azureFile)
 }
 
 func (_ Azure) Detect(projectdir string) bool {
-	return utils.FileExists(path.Join(projectdir, azureFile))
+	return utils.FileExists(path.Join(git.GetGitRoot(projectdir), azureFile))
 }
 
 func (_ Azure) Type() ProviderType {

setools/ciproviders/ghactions.go

@@ -3,29 +3,26 @@ package ciproviders
 import (
 	"path"
 
+	"github.com/bvobart/mllint/setools/git"
 	"github.com/bvobart/mllint/utils"
 )
 
 const ghactionsFolder = ".github/workflows"
 
 type GHActions struct{}
 
-func (_ GHActions) ConfigFile() string {
-	return ghactionsFolder
+func (_ GHActions) ConfigFile(projectdir string) string {
+	return path.Join(git.GetGitRoot(projectdir), ghactionsFolder)
 }
 
 func (_ GHActions) Detect(projectdir string) bool {
-	workflowsdir := path.Join(projectdir, ghactionsFolder)
+	workflowsdir := path.Join(git.GetGitRoot(projectdir), ghactionsFolder)
 	if !utils.FolderExists(workflowsdir) {
 		return false
 	}
 
 	isEmpty, err := utils.FolderIsEmpty(workflowsdir)
-	if err != nil {
-		return false
-	}
-
-	return !isEmpty
+	return err == nil && !isEmpty
 }
 
 func (_ GHActions) Type() ProviderType {