From e91049c3c5a81bc18fb5394de92a6d7617421938 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Tue, 6 Jan 2026 00:56:49 +0900 Subject: [PATCH] internal/pkg: pass cure context as single value This cleans up the function signature and makes backwards compatible API changes possible. Signed-off-by: Ophestra --- internal/pkg/net.go | 4 +-- internal/pkg/pkg.go | 68 ++++++++++++++++++++++++++++------------ internal/pkg/pkg_test.go | 60 ++++++++++++++++++----------------- internal/pkg/tar.go | 9 +++--- 4 files changed, 86 insertions(+), 55 deletions(-) diff --git a/internal/pkg/net.go b/internal/pkg/net.go index 074c5f7..9a6814f 100644 --- a/internal/pkg/net.go +++ b/internal/pkg/net.go @@ -7,8 +7,6 @@ import ( "net/http" "sync" "syscall" - - "hakurei.app/container/check" ) // An httpArtifact is an [Artifact] backed by a [http] url string. The method is @@ -101,7 +99,7 @@ func (a *httpArtifact) do() (data []byte, err error) { } // Cure returns syscall.ENOTSUP. Callers should use Data instead. -func (a *httpArtifact) Cure(*check.Absolute, *check.Absolute, CacheDataFunc) error { +func (a *httpArtifact) Cure(*CureContext) error { return syscall.ENOTSUP } diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index ec81995..e1055ac 100644 --- a/internal/pkg/pkg.go +++ b/internal/pkg/pkg.go @@ -53,9 +53,36 @@ func MustDecode(s string) Checksum { } } -// CacheDataFunc tries to load [File] from [Cache], and if that fails, obtains -// it via [File.Data] instead. -type CacheDataFunc func(f File) (data []byte, err error) +// CureContext is passed to [Artifact.Cure] and contains information and methods +// useful for curing the [Artifact], like requesting the data of [File], or that +// other artifacts be cured. +// +// Methods of CureContext are safe for concurrent use. CureContext is valid +// until [Artifact.Cure] returns. +type CureContext struct { + // Address of underlying [Cache], should be zeroed after [Artifact.Cure] + // returns and must not be exposed directly. + cache *Cache + + // Populated during [Cache.Cure]. + work, temp *check.Absolute +} + +// GetWorkDir returns a pathname to a directory which [Artifact] is expected to +// write its output to. This is not the final resting place of the [Artifact] +// and this pathname should not be directly referred to in the final contents. +func (c *CureContext) GetWorkDir() *check.Absolute { return c.work } + +// GetTempDir returns a pathname which implementations may use as scratch space. +// A directory is not created automatically, implementations are expected to +// create it if they wish to use it, using [os.MkdirAll]. +func (c *CureContext) GetTempDir() *check.Absolute { return c.temp } + +// LoadData tries to load [File] from [Cache], and if that fails, obtains it via +// [File.Data] instead. Notably, it does not cure [File]. +func (c *CureContext) LoadData(f File) (data []byte, err error) { + return c.cache.loadData(f) +} // An Artifact is a read-only reference to a piece of data that may be created // deterministically but might not currently be available in memory or on the @@ -83,18 +110,15 @@ type Artifact interface { // Result must remain identical across multiple invocations. Dependencies() []Artifact - // Cure cures the current [Artifact] to the caller-specified temporary - // pathname. This is not the final resting place of the [Artifact] and this - // pathname should not be directly referred to in the final contents. + // Cure cures the current [Artifact] to the working directory obtained via + // [CureContext.GetWorkDir]. // // 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]. // - // 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) + // Implementations must not retain c. + Cure(c *CureContext) (err error) } // KnownIdent is optionally implemented by [Artifact] and is used instead of @@ -515,7 +539,7 @@ func (c *Cache) finaliseIdent( close(done) } -// loadData provides [CacheDataFunc] for [Artifact.Cure]. +// loadData provides [CureContext.LoadData] for [Artifact.Cure]. func (c *Cache) loadData(f File) (data []byte, err error) { var r *os.File if kc, ok := f.(KnownChecksum); ok { @@ -790,10 +814,14 @@ func (c *Cache) Cure(a Artifact) ( return } - workPathname := c.base.Append(dirWork, ids) + cc := CureContext{ + cache: c, + work: c.base.Append(dirWork, ids), + temp: c.base.Append(dirTemp, ids), + } defer func() { if err != nil { - chmodErr, removeErr := removeAll(workPathname) + chmodErr, removeErr := removeAll(cc.work) if chmodErr != nil || removeErr != nil { err = errors.Join(err, chmodErr, removeErr) } else if errors.Is(err, os.ErrExist) { @@ -803,17 +831,17 @@ func (c *Cache) Cure(a Artifact) ( } }() - tempPathname := c.base.Append(dirTemp, ids) - if err = a.Cure(workPathname, tempPathname, c.loadData); err != nil { + if err = a.Cure(&cc); err != nil { return } - if chmodErr, removeErr := removeAll(tempPathname); chmodErr != nil || removeErr != nil { + cc.cache = nil + if chmodErr, removeErr := removeAll(cc.temp); chmodErr != nil || removeErr != nil { err = errors.Join(err, chmodErr, removeErr) return } var fi os.FileInfo - if fi, err = os.Lstat(workPathname.String()); err != nil { + if fi, err = os.Lstat(cc.work.String()); err != nil { if errors.Is(err, os.ErrNotExist) { err = NoOutputError{} } @@ -831,7 +859,7 @@ func (c *Cache) Cure(a Artifact) ( var gotChecksum Checksum if gotChecksum, err = HashFS( - dotOverrideFS{os.DirFS(workPathname.String()).(dirFS)}, + dotOverrideFS{os.DirFS(cc.work.String()).(dirFS)}, ".", ); err != nil { return @@ -854,12 +882,12 @@ func (c *Cache) Cure(a Artifact) ( } } - if err = os.Chmod(workPathname.String(), 0700); err != nil { + if err = os.Chmod(cc.work.String(), 0700); err != nil { return } c.checksumMu.Lock() if err = os.Rename( - workPathname.String(), + cc.work.String(), checksumPathname.String(), ); err != nil { if !errors.Is(err, os.ErrExist) { diff --git a/internal/pkg/pkg_test.go b/internal/pkg/pkg_test.go index 8826e5a..dca3932 100644 --- a/internal/pkg/pkg_test.go +++ b/internal/pkg/pkg_test.go @@ -73,19 +73,13 @@ type stubArtifact struct { params []byte deps []pkg.Artifact - cure func(work, temp *check.Absolute, loadData pkg.CacheDataFunc) error + cure func(c *pkg.CureContext) error } -func (a stubArtifact) Kind() pkg.Kind { return a.kind } -func (a stubArtifact) Params() []byte { return a.params } -func (a stubArtifact) Dependencies() []pkg.Artifact { return a.deps } - -func (a stubArtifact) Cure( - work, temp *check.Absolute, - loadData pkg.CacheDataFunc, -) error { - return a.cure(work, temp, loadData) -} +func (a stubArtifact) Kind() pkg.Kind { return a.kind } +func (a stubArtifact) Params() []byte { return a.params } +func (a stubArtifact) Dependencies() []pkg.Artifact { return a.deps } +func (a stubArtifact) Cure(c *pkg.CureContext) error { return a.cure(c) } // A stubFile implements [File] with hardcoded behaviour. type stubFile struct { @@ -109,7 +103,7 @@ func newStubFile( kind, nil, nil, - func(*check.Absolute, *check.Absolute, pkg.CacheDataFunc) error { + func(*pkg.CureContext) error { panic("unreachable") }, }}} @@ -410,7 +404,8 @@ func TestCache(t *testing.T) { binary.LittleEndian.AppendUint64(nil, pkg.TarGzip), overrideIdent{testdataChecksum, stubArtifact{}}, ) - makeSample := func(work, _ *check.Absolute, _ pkg.CacheDataFunc) error { + makeSample := func(c *pkg.CureContext) error { + work := c.GetWorkDir() if err := os.Mkdir(work.String(), 0700); err != nil { return err } @@ -511,15 +506,15 @@ func TestCache(t *testing.T) { {"cure fault", overrideIdent{pkg.ID{0xff, 0}, stubArtifact{ kind: pkg.KindTar, - cure: func(work, _ *check.Absolute, _ pkg.CacheDataFunc) error { - return makeGarbage(work, stub.UniqueError(0xcafe)) + cure: func(c *pkg.CureContext) error { + return makeGarbage(c.GetWorkDir(), 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 { - return makeGarbage(work, nil) + cure: func(c *pkg.CureContext) error { + return makeGarbage(c.GetWorkDir(), nil) }, }}}, nil, pkg.Checksum{}, &pkg.ChecksumMismatchError{ Got: pkg.MustDecode( @@ -538,8 +533,8 @@ 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 { - _, err := loadData(overrideChecksumFile{checksum: wantChecksum}) + cure: func(c *pkg.CureContext) error { + _, err := c.LoadData(overrideChecksumFile{checksum: wantChecksum}) return err }, }}, nil, pkg.Checksum{}, &os.PathError{ @@ -553,22 +548,25 @@ 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(c *pkg.CureContext) 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 { - return os.WriteFile(work.String(), []byte{0}, 0400) + cure: func(c *pkg.CureContext) error { + return os.WriteFile(c.GetWorkDir().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 { - return os.Symlink(work.String(), work.String()) + cure: func(c *pkg.CureContext) error { + return os.Symlink( + c.GetWorkDir().String(), + c.GetWorkDir().String(), + ) }, }}, nil, pkg.Checksum{}, pkg.InvalidFileModeError( fs.ModeSymlink | 0777, @@ -584,7 +582,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(c *pkg.CureContext) error { close(ready) <-n return wantErr @@ -616,10 +614,16 @@ 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 { - return os.WriteFile(work.String(), []byte{0}, 0400) + cure: func(c *pkg.CureContext) error { + return os.WriteFile( + c.GetWorkDir().String(), + []byte{0}, + 0400, + ) }, - }}, nil, pkg.Checksum{}, errors.New("non-file artifact produced regular file")}, + }}, nil, pkg.Checksum{}, errors.New( + "non-file artifact produced regular file", + )}, }) wantErrScrub := &pkg.ScrubError{ diff --git a/internal/pkg/tar.go b/internal/pkg/tar.go index f058d24..0d1c572 100644 --- a/internal/pkg/tar.go +++ b/internal/pkg/tar.go @@ -69,12 +69,13 @@ func (e DisallowedTypeflagError) Error() string { } // Cure cures the [Artifact], producing a directory located at work. -func (a *tarArtifact) Cure(work, temp *check.Absolute, loadData CacheDataFunc) (err error) { +func (a *tarArtifact) Cure(c *CureContext) (err error) { + temp := c.GetTempDir() var tr io.ReadCloser { var data []byte - data, err = loadData(a.f) + data, err = c.LoadData(a.f) if err != nil { return } @@ -209,9 +210,9 @@ func (a *tarArtifact) Cure(work, temp *check.Absolute, loadData CacheDataFunc) ( if err = os.Chmod(p.String(), 0700); err != nil { return } - err = os.Rename(p.String(), work.String()) + err = os.Rename(p.String(), c.GetWorkDir().String()) } else { - err = os.Rename(temp.String(), work.String()) + err = os.Rename(temp.String(), c.GetWorkDir().String()) } return }