From 32fa4bd1b82b35c252ba950c1826f61a191e1cf7 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 15 Mar 2022 14:19:05 -0700 Subject: [PATCH 1/4] Hash correct file paths for global dependencies --- cli/internal/context/context.go | 7 ++++--- cli/internal/fs/package_deps_hash.go | 26 +++++++------------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/cli/internal/context/context.go b/cli/internal/context/context.go index d43baa2e6cb96..356d6adf93108 100644 --- a/cli/internal/context/context.go +++ b/cli/internal/context/context.go @@ -536,11 +536,12 @@ func calculateGlobalHash(rootpath string, rootPackageJSON *fs.PackageJSON, exter if !util.IsYarn(backend.Name) { // If we are not in Yarn, add the specfile and lockfile to global deps - globalDeps.Add(backend.Specfile) - globalDeps.Add(backend.Lockfile) + globalDeps.Add(filepath.Join(rootpath, backend.Specfile)) + globalDeps.Add(filepath.Join(rootpath, backend.Lockfile)) } - globalFileHashMap, err := fs.GitHashForFiles(globalDeps.UnsafeListOfStrings(), rootpath) + // No prefix, global deps already have full paths + globalFileHashMap, err := fs.GitHashForFiles(globalDeps.UnsafeListOfStrings()) if err != nil { return "", fmt.Errorf("error hashing files. make sure that git has been initialized %w", err) } diff --git a/cli/internal/fs/package_deps_hash.go b/cli/internal/fs/package_deps_hash.go index d788a1654b7c9..36b144ce38365 100644 --- a/cli/internal/fs/package_deps_hash.go +++ b/cli/internal/fs/package_deps_hash.go @@ -7,7 +7,6 @@ import ( "path/filepath" "regexp" "strings" - "github.com/vercel/turborepo/cli/internal/util" ) // Predefine []byte variables to avoid runtime allocations. @@ -53,23 +52,17 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { } currentlyChangedFiles := parseGitStatus(gitStatusOutput, p.PackagePath) var filesToHash []string - excludedPathsSet := new(util.Set) for filename, changeType := range currentlyChangedFiles { if changeType == "D" || (len(changeType) == 2 && string(changeType)[1] == []byte("D")[0]) { delete(result, filename) } else { - if !excludedPathsSet.Includes(filename) { - filesToHash = append(filesToHash, filename) - } + filesToHash = append(filesToHash, filepath.Join(p.PackagePath, filename)) } } // log.Printf("[TRACE] %v:", gitStatusOutput) // log.Printf("[TRACE] start GitHashForFiles") - current, err := GitHashForFiles( - filesToHash, - p.PackagePath, - ) + current, err := GitHashForFiles(filesToHash) if err != nil { return nil, fmt.Errorf("could not retrieve git hash for files in %s", p.PackagePath) } @@ -84,20 +77,15 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { } // GitHashForFiles a list of files returns a map of with their git hash values. It uses -// git hash-object under the -func GitHashForFiles(filesToHash []string, PackagePath string) (map[string]string, error) { +// git hash-object under the hood. +// Note that filesToHash must have full paths. +func GitHashForFiles(filesToHash []string) (map[string]string, error) { changes := make(map[string]string) if len(filesToHash) > 0 { - var input = []string{"hash-object"} - - for _, filename := range filesToHash { - input = append(input, filepath.Join(PackagePath, filename)) - } - // fmt.Println(input) + input := []string{"hash-object"} + input = append(input, filesToHash...) cmd := exec.Command("git", input...) // https://blog.kowalczyk.info/article/wOYk/advanced-command-execution-in-go-with-osexec.html - cmd.Stdin = strings.NewReader(strings.Join(input, "\n")) - cmd.Dir = PackagePath out, err := cmd.CombinedOutput() if err != nil { return nil, fmt.Errorf("git hash-object exited with status: %w", err) From 4e0afecd50ceae7f1bb6f33695629395dc073068 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 15 Mar 2022 16:11:46 -0700 Subject: [PATCH 2/4] Normalize paths for hashing --- cli/internal/context/context.go | 2 +- cli/internal/fs/package_deps_hash.go | 32 +++++++++++------- cli/internal/fs/package_deps_hash_test.go | 41 +++++++++++++++++++++++ 3 files changed, 62 insertions(+), 13 deletions(-) diff --git a/cli/internal/context/context.go b/cli/internal/context/context.go index 356d6adf93108..2be1d3f09d810 100644 --- a/cli/internal/context/context.go +++ b/cli/internal/context/context.go @@ -541,7 +541,7 @@ func calculateGlobalHash(rootpath string, rootPackageJSON *fs.PackageJSON, exter } // No prefix, global deps already have full paths - globalFileHashMap, err := fs.GitHashForFiles(globalDeps.UnsafeListOfStrings()) + globalFileHashMap, err := fs.GetHashableDeps(globalDeps.UnsafeListOfStrings(), rootpath) if err != nil { return "", fmt.Errorf("error hashing files. make sure that git has been initialized %w", err) } diff --git a/cli/internal/fs/package_deps_hash.go b/cli/internal/fs/package_deps_hash.go index 36b144ce38365..71801705b8233 100644 --- a/cli/internal/fs/package_deps_hash.go +++ b/cli/internal/fs/package_deps_hash.go @@ -7,6 +7,8 @@ import ( "path/filepath" "regexp" "strings" + + "github.com/pkg/errors" ) // Predefine []byte variables to avoid runtime allocations. @@ -59,27 +61,33 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { filesToHash = append(filesToHash, filepath.Join(p.PackagePath, filename)) } } + return GetHashableDeps(filesToHash, p.PackagePath) +} - // log.Printf("[TRACE] %v:", gitStatusOutput) - // log.Printf("[TRACE] start GitHashForFiles") - current, err := GitHashForFiles(filesToHash) +// GetHashableDeps hashes the list of given files, then returns a map of normalized path to hash +// this map is suitable for cross-platform caching. +func GetHashableDeps(absolutePaths []string, relativeTo string) (map[string]string, error) { + fileHashes, err := gitHashForFiles(absolutePaths) if err != nil { - return nil, fmt.Errorf("could not retrieve git hash for files in %s", p.PackagePath) + return nil, errors.Wrapf(err, "failed to hash files %v", strings.Join(absolutePaths, ", ")) } - // log.Printf("[TRACE] end GitHashForFiles") - // log.Printf("[TRACE] GitHashForFiles files %v", current) - for filename, hash := range current { - // log.Printf("[TRACE] GitHashForFiles files %v: %v", filename, hash) - result[filename] = hash + result := make(map[string]string) + for filename, hash := range fileHashes { + // Normalize path as POSIX-style and relative to "relativeTo" + relativePath, err := filepath.Rel(relativeTo, filename) + if err != nil { + return nil, errors.Wrapf(err, "failed to get relative path from %v to %v", relativeTo, relativePath) + } + key := filepath.ToSlash(relativePath) + result[key] = hash } - // log.Printf("[TRACE] GitHashForFiles result %v", result) return result, nil } -// GitHashForFiles a list of files returns a map of with their git hash values. It uses +// gitHashForFiles a list of files returns a map of with their git hash values. It uses // git hash-object under the hood. // Note that filesToHash must have full paths. -func GitHashForFiles(filesToHash []string) (map[string]string, error) { +func gitHashForFiles(filesToHash []string) (map[string]string, error) { changes := make(map[string]string) if len(filesToHash) > 0 { input := []string{"hash-object"} diff --git a/cli/internal/fs/package_deps_hash_test.go b/cli/internal/fs/package_deps_hash_test.go index be9e8ad39daa9..0e0673df6408d 100644 --- a/cli/internal/fs/package_deps_hash_test.go +++ b/cli/internal/fs/package_deps_hash_test.go @@ -1,6 +1,8 @@ package fs import ( + "os" + "path/filepath" "strings" "testing" @@ -83,3 +85,42 @@ R turbo.config.js -> turboooz.config.js ?? package_deps_hash_test.go` assert.EqualValues(t, want, parseGitStatus(input, "")) } + +func Test_GetHashableDeps(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd %v", err) + } + cliDir, err := filepath.Abs(filepath.Join(cwd, "..", "..")) + if err != nil { + t.Fatalf("failed to get cli dir: %v", err) + } + if filepath.Base(cliDir) != "cli" { + t.Fatalf("did not find cli dir, found %v", cliDir) + } + turboPath := filepath.Join(cliDir, "..", "turbo.json") + makefilePath := filepath.Join(cliDir, "Makefile") + mainPath := filepath.Join(cliDir, "cmd", "turbo", "main.go") + hashes, err := GetHashableDeps([]string{turboPath, makefilePath, mainPath}, cliDir) + if err != nil { + t.Fatalf("failed to hash files: %v", err) + } + // Note that the paths here are platform independent, so hardcoded slashes should be fine + expected := []string{ + "../turbo.json", + "Makefile", + "cmd/turbo/main.go", + } + for _, key := range expected { + if _, ok := hashes[key]; !ok { + t.Errorf("hashes missing %v", key) + } + } + if len(hashes) != len(expected) { + keys := []string{} + for key := range hashes { + keys = append(keys, key) + } + t.Errorf("hashes mismatch. got %v want %v", strings.Join(keys, ", "), strings.Join(expected, ", ")) + } +} From 7a1f07806764b061fd33f39ad2a3a2e0c8d4f9ff Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 15 Mar 2022 16:58:27 -0700 Subject: [PATCH 3/4] Add back extra files to be hashed that were dropped accidentally --- cli/internal/fs/package_deps_hash.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cli/internal/fs/package_deps_hash.go b/cli/internal/fs/package_deps_hash.go index 71801705b8233..c26bb871321b4 100644 --- a/cli/internal/fs/package_deps_hash.go +++ b/cli/internal/fs/package_deps_hash.go @@ -38,6 +38,7 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { return nil, fmt.Errorf("could not get git hashes for files in package %s: %w", p.PackagePath, err) } // Add all the checked in hashes. + // TODO(gsoltis): are these platform-dependent paths? result := parseGitLsTree(gitLsOutput) if len(p.ExcludedPaths) > 0 { @@ -61,7 +62,14 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { filesToHash = append(filesToHash, filepath.Join(p.PackagePath, filename)) } } - return GetHashableDeps(filesToHash, p.PackagePath) + hashes, err := GetHashableDeps(filesToHash, p.PackagePath) + if err != nil { + return nil, err + } + for key, hash := range hashes { + result[key] = hash + } + return result, nil } // GetHashableDeps hashes the list of given files, then returns a map of normalized path to hash From d75f7054336954c88a1c00ba971b2ba5845a8991 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Wed, 16 Mar 2022 09:37:01 -0700 Subject: [PATCH 4/4] Normalize paths coming from ls tree --- cli/internal/fs/package_deps_hash.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cli/internal/fs/package_deps_hash.go b/cli/internal/fs/package_deps_hash.go index c26bb871321b4..a8c75fa109019 100644 --- a/cli/internal/fs/package_deps_hash.go +++ b/cli/internal/fs/package_deps_hash.go @@ -62,14 +62,20 @@ func GetPackageDeps(p *PackageDepsOptions) (map[string]string, error) { filesToHash = append(filesToHash, filepath.Join(p.PackagePath, filename)) } } + normalized := make(map[string]string) + // These paths are platform-dependent, but already relative to the package root + for platformSpecificPath, hash := range result { + platformIndependentPath := filepath.ToSlash(platformSpecificPath) + normalized[platformIndependentPath] = hash + } hashes, err := GetHashableDeps(filesToHash, p.PackagePath) if err != nil { return nil, err } - for key, hash := range hashes { - result[key] = hash + for platformIndependentPath, hash := range hashes { + normalized[platformIndependentPath] = hash } - return result, nil + return normalized, nil } // GetHashableDeps hashes the list of given files, then returns a map of normalized path to hash