Skip to content

Commit e0ea7ee

Browse files
committed
fix zstash for Windows CI
1 parent b931d31 commit e0ea7ee

9 files changed

Lines changed: 71 additions & 21 deletions

File tree

.bk.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
selected_org: buildkite
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
testdata/** -text

internal/zstash/archive/create_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"path/filepath"
7+
"runtime"
78
"slices"
89
"strings"
910
"testing"
@@ -12,6 +13,12 @@ import (
1213
)
1314

1415
func TestBuildArchive(t *testing.T) {
16+
if runtime.GOOS == "windows" {
17+
// The expected sha256/size encode unix file modes and LF line
18+
// endings, which differ on Windows.
19+
t.Skip("archive byte layout is platform-specific")
20+
}
21+
1522
_, err := trace.NewProvider(context.Background(), "noop", "test", "0.0.1")
1623
if err != nil {
1724
t.Fatalf("trace.NewProvider: %v", err)
@@ -22,7 +29,7 @@ func TestBuildArchive(t *testing.T) {
2229
t.Fatalf("os.Getwd: %v", err)
2330
}
2431

25-
t.Setenv("HOME", home)
32+
setHomeDir(t, home)
2633

2734
archiveInfo, err := BuildArchive(context.Background(), []string{"testdata"}, "test")
2835
if err != nil {
@@ -51,7 +58,7 @@ func TestBuildAndExtractArchive_MultipleHomeDirPaths(t *testing.T) {
5158
}
5259

5360
home := t.TempDir()
54-
t.Setenv("HOME", home)
61+
setHomeDir(t, home)
5562

5663
goBuildDir := filepath.Join(home, ".go-build")
5764
err = os.MkdirAll(goBuildDir, 0o755)
@@ -168,7 +175,7 @@ func TestBuildArchive_MissingPathOnFilesystem(t *testing.T) {
168175
}
169176

170177
home := t.TempDir()
171-
t.Setenv("HOME", home)
178+
setHomeDir(t, home)
172179

173180
goBuildDir := filepath.Join(home, ".go-build")
174181
err = os.MkdirAll(goBuildDir, 0o755)
@@ -220,7 +227,7 @@ func TestExtractArchive_MissingPathInArchive(t *testing.T) {
220227
}
221228

222229
home := t.TempDir()
223-
t.Setenv("HOME", home)
230+
setHomeDir(t, home)
224231

225232
goBuildDir := filepath.Join(home, ".go-build")
226233
err = os.MkdirAll(goBuildDir, 0o755)

internal/zstash/archive/extract.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,12 @@ func ExtractFiles(ctx context.Context, zipFile *os.File, zipFileLen int64, paths
5555
foundPaths := make(map[string]bool)
5656

5757
err = extract.ExtractWithPathMapper(ctx, func(file *zip.File) (string, error) {
58+
// Zip entry names always use forward slashes (per the zip spec),
59+
// but mapping.RelativePath comes from filepath.Rel and may use the
60+
// OS native separator (backslash on Windows). Normalise the mapping
61+
// to forward slashes for the comparison.
5862
for _, mapping := range mappings {
59-
if strings.HasPrefix(file.Name, mapping.RelativePath) {
63+
if strings.HasPrefix(file.Name, filepath.ToSlash(mapping.RelativePath)) {
6064
foundPaths[mapping.Path] = true
6165
return filepath.Join(mapping.Chroot, file.Name), nil
6266
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package archive
2+
3+
import "testing"
4+
5+
// setHomeDir overrides the home directory used by os.UserHomeDir() for the
6+
// duration of the test. On unix-like systems os.UserHomeDir() reads $HOME,
7+
// while on Windows it reads %USERPROFILE%; set both so callers don't have
8+
// to care about the host platform.
9+
func setHomeDir(t *testing.T, home string) {
10+
t.Helper()
11+
t.Setenv("HOME", home)
12+
t.Setenv("USERPROFILE", home)
13+
}

internal/zstash/archive/mappings_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88

99
func TestPathsToMappings_AbsolutePathUnderHome(t *testing.T) {
1010
home := t.TempDir()
11-
t.Setenv("HOME", home)
11+
setHomeDir(t, home)
1212

1313
paths := []string{
1414
filepath.Join(home, ".go-build"),

internal/zstash/cache_integration_test.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,18 +270,25 @@ func setupTestCache(t *testing.T, storageType string) (cacheClient *Cache, cache
270270
// Create mock API client
271271
mockClient := newMockAPIClient(storageType)
272272

273-
// Build storage URL based on type (need absolute path for file:// URLs)
273+
// Build storage URL based on type (need absolute path for file:// URLs).
274+
// On Windows, paths like "C:\foo" must be encoded as "/C:/foo" so the
275+
// resulting URL is the well-formed "file:///C:/foo".
274276
absStorageDir, err := filepath.Abs(storageDir)
275277
if err != nil {
276278
t.Fatalf("filepath.Abs: %v", err)
277279
}
278280

281+
urlPath := filepath.ToSlash(absStorageDir)
282+
if !strings.HasPrefix(urlPath, "/") {
283+
urlPath = "/" + urlPath
284+
}
285+
279286
var bucketURL string
280287
switch storageType {
281288
case "local_file":
282-
bucketURL = fmt.Sprintf("file://%s", absStorageDir)
289+
bucketURL = fmt.Sprintf("file://%s", urlPath)
283290
case "local_s3":
284-
bucketURL = fmt.Sprintf("file://%s", absStorageDir) // Use file:// for testing
291+
bucketURL = fmt.Sprintf("file://%s", urlPath) // Use file:// for testing
285292
default:
286293
t.Fatalf("unsupported storage type: %s", storageType)
287294
}

internal/zstash/restore.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,28 @@ func cleanPath(ctx context.Context, dir string) error {
331331
}
332332

333333
// Module cache has 0555 directories; make them writable in order to remove content.
334-
// Use os.Root to confine chmod operations within `clean` and prevent symlink TOCTOU traversal.
334+
if err := makeTreeWritable(ctx, clean); err != nil {
335+
return err
336+
}
337+
338+
// Check context again before potentially long RemoveAll
339+
if ctx.Err() != nil {
340+
return ctx.Err()
341+
}
342+
343+
if err := os.RemoveAll(clean); err != nil {
344+
return fmt.Errorf("cleanPath: failed to remove %q: %w", clean, err)
345+
}
346+
347+
return nil
348+
}
349+
350+
// makeTreeWritable walks `clean` and chmods every directory to 0755 so that
351+
// the subsequent os.RemoveAll can delete read-only entries (e.g. Go module
352+
// cache). The os.Root handle is closed before returning so that the caller
353+
// can remove `clean` on platforms (Windows) that disallow removing a
354+
// directory with an open handle.
355+
func makeTreeWritable(ctx context.Context, clean string) error {
335356
root, err := os.OpenRoot(clean)
336357
if err != nil {
337358
if errors.Is(err, fs.ErrNotExist) {
@@ -342,7 +363,6 @@ func cleanPath(ctx context.Context, dir string) error {
342363
defer func() { _ = root.Close() }()
343364

344365
err = fs.WalkDir(root.FS(), ".", func(relPath string, info fs.DirEntry, walkErr error) error {
345-
// Respect context cancellation for long directory trees
346366
select {
347367
case <-ctx.Done():
348368
return ctx.Err()
@@ -367,15 +387,5 @@ func cleanPath(ctx context.Context, dir string) error {
367387
}
368388
return fmt.Errorf("cleanPath: error preparing %q for removal: %w", clean, err)
369389
}
370-
371-
// Check context again before potentially long RemoveAll
372-
if ctx.Err() != nil {
373-
return ctx.Err()
374-
}
375-
376-
if err := os.RemoveAll(clean); err != nil {
377-
return fmt.Errorf("cleanPath: failed to remove %q: %w", clean, err)
378-
}
379-
380390
return nil
381391
}

internal/zstash/store/file.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os"
1313
"path/filepath"
1414
"regexp"
15+
"runtime"
1516
"strings"
1617
"time"
1718

@@ -92,6 +93,12 @@ func NewLocalFileBlob(ctx context.Context, fileURL string) (*LocalFileBlob, erro
9293
path = filepath.Join(homeDir, path)
9394
}
9495

96+
// A Windows file URL like "file:///C:/foo/bar" parses to u.Path = "/C:/foo/bar".
97+
// Strip the spurious leading "/" so it becomes a valid OS path "C:/foo/bar".
98+
if runtime.GOOS == "windows" && len(path) >= 3 && path[0] == '/' && path[2] == ':' {
99+
path = path[1:]
100+
}
101+
95102
root := filepath.Clean(filepath.FromSlash(path))
96103
if root == "" || root == "/" || root == "." {
97104
return nil, fmt.Errorf("invalid root directory: %s", root)

0 commit comments

Comments
 (0)