From f95a5bc1236859180dc04637f6f70e99d4ccb2c1 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 5 Mar 2019 13:44:26 -0500 Subject: [PATCH] refactor(git): checkout whole repository with worktree Signed-off-by: Jacob LeGrone --- Gopkg.lock | 9 ---- pkg/chart/chart.go | 92 ++++++++++++++++------------------------- pkg/chart/chart_test.go | 8 +--- pkg/tool/git.go | 18 ++------ 4 files changed, 41 insertions(+), 86 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 8b747eaa..36bd88dc 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -108,14 +108,6 @@ revision = "3536a929edddb9a5b34bd6861dc4a9647cb459fe" version = "v1.1.2" -[[projects]] - digest = "1:f55dbdd667c57775be87afc7e3921b6135ebea61350e098d92d6ee8542f44060" - name = "github.com/otiai10/copy" - packages = ["."] - pruneopts = "" - revision = "a15b9cb96cf2f7930107fe0a9a6faf1b91e69df4" - version = "v1.0.1" - [[projects]] digest = "1:894aef961c056b6d85d12bac890bf60c44e99b46292888bfa66caf529f804457" name = "github.com/pelletier/go-toml" @@ -261,7 +253,6 @@ "github.com/Masterminds/semver", "github.com/hashicorp/go-retryablehttp", "github.com/mitchellh/go-homedir", - "github.com/otiai10/copy", "github.com/pkg/errors", "github.com/spf13/cobra", "github.com/spf13/cobra/doc", diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index d3bdb84f..e1e30b42 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -25,7 +25,6 @@ import ( "github.com/helm/chart-testing/pkg/config" "github.com/helm/chart-testing/pkg/tool" "github.com/helm/chart-testing/pkg/util" - "github.com/otiai10/copy" "github.com/pkg/errors" ) @@ -35,13 +34,10 @@ import ( // // Show returns the contents of file on the specified remote/branch. // -// CheckoutDir replaces the contents of a directory with the contents of the same directory at -// the specified git ref. CheckoutDir does not switch branches. +// AddWorkingTree checks out the contents of the repository at a commit ref into the specified path. // -// CleanDir resets a directory to HEAD by recursively removing untracked directories and files. +// RemoveWorkingTree removes the working tree at the specified path. // -// IsDirClean returns true if there are no untracked changes since HEAD. -//. // MergeBase returns the SHA1 of the merge base of commit1 and commit2. // // ListChangedFilesInDirs diffs commit against HEAD and returns changed files for the specified dirs. @@ -53,9 +49,8 @@ import ( type Git interface { FileExistsOnBranch(file string, remote string, branch string) bool Show(file string, remote string, branch string) (string, error) - CheckoutDir(directory string, ref string) error - CleanDir(directory string) error - IsDirClean(directory string) (bool, error) + AddWorkingTree(path string, ref string) error + RemoveWorkingTree(path string) error MergeBase(commit1 string, commit2 string) (string, error) ListChangedFilesInDirs(commit string, dirs ...string) ([]string, error) GetUrlForRemote(remote string) (string, error) @@ -164,7 +159,6 @@ type Testing struct { accountValidator AccountValidator directoryLister DirectoryLister chartUtils ChartUtils - mergeBase string } // TestResults holds results and overall status @@ -195,16 +189,12 @@ func NewTesting(config config.Configuration) Testing { } } -// tempChartDir converts a chart parent directory path to a temp directory path -func tempChartParentDir(parentDir string) string { - return fmt.Sprintf("%s.chart-testing.tmp", parentDir) -} +const ctPreviousRevisionTree = "ct_previous_revision" -// tempChartPath converts a chart path to its corresponding temp path -func tempChartPath(chart string) string { - parentDir := path.Dir(chart) - chartName := path.Base(chart) - return path.Join(tempChartParentDir(parentDir), chartName) +// computePreviousRevisionPath converts any file or directory path to the same path in the +// previous revision's working tree. +func computePreviousRevisionPath(dir string) string { + return path.Join(ctPreviousRevisionTree, dir) } func (t *Testing) processCharts(action func(chart string, valuesFiles []string) TestResult) ([]TestResult, error) { @@ -216,19 +206,6 @@ func (t *Testing) processCharts(action func(chart string, valuesFiles []string) return results, nil } - if t.config.Upgrade { - // Validate that working directory is in a git repository - err := t.git.ValidateRepository() - if err != nil { - return results, fmt.Errorf("Must be in a git repository to test chart upgrades") - } - mergeBase, err := t.git.MergeBase(fmt.Sprintf("%s/%s", t.config.Remote, t.config.TargetBranch), "HEAD") - if err != nil { - return results, errors.Wrap(err, "Error identifying merge base") - } - t.mergeBase = mergeBase - } - fmt.Println() util.PrintDelimiterLine("-") fmt.Println(" Charts to be processed:") @@ -268,26 +245,20 @@ func (t *Testing) processCharts(action func(chart string, valuesFiles []string) TestResults: results, } + // Checkout previous chart revisions and build their dependencies if t.config.Upgrade { - changedParentDirs := map[string]bool{} - for _, dir := range charts { - changedParentDirs[path.Dir(dir)] = true + mergeBase, err := t.computeMergeBase() + if err != nil { + return results, errors.Wrap(err, "Error identifying merge base") } - for dir := range changedParentDirs { - // Check for uncommitted changes that would be lost when checking out older revision - clean, _ := t.git.IsDirClean(dir) - if !clean { - return results, fmt.Errorf("Directory %s has uncommitted changes", dir) + t.git.AddWorkingTree(ctPreviousRevisionTree, mergeBase) + defer t.git.RemoveWorkingTree(ctPreviousRevisionTree) + + for _, chart := range charts { + if err := t.helm.BuildDependencies(computePreviousRevisionPath(chart)); err != nil { + // Only print error (don't exit) if building dependencies for previous revision fails. + fmt.Println(errors.Wrap(err, fmt.Sprintf("Error building dependencies for previous revision of chart '%s'", chart))) } - // Set contents of charts directory to the target branch - t.git.CheckoutDir(dir, t.mergeBase) - // Copy charts directory contents to a temp directory - prevRevisionDir := tempChartParentDir(dir) - copy.Copy(dir, prevRevisionDir) - // Reset charts directory to last commit - t.git.CheckoutDir(dir, "HEAD") - // Schedule cleanup of untracked temp directory - defer t.git.CleanDir(prevRevisionDir) } } @@ -298,12 +269,6 @@ func (t *Testing) processCharts(action func(chart string, valuesFiles []string) return nil, errors.Wrapf(err, "Error building dependencies for chart '%s'", chart) } - if t.config.Upgrade { - if err := t.helm.BuildDependencies(tempChartPath(chart)); err != nil { - return nil, errors.Wrapf(err, "Error building dependencies for previous revision of chart '%s'", chart) - } - } - result := action(chart, valuesFiles) if result.Error != nil { testResults.OverallSuccess = false @@ -450,7 +415,7 @@ func (t *Testing) UpgradeChart(chart string) TestResult { fmt.Println(errors.Wrap(r, fmt.Sprintf("Skipping upgrade test of '%s' because", chart))) } } else { - result.Error = t.testUpgrade(tempChartPath(chart), chart, false) + result.Error = t.testUpgrade(computePreviousRevisionPath(chart), chart, false) } return result @@ -597,12 +562,25 @@ func (t *Testing) FindValuesFilesForCI(chart string) []string { return matches } +func (t *Testing) computeMergeBase() (string, error) { + err := t.git.ValidateRepository() + if err != nil { + return "", fmt.Errorf("Must be in a git repository") + } + return t.git.MergeBase(fmt.Sprintf("%s/%s", t.config.Remote, t.config.TargetBranch), "HEAD") +} + // ComputeChangedChartDirectories takes the merge base of HEAD and the configured remote and target branch and computes a // slice of changed charts from that in the configured chart directories excluding those configured to be excluded. func (t *Testing) ComputeChangedChartDirectories() ([]string, error) { cfg := t.config - allChangedChartFiles, err := t.git.ListChangedFilesInDirs(t.mergeBase, cfg.ChartDirs...) + mergeBase, err := t.computeMergeBase() + if err != nil { + return nil, err + } + + allChangedChartFiles, err := t.git.ListChangedFilesInDirs(mergeBase, cfg.ChartDirs...) if err != nil { return nil, errors.Wrap(err, "Error creating diff") } diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index cfb51cdf..c1f2ea3b 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -51,18 +51,14 @@ func (g fakeGit) ListChangedFilesInDirs(commit string, dirs ...string) ([]string }, nil } -func (g fakeGit) CheckoutDir(directory string, ref string) error { +func (g fakeGit) AddWorkingTree(path string, ref string) error { return nil } -func (g fakeGit) CleanDir(directory string) error { +func (g fakeGit) RemoveWorkingTree(path string) error { return nil } -func (g fakeGit) IsDirClean(directory string) (bool, error) { - return true, nil -} - func (g fakeGit) GetUrlForRemote(remote string) (string, error) { return "git@github.com/helm/chart-testing", nil } diff --git a/pkg/tool/git.go b/pkg/tool/git.go index cf13e293..e60c825b 100644 --- a/pkg/tool/git.go +++ b/pkg/tool/git.go @@ -38,22 +38,12 @@ func (g Git) FileExistsOnBranch(file string, remote string, branch string) bool return err == nil } -func (g Git) CheckoutDir(directory string, ref string) error { - _, err := g.exec.RunProcessAndCaptureOutput("git", "checkout", ref, "--", directory) - return err +func (g Git) AddWorkingTree(path string, ref string) error { + return g.exec.RunProcess("git", "worktree", "add", path, ref) } -func (g Git) CleanDir(directory string) error { - _, err := g.exec.RunProcessAndCaptureOutput("git", "clean", "-d", "--force", "--", directory) - return err -} - -func (g Git) IsDirClean(dir string) (bool, error) { - err := g.exec.RunProcess("git", "diff-index", "--quiet", "HEAD", "--", dir) - if err != nil { - return false, err - } - return true, nil +func (g Git) RemoveWorkingTree(path string) error { + return g.exec.RunProcess("git", "worktree", "remove", path) } func (g Git) Show(file string, remote string, branch string) (string, error) {