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

Fixes and tests for ingredientcall end-to-end #3575

Merged
merged 43 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
4eb18a8
Add doublestar support for globbing and fix inconsistencies in hash c…
Naatan Oct 31, 2024
b1af420
Add CommonParentPath function
Naatan Oct 31, 2024
8472bd9
Add GetwdUnsafe
Naatan Oct 31, 2024
bb3304b
`artifacts dl` should fail if artifact status is not successful
Naatan Oct 31, 2024
40b99ef
Add structured output for `artifacts dl`
Naatan Oct 31, 2024
9086810
model.BuildPlannerVersionConstraintsToString > model.VersionRequireme…
Naatan Oct 31, 2024
ad3aa7b
Added sliceutils.Cast
Naatan Oct 31, 2024
ba97d3c
Automatically set Path according to name/namespace
Naatan Oct 31, 2024
a0f86f5
Fix hashglobs variable notation
Naatan Oct 31, 2024
1cb3d36
Fix org namespace values
Naatan Oct 31, 2024
da63e3d
Missing import
Naatan Oct 31, 2024
312771b
Added archiver package
Naatan Oct 31, 2024
908b685
Buildscript external values support ints and slices
Naatan Oct 31, 2024
209ced7
Fix successful request throwing error
Naatan Oct 31, 2024
57c7997
Various fixes to ingredientcall during end to end testing
Naatan Oct 31, 2024
18545b2
Test ingredientcall
Naatan Oct 31, 2024
b810194
Add comments
Naatan Oct 31, 2024
1273d0b
Merge remote-tracking branch 'origin/version/0-47-0-RC1' into DX-3105
Naatan Oct 31, 2024
104a522
Compatibility fixes
Naatan Oct 31, 2024
32bda03
Disable test for now
Naatan Oct 31, 2024
69236e0
Drop old test
Naatan Oct 31, 2024
71d8071
Remove unused mutex
Naatan Nov 1, 2024
dae0380
Enable ingredientcall integration test
Naatan Nov 1, 2024
4546624
More test cases and support windows paths
Naatan Nov 1, 2024
7e2ba42
Clean up parent node logic
Naatan Nov 1, 2024
f4b3170
Fix unit test failures
Naatan Nov 5, 2024
fa24657
Update hashes in test
Naatan Nov 5, 2024
d0d975c
Add log info
Naatan Nov 5, 2024
96baec2
Also handle requirements in solve_legacy
Naatan Nov 6, 2024
332823c
Add debug info
Naatan Nov 6, 2024
ba3e9a5
Ensure function call hash is consistently calculated
Naatan Nov 6, 2024
774120b
Increase solving timeout
Naatan Nov 6, 2024
864af32
Add debug info
Naatan Nov 6, 2024
445dbe4
Give windows more time
Naatan Nov 6, 2024
67c2110
Ensure filepaths are using forward slash
Naatan Nov 7, 2024
6c38c9e
Drop redundant calls
Naatan Nov 7, 2024
f380d3d
Use relative paths for archives
Naatan Nov 7, 2024
c1fc008
Add comment around absolute paths
Naatan Nov 8, 2024
c20a2fb
Ensure we use the correct atTime when ingredient was cached at a newe…
Naatan Nov 8, 2024
543089c
Fix time comparison issues with buildscripts due to inconsistent time…
Naatan Nov 8, 2024
bceec93
Fix test
Naatan Nov 12, 2024
5d70309
Fix unit test
Naatan Nov 12, 2024
41062d2
Fix TestRealWorld
Naatan Nov 12, 2024
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
61 changes: 36 additions & 25 deletions cmd/state-svc/internal/hash/file_hasher.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"time"

"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/rtutils"
"github.com/bmatcuk/doublestar/v4"
"github.com/cespare/xxhash"
"github.com/patrickmn/go-cache"
)
Expand All @@ -36,64 +36,75 @@ func NewFileHasher() *FileHasher {
}

