From f3aa31e401e58ea20aa0c0fd09b55f237bbfb54a Mon Sep 17 00:00:00 2001 From: Ophestra Date: Mon, 5 Jan 2026 01:05:27 +0900 Subject: [PATCH] internal/pkg: temporary scratch space for cure This allows for more flexibility during implementation. The use case that required this was for expanding single directory tarballs. Signed-off-by: Ophestra --- internal/pkg/net.go | 2 +- internal/pkg/pkg.go | 59 +++++++++++++++++++++++++--------------- internal/pkg/pkg_test.go | 26 +++++++++--------- internal/pkg/tar.go | 2 +- 4 files changed, 52 insertions(+), 37 deletions(-) diff --git a/internal/pkg/net.go b/internal/pkg/net.go index 74eeca5..074c5f7 100644 --- a/internal/pkg/net.go +++ b/internal/pkg/net.go @@ -101,7 +101,7 @@ func (a *httpArtifact) do() (data []byte, err error) { } // Cure returns syscall.ENOTSUP. Callers should use Data instead. -func (a *httpArtifact) Cure(*check.Absolute, CacheDataFunc) error { +func (a *httpArtifact) Cure(*check.Absolute, *check.Absolute, CacheDataFunc) error { return syscall.ENOTSUP } diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index b50dc48..8b07786 100644 --- a/internal/pkg/pkg.go +++ b/internal/pkg/pkg.go @@ -89,7 +89,11 @@ type Artifact interface { // If the implementation produces a single file, it must implement [File] // as well. In that case, Cure must produce a single regular file with // contents identical to that returned by [File.Data]. - Cure(work *check.Absolute, loadData CacheDataFunc) (err error) + // + // Implementations may use temp as scratch space. The caller is not required + // to create a directory here, implementations are expected to create it if + // they wish to use it, using [os.MkdirAll]. + Cure(work, temp *check.Absolute, loadData CacheDataFunc) (err error) } // KnownIdent is optionally implemented by [Artifact] and is used instead of @@ -183,6 +187,9 @@ const ( // dirWork is the directory name appended to Cache.base for working // pathnames set up during [Cache.Cure]. dirWork = "work" + // dirTemp is the directory name appended to Cache.base for scratch space + // pathnames allocated during [Cache.Cure]. + dirTemp = "temp" // checksumLinknamePrefix is prepended to the encoded [Checksum] value // of an [Artifact] when creating a symbolic link to dirChecksum. @@ -346,6 +353,28 @@ func (NoOutputError) Error() string { return "artifact cured successfully but did not produce any output" } +// removeAll is similar to [os.RemoveAll] but is robust against any permissions. +func removeAll(pathname *check.Absolute) (chmodErr, removeErr error) { + chmodErr = filepath.WalkDir(pathname.String(), func( + path string, + d fs.DirEntry, + err error, + ) error { + if err != nil { + return err + } + if d.IsDir() { + return os.Chmod(path, 0700) + } + return nil + }) + if errors.Is(chmodErr, os.ErrNotExist) { + chmodErr = nil + } + removeErr = os.RemoveAll(pathname.String()) + return +} + // Cure cures the [Artifact] and returns its pathname and [Checksum]. func (c *Cache) Cure(a Artifact) ( pathname *check.Absolute, @@ -489,27 +518,8 @@ func (c *Cache) Cure(a Artifact) ( workPathname := c.base.Append(dirWork, ids) defer func() { - // must not use the value of checksum string as it might be zeroed - // to cancel the deferred symlink operation - if err != nil { - chmodErr := filepath.WalkDir(workPathname.String(), func( - path string, - d fs.DirEntry, - err error, - ) error { - if err != nil { - return err - } - if d.IsDir() { - return os.Chmod(path, 0700) - } - return nil - }) - if errors.Is(chmodErr, os.ErrNotExist) { - chmodErr = nil - } - removeErr := os.RemoveAll(workPathname.String()) + chmodErr, removeErr := removeAll(workPathname) if chmodErr != nil || removeErr != nil { err = errors.Join(err, chmodErr, removeErr) } else if errors.Is(err, os.ErrExist) { @@ -519,7 +529,12 @@ func (c *Cache) Cure(a Artifact) ( } }() - if err = a.Cure(workPathname, c.loadData); err != nil { + tempPathname := c.base.Append(dirTemp, ids) + if err = a.Cure(workPathname, tempPathname, c.loadData); err != nil { + return + } + if chmodErr, removeErr := removeAll(tempPathname); chmodErr != nil || removeErr != nil { + err = errors.Join(err, chmodErr, removeErr) return } diff --git a/internal/pkg/pkg_test.go b/internal/pkg/pkg_test.go index edd5f07..1c650f1 100644 --- a/internal/pkg/pkg_test.go +++ b/internal/pkg/pkg_test.go @@ -72,7 +72,7 @@ type stubArtifact struct { params []byte deps []pkg.Artifact - cure func(work *check.Absolute, loadData pkg.CacheDataFunc) error + cure func(work, temp *check.Absolute, loadData pkg.CacheDataFunc) error } func (a stubArtifact) Kind() pkg.Kind { return a.kind } @@ -80,10 +80,10 @@ func (a stubArtifact) Params() []byte { return a.params } func (a stubArtifact) Dependencies() []pkg.Artifact { return a.deps } func (a stubArtifact) Cure( - work *check.Absolute, + work, temp *check.Absolute, loadData pkg.CacheDataFunc, ) error { - return a.cure(work, loadData) + return a.cure(work, temp, loadData) } // A stubFile implements [File] with hardcoded behaviour. @@ -108,7 +108,7 @@ func newStubFile( kind, nil, nil, - func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + func(*check.Absolute, *check.Absolute, pkg.CacheDataFunc) error { panic("unreachable") }, }}} @@ -375,7 +375,7 @@ func TestCache(t *testing.T) { binary.LittleEndian.AppendUint64(nil, pkg.TarGzip), overrideIdent{testdataChecksum, stubArtifact{}}, ) - makeSample := func(work *check.Absolute, _ pkg.CacheDataFunc) error { + makeSample := func(work, _ *check.Absolute, _ pkg.CacheDataFunc) error { if err := os.Mkdir(work.String(), 0700); err != nil { return err } @@ -476,14 +476,14 @@ func TestCache(t *testing.T) { {"cure fault", overrideIdent{pkg.ID{0xff, 0}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, _ pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, _ pkg.CacheDataFunc) error { return makeGarbage(work, stub.UniqueError(0xcafe)) }, }}, nil, pkg.Checksum{}, stub.UniqueError(0xcafe)}, {"checksum mismatch", overrideChecksum{pkg.Checksum{}, overrideIdent{pkg.ID{0xff, 1}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, _ pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, _ pkg.CacheDataFunc) error { return makeGarbage(work, nil) }, }}}, nil, pkg.Checksum{}, &pkg.ChecksumMismatchError{ @@ -503,7 +503,7 @@ func TestCache(t *testing.T) { {"loadData directory", overrideIdent{pkg.ID{0xff, 3}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { _, err := loadData(overrideChecksumFile{checksum: wantChecksum}) return err }, @@ -518,21 +518,21 @@ func TestCache(t *testing.T) { {"no output", overrideIdent{pkg.ID{0xff, 4}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { return nil }, }}, nil, pkg.Checksum{}, pkg.NoOutputError{}}, {"file output", overrideIdent{pkg.ID{0xff, 5}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { return os.WriteFile(work.String(), []byte{0}, 0400) }, }}, nil, pkg.Checksum{}, errors.New("non-file artifact produced regular file")}, {"symlink output", overrideIdent{pkg.ID{0xff, 6}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { return os.Symlink(work.String(), work.String()) }, }}, nil, pkg.Checksum{}, pkg.InvalidFileModeError( @@ -549,7 +549,7 @@ func TestCache(t *testing.T) { go func() { if _, _, err := c.Cure(overrideIdent{pkg.ID{0xff}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { close(ready) <-n return wantErr @@ -579,7 +579,7 @@ func TestCache(t *testing.T) { {"file output", overrideIdent{pkg.ID{0xff, 2}, stubArtifact{ kind: pkg.KindTar, - cure: func(work *check.Absolute, loadData pkg.CacheDataFunc) error { + cure: func(work, _ *check.Absolute, loadData pkg.CacheDataFunc) error { return os.WriteFile(work.String(), []byte{0}, 0400) }, }}, nil, pkg.Checksum{}, errors.New("non-file artifact produced regular file")}, diff --git a/internal/pkg/tar.go b/internal/pkg/tar.go index 24411c5..ac36cf9 100644 --- a/internal/pkg/tar.go +++ b/internal/pkg/tar.go @@ -69,7 +69,7 @@ func (e DisallowedTypeflagError) Error() string { } // Cure cures the [Artifact], producing a directory located at work. -func (a *tarArtifact) Cure(work *check.Absolute, loadData CacheDataFunc) (err error) { +func (a *tarArtifact) Cure(work, temp *check.Absolute, loadData CacheDataFunc) (err error) { var tr io.ReadCloser {