Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use filename in cache key to prevent collisions under rename #3203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/executor/composite_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ func hashDir(p string, context util.FileContext) (bool, string, error) {
if err != nil {
return err
}

absPath, err := filepath.Abs(path)
if err != nil {
return err
}

absRoot, err := filepath.Abs(context.Root)
if err != nil {
return err
}

if _, err := sha.Write([]byte(strings.TrimPrefix(absPath, absRoot))); err != nil {
return err
}

if _, err := sha.Write([]byte(fileHash)); err != nil {
return err
}
Expand Down
87 changes: 83 additions & 4 deletions pkg/executor/composite_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func Test_CompositeCache_AddPath_dir(t *testing.T) {
t.Errorf("expected hash %v to equal hash %v", hash1, hash2)
}
}

func Test_CompositeCache_AddPath_file(t *testing.T) {
tmpfile, err := os.CreateTemp("/tmp", "foo.txt")
if err != nil {
Expand Down Expand Up @@ -206,8 +207,6 @@ func Test_CompositeKey_AddPath_Works(t *testing.T) {
},
}

fileContext := util.FileContext{}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
testDir1 := t.TempDir()
Expand All @@ -222,11 +221,11 @@ func Test_CompositeKey_AddPath_Works(t *testing.T) {
t.Fatalf("Error creating filesytem structure: %s", err)
}

hash1, err := hashDirectory(testDir1, fileContext)
hash1, err := hashDirectory(testDir1, util.FileContext{Root: testDir1})
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
}
hash2, err := hashDirectory(testDir2, fileContext)
hash2, err := hashDirectory(testDir2, util.FileContext{Root: testDir2})
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
}
Expand Down Expand Up @@ -435,10 +434,13 @@ func Test_CompositeKey_AddPath_WithExtraFilIgnored_Works(t *testing.T) {
t.Fatalf("Error creating filesytem structure: %s", err)
}

fileContext.Root = testDir1
hash1, err := hashDirectory(testDir1, fileContext)
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
}

fileContext.Root = testDir2
hash2, err := hashDirectory(testDir2, fileContext)
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
Expand Down Expand Up @@ -508,10 +510,13 @@ func Test_CompositeKey_AddPath_WithExtraDirIgnored_Works(t *testing.T) {
t.Fatalf("Error creating filesytem structure: %s", err)
}

fileContext.Root = testDir1
hash1, err := hashDirectory(testDir1, fileContext)
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
}

fileContext.Root = testDir2
hash2, err := hashDirectory(testDir2, fileContext)
if err != nil {
t.Fatalf("Failed to calculate hash: %s", err)
Expand All @@ -523,3 +528,77 @@ func Test_CompositeKey_AddPath_WithExtraDirIgnored_Works(t *testing.T) {
})
}
}

func Test_CompositeCache_AddPath_DiffFileNames_Cache_Differently_Works(t *testing.T) {
tmpDir1 := t.TempDir()
tmpDir2 := t.TempDir()

content := `meow meow meow`
if err := os.WriteFile(filepath.Join(tmpDir1, "foo.txt"), []byte(content), 0777); err != nil {
t.Errorf("got error writing temp file %v", err)
}

if err := os.WriteFile(filepath.Join(tmpDir2, "bar.txt"), []byte(content), 0777); err != nil {
t.Errorf("got error writing temp file %v", err)
}

fn := func(p string) string {
r := NewCompositeCache()
if err := r.AddPath(p, util.FileContext{Root: p}); err != nil {
t.Errorf("expected error to be nil but was %v", err)
}

if len(r.keys) != 1 {
t.Errorf("expected len of keys to be 1 but was %v", len(r.keys))
}

hash, err := r.Hash()
if err != nil {
t.Errorf("couldnt generate hash from test cache")
}
return hash
}

hash1 := fn(tmpDir1)
hash2 := fn(tmpDir2)
if hash1 == hash2 {
t.Errorf("expected hash %v to be different than hash %v", hash1, hash2)
}
}

func Test_CompositeCache_AddPath_SameFileNames_In_Diff_Contexts_Works(t *testing.T) {
tmpDir1 := t.TempDir()
tmpDir2 := t.TempDir()

content := `meow meow meow`
if err := os.WriteFile(filepath.Join(tmpDir1, "foo.txt"), []byte(content), 0777); err != nil {
t.Errorf("got error writing temp file %v", err)
}

if err := os.WriteFile(filepath.Join(tmpDir2, "foo.txt"), []byte(content), 0777); err != nil {
t.Errorf("got error writing temp file %v", err)
}

fn := func(p string) string {
r := NewCompositeCache()
if err := r.AddPath(p, util.FileContext{Root: p}); err != nil {
t.Errorf("expected error to be nil but was %v", err)
}

if len(r.keys) != 1 {
t.Errorf("expected len of keys to be 1 but was %v", len(r.keys))
}

hash, err := r.Hash()
if err != nil {
t.Errorf("couldnt generate hash from test cache")
}
return hash
}

hash1 := fn(tmpDir1)
hash2 := fn(tmpDir2)
if hash1 != hash2 {
t.Errorf("expected hash %v to be the same as hash %v", hash1, hash2)
}
}
Loading