func (fh *FileHasher) HashFiles(wd string, globs []string) (_ string, _ []hashedFile, rerr error) {
sort.Strings(globs) // ensure consistent ordering
fs := os.DirFS(wd)
hashedFiles := []hashedFile{}
hasher := xxhash.New()
hashes := []string{}
for _, glob := range globs {
files, err := filepath.Glob(glob)
files, err := doublestar.Glob(fs, glob)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filepath.Glob() does not support double star globs (ie. */**) nor specifying the working directory. This library addresses both limitations.

if err != nil {
return "", nil, errs.Wrap(err, "Could not match glob: %s", glob)
}
sort.Strings(files) // ensure consistent ordering
for _, f := range files {
if !filepath.IsAbs(f) {
af, err := filepath.Abs(filepath.Join(wd, f))
if err != nil {
return "", nil, errs.Wrap(err, "Could not get absolute path for file: %s", f)
}
f = af
for _, relativePath := range files {
absolutePath, err := filepath.Abs(filepath.Join(wd, relativePath))
if err != nil {
return "", nil, errs.Wrap(err, "Could not get absolute path for file: %s", relativePath)
}
file, err := os.Open(f)
fileInfo, err := os.Stat(absolutePath)
if err != nil {
return "", nil, errs.Wrap(err, "Could not open file: %s", file.Name())
return "", nil, errs.Wrap(err, "Could not stat file: %s", absolutePath)
}
defer rtutils.Closer(file.Close, &rerr)

fileInfo, err := file.Stat()
if err != nil {
return "", nil, errs.Wrap(err, "Could not stat file: %s", file.Name())
if fileInfo.IsDir() {
continue
}

var hash string
cachedHash, ok := fh.cache.Get(cacheKey(file.Name(), fileInfo.ModTime()))
cachedHash, ok := fh.cache.Get(cacheKey(fileInfo.Name(), fileInfo.ModTime()))
if ok {
hash, ok = cachedHash.(string)
if !ok {
return "", nil, errs.New("Could not convert cache value to string")
}
} else {
fileHasher := xxhash.New()
// include filepath in hash, because moving files should affect the hash
fmt.Fprintf(fileHasher, "%016x", relativePath)
file, err := os.Open(absolutePath)
if err != nil {
return "", nil, errs.Wrap(err, "Could not open file: %s", absolutePath)
}
defer file.Close()
if _, err := io.Copy(fileHasher, file); err != nil {
return "", nil, errs.Wrap(err, "Could not hash file: %s", file.Name())
return "", nil, errs.Wrap(err, "Could not hash file: %s", fileInfo.Name())
}

hash = fmt.Sprintf("%016x", fileHasher.Sum64())
}

fh.cache.Set(cacheKey(file.Name(), fileInfo.ModTime()), hash, cache.NoExpiration)
fh.cache.Set(cacheKey(fileInfo.Name(), fileInfo.ModTime()), hash, cache.NoExpiration)

hashes = append(hashes, hash)
hashedFiles = append(hashedFiles, hashedFile{
Pattern: glob,
Path: file.Name(),
Path: relativePath,
Hash: hash,
})

// Incorporate the individual file hash into the overall hash in hex format
fmt.Fprintf(hasher, "%016x", hash)
}
}

return fmt.Sprintf("%016x", hasher.Sum64()), hashedFiles, nil
if hashedFiles == nil {
return "", nil, nil
}

// Ensure the overall hash is consistently calculated
sort.Slice(hashedFiles, func(i, j int) bool { return hashedFiles[i].Path < hashedFiles[j].Path })
h := xxhash.New()
for _, f := range hashedFiles {
fmt.Fprintf(h, "%016x", f.Hash)
}
Comment on lines +100 to +105
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to calculate the overall hash based on a sorted slice of all the individual hashes for all files. This way we get the same hash even if the files are sorted differently, or more importantly (what I ran into); if the patterns are different but produce the same result.


return fmt.Sprintf("%016x", h.Sum64()), hashedFiles, nil
}

func cacheKey(file string, modTime time.Time) string {
Expand Down
114 changes: 70 additions & 44 deletions cmd/state-svc/internal/hash/file_hasher_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
package hash

import (
"fmt"
"os"
"path/filepath"
"sort"
"strings"
"testing"
"time"

"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/fileutils"
"github.com/ActiveState/cli/internal/osutils"
"github.com/patrickmn/go-cache"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type testCache struct {
Expand All @@ -31,23 +39,39 @@ func (tc *testCache) Set(key string, value interface{}, expiration time.Duration
}

func TestFileHasher_HashFiles(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
dir := fileutils.TempDirUnsafe()
file1 := createTempFile(t, dir, "file1.txt")
file2 := createTempFile(t, dir, "file2.info")
subfile1 := createTempFile(t, dir, "dir1/subfile1.txt")

hasher := NewFileHasher()

hash1, err := hasher.HashFiles([]string{file1, file2})
assert.NoError(t, err)
hash1, files1, err := hasher.HashFiles(dir, []string{file1, file2, subfile1})
require.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
assert.NoError(t, err)
hash2, files2, err := hasher.HashFiles(dir, []string{"./**/*"})
require.NoError(t, err, errs.JoinMessage(err))

for _, f := range files1 {
assert.False(t, strings.HasPrefix(f.Path, dir), fmt.Sprintf("'%s' should not be prefixed with '%s'", f.Path, dir))
}

sort.Slice(files1, func(i, j int) bool { return files1[i].Path < files1[j].Path })
sort.Slice(files2, func(i, j int) bool { return files2[i].Path < files2[j].Path })
require.Len(t, files2, 3)
require.Len(t, files2, len(files1))

for i, f := range files1 {
assert.Equal(t, f.Path, files2[i].Path)
assert.Equal(t, f.Hash, files2[i].Hash)
}

assert.Equal(t, hash1, hash2)
}

func TestFileHasher_CacheHit(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
file1 := createTempFile(t, "", "file1")
file2 := createTempFile(t, "", "file2")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -57,10 +81,10 @@ func TestFileHasher_CacheHit(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.Equal(t, hash1, hash2)
Expand All @@ -69,8 +93,8 @@ func TestFileHasher_CacheHit(t *testing.T) {
}

func TestFileHasher_CacheMiss(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
file1 := createTempFile(t, "", "file1")
file2 := createTempFile(t, "", "file2")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -80,7 +104,7 @@ func TestFileHasher_CacheMiss(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

if err := os.Chtimes(file1, time.Now(), time.Now()); err != nil {
Expand All @@ -92,7 +116,7 @@ func TestFileHasher_CacheMiss(t *testing.T) {
err = file.Sync()
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.Equal(t, hash1, hash2)
Expand All @@ -102,11 +126,11 @@ func TestFileHasher_CacheMiss(t *testing.T) {

func TestFileHasher_ContentAgnostic(t *testing.T) {
// Files have same content but different names and modification times
file1 := createTempFile(t, "file1")
file1 := createTempFile(t, "", "file1")

// Ensure mod times are different
time.Sleep(1 * time.Millisecond)
file2 := createTempFile(t, "file1")
file2 := createTempFile(t, "", "file1")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -116,10 +140,10 @@ func TestFileHasher_ContentAgnostic(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.Equal(t, hash1, hash2)
Expand All @@ -128,9 +152,9 @@ func TestFileHasher_ContentAgnostic(t *testing.T) {
}

func TestFileHasher_NotEqualFileAdded(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
file3 := createTempFile(t, "file3")
file1 := createTempFile(t, "", "file1")
file2 := createTempFile(t, "", "file2")
file3 := createTempFile(t, "", "file3")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -140,10 +164,10 @@ func TestFileHasher_NotEqualFileAdded(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2, file3})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2, file3})
assert.NoError(t, err)

assert.NotEqual(t, hash1, hash2)
Expand All @@ -152,9 +176,9 @@ func TestFileHasher_NotEqualFileAdded(t *testing.T) {
}

func TestFileHasher_NotEqualFileRemoved(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
file3 := createTempFile(t, "file3")
file1 := createTempFile(t, "", "file1")
file2 := createTempFile(t, "", "file2")
file3 := createTempFile(t, "", "file3")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -164,10 +188,10 @@ func TestFileHasher_NotEqualFileRemoved(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2, file3})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2, file3})
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.NotEqual(t, hash1, hash2)
Expand All @@ -176,8 +200,8 @@ func TestFileHasher_NotEqualFileRemoved(t *testing.T) {
}

func TestFileHasher_NotEqualContentChanged(t *testing.T) {
file1 := createTempFile(t, "file1")
file2 := createTempFile(t, "file2")
file1 := createTempFile(t, "", "file1")
file2 := createTempFile(t, "", "file2")

tc := &testCache{
cache: cache.New(cache.NoExpiration, cache.NoExpiration),
Expand All @@ -187,10 +211,10 @@ func TestFileHasher_NotEqualContentChanged(t *testing.T) {
cache: tc,
}

hash1, err := hasher.HashFiles([]string{file1, file2})
hash1, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

hash2, err := hasher.HashFiles([]string{file1, file2})
hash2, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.Equal(t, hash1, hash2)
Expand All @@ -203,26 +227,28 @@ func TestFileHasher_NotEqualContentChanged(t *testing.T) {
t.Fatal(err)
}

hash2Modified, err := hasher.HashFiles([]string{file1, file2})
hash2Modified, _, err := hasher.HashFiles(osutils.GetwdUnsafe(), []string{file1, file2})
assert.NoError(t, err)

assert.NotEqual(t, hash1, hash2Modified)
assert.Len(t, tc.hits, 3)
assert.Len(t, tc.misses, 3)
}

func createTempFile(t *testing.T, content string) string {
tmpfile, err := os.CreateTemp("", "testfile")
if err != nil {
t.Fatal(err)
}

if _, err := tmpfile.Write([]byte(content)); err != nil {
t.Fatal(err)
func createTempFile(t *testing.T, dir, path string) string {
if dir == "" {
dir = t.TempDir()
}
if err := tmpfile.Close(); err != nil {
t.Fatal(err)
if path == "" {
tmpfile, err := os.CreateTemp(dir, "")
if err != nil {
t.Fatal(err)
}
path = tmpfile.Name()
tmpfile.Close()
}
err := fileutils.WriteFile(filepath.Join(dir, path), []byte(path)) // Contents aren't important so long as they're consistent
require.NoError(t, err, errs.JoinMessage(err))

return tmpfile.Name()
return path
}
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,14 @@ require (

require (
github.com/ActiveState/graphql v0.0.0-20230719154233-6949037a6e48
github.com/bmatcuk/doublestar/v4 v4.7.1
github.com/brunoga/deep v1.2.4
github.com/cespare/xxhash v1.1.0
github.com/charmbracelet/bubbles v0.18.0
github.com/charmbracelet/bubbletea v0.25.0
github.com/charmbracelet/lipgloss v0.9.1
github.com/go-git/go-git/v5 v5.12.0
github.com/gowebpki/jcs v1.0.1
github.com/klauspost/compress v1.11.4
github.com/mholt/archiver/v3 v3.5.1
github.com/zijiren233/yaml-comment v0.2.1
Expand Down Expand Up @@ -143,7 +145,7 @@ require (
github.com/josharian/intern v1.0.0 // indirect
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
github.com/labstack/gommon v0.3.1 // indirect
github.com/labstack/gommon v0.3.1
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/matryer/is v1.2.0 // indirect
Expand Down
Loading
Loading