diff --git a/cmd/mbf/main.go b/cmd/mbf/main.go index c5a8868b..17ae1b05 100644 --- a/cmd/mbf/main.go +++ b/cmd/mbf/main.go @@ -69,11 +69,10 @@ func main() { }() var ( - flagQuiet bool - flagCures int - flagBase string - flagTShift int - flagIdle bool + flagQuiet bool + flagCures int + flagBase string + flagIdle bool ) c := command.New(os.Stderr, log.Printf, "mbf", func([]string) (err error) { msg.SwapVerbose(!flagQuiet) @@ -89,16 +88,7 @@ func main() { } else if base, err = check.NewAbs(flagBase); err != nil { return } - if cache, err = pkg.Open(ctx, msg, flagCures, base); err == nil { - if flagTShift < 0 { - cache.SetThreshold(0) - } else if flagTShift > 31 { - cache.SetThreshold(1 << 31) - } else { - cache.SetThreshold(1 << flagTShift) - } - } - + cache, err = pkg.Open(ctx, msg, 0, flagCures, base) if flagIdle { pkg.SetSchedIdle = true } @@ -116,10 +106,6 @@ func main() { &flagBase, "d", command.StringFlag("$MBF_CACHE_DIR"), "Directory to store cured artifacts", - ).Flag( - &flagTShift, - "tshift", command.IntFlag(-1), - "Dependency graph size exponent, to the power of 2", ).Flag( &flagIdle, "sched-idle", command.BoolFlag(false), diff --git a/internal/pkg/exec_test.go b/internal/pkg/exec_test.go index b3eaa74e..cbd49013 100644 --- a/internal/pkg/exec_test.go +++ b/internal/pkg/exec_test.go @@ -33,8 +33,7 @@ func TestExec(t *testing.T) { ) checkWithCache(t, []cacheTestCase{ - {"offline", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"offline", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() cureMany(t, c, []cureStep{ @@ -111,8 +110,7 @@ func TestExec(t *testing.T) { testtoolDestroy(t, base, c) }, pkg.MustDecode("Q5DluWQCAeohLoiGRImurwFp3vdz9IfQCoj7Fuhh73s4KQPRHpEQEnHTdNHmB8Fx")}, - {"net", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"net", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() wantChecksum := pkg.MustDecode( @@ -146,8 +144,7 @@ func TestExec(t *testing.T) { testtoolDestroy(t, base, c) }, pkg.MustDecode("bPYvvqxpfV7xcC1EptqyKNK1klLJgYHMDkzBcoOyK6j_Aj5hb0mXNPwTwPSK5F6Z")}, - {"overlay root", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"overlay root", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() cureMany(t, c, []cureStep{ @@ -172,8 +169,7 @@ func TestExec(t *testing.T) { testtoolDestroy(t, base, c) }, pkg.MustDecode("PO2DSSCa4yoSgEYRcCSZfQfwow1yRigL3Ry-hI0RDI4aGuFBha-EfXeSJnG_5_Rl")}, - {"overlay work", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"overlay work", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() cureMany(t, c, []cureStep{ @@ -203,8 +199,7 @@ func TestExec(t *testing.T) { testtoolDestroy(t, base, c) }, pkg.MustDecode("iaRt6l_Wm2n-h5UsDewZxQkCmjZjyL8r7wv32QT2kyV55-Lx09Dq4gfg9BiwPnKs")}, - {"multiple layers", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"multiple layers", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() cureMany(t, c, []cureStep{ @@ -256,8 +251,7 @@ func TestExec(t *testing.T) { testtoolDestroy(t, base, c) }, pkg.MustDecode("O2YzyR7IUGU5J2CADy0hUZ3A5NkP_Vwzs4UadEdn2oMZZVWRtH0xZGJ3HXiimTnZ")}, - {"overlay layer promotion", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) + {"overlay layer promotion", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { testtool, testtoolDestroy := newTesttool() cureMany(t, c, []cureStep{ diff --git a/internal/pkg/file_test.go b/internal/pkg/file_test.go index ca0046ab..734be8b0 100644 --- a/internal/pkg/file_test.go +++ b/internal/pkg/file_test.go @@ -11,9 +11,7 @@ func TestFile(t *testing.T) { t.Parallel() checkWithCache(t, []cacheTestCase{ - {"file", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) - + {"file", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { cureMany(t, c, []cureStep{ {"short", pkg.NewFile("null", []byte{0}), base.Append( "identifier", diff --git a/internal/pkg/ir_test.go b/internal/pkg/ir_test.go index a946d60f..de33c74f 100644 --- a/internal/pkg/ir_test.go +++ b/internal/pkg/ir_test.go @@ -85,7 +85,7 @@ func TestIRRoundtrip(t *testing.T) { testCasesCache := make([]cacheTestCase, len(testCases)) for i, tc := range testCases { want := tc.a - testCasesCache[i] = cacheTestCase{tc.name, nil, + testCasesCache[i] = cacheTestCase{tc.name, 0, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { r, w := io.Pipe() diff --git a/internal/pkg/net_test.go b/internal/pkg/net_test.go index 9f668e71..f525efbd 100644 --- a/internal/pkg/net_test.go +++ b/internal/pkg/net_test.go @@ -32,7 +32,7 @@ func TestHTTPGet(t *testing.T) { })) checkWithCache(t, []cacheTestCase{ - {"direct", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"direct", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { var r pkg.RContext rCacheVal := reflect.ValueOf(&r).Elem().FieldByName("cache") reflect.NewAt( @@ -94,7 +94,7 @@ func TestHTTPGet(t *testing.T) { } }, pkg.MustDecode("E4vEZKhCcL2gPZ2Tt59FS3lDng-d_2SKa2i5G_RbDfwGn6EemptFaGLPUDiOa94C")}, - {"cure", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"cure", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { var r pkg.RContext rCacheVal := reflect.ValueOf(&r).Elem().FieldByName("cache") reflect.NewAt( diff --git a/internal/pkg/pkg.go b/internal/pkg/pkg.go index ea7ac815..a542f3d2 100644 --- a/internal/pkg/pkg.go +++ b/internal/pkg/pkg.go @@ -13,7 +13,6 @@ import ( "hash" "io" "io/fs" - "iter" "maps" "os" "path/filepath" @@ -331,23 +330,6 @@ type FloodArtifact interface { Artifact } -// Flood returns an iterator over the dependency tree of an [Artifact]. -func Flood(a Artifact) iter.Seq[Artifact] { - return func(yield func(Artifact) bool) { - for _, d := range a.Dependencies() { - if !yield(d) { - return - } - - for d0 := range Flood(d) { - if !yield(d0) { - return - } - } - } - } -} - // TrivialArtifact refers to an [Artifact] that cures without requiring that // any other [Artifact] is cured before it. Its dependency tree is ignored after // computing its identifier. @@ -506,6 +488,23 @@ type pendingArtifactDep struct { *sync.WaitGroup } +const ( + // CValidateKnown arranges for [KnownChecksum] outcomes to be validated to + // match its intended checksum. + // + // A correct implementation of [KnownChecksum] does not successfully cure + // with output not matching its intended checksum. When an implementation + // fails to perform this validation correctly, the on-disk format enters + // an inconsistent state (correctable by [Cache.Scrub]). + // + // This flag causes [Cache.Cure] to always compute the checksum, and reject + // a cure if it does not match the intended checksum. + // + // This behaviour significantly reduces performance and is not recommended + // outside of testing a custom [Artifact] implementation. + CValidateKnown = 1 << iota +) + // Cache is a support layer that implementations of [Artifact] can use to store // cured [Artifact] data in a content addressed fashion. type Cache struct { @@ -525,12 +524,8 @@ type Cache struct { // Directory where all [Cache] related files are placed. base *check.Absolute - - // Whether to validate [FileArtifact.Cure] for a [KnownChecksum] file. This - // significantly reduces performance. - strict bool - // Maximum size of a dependency graph. - threshold uintptr + // Immutable cure options set by [Open]. + flags int // Artifact to [unique.Handle] of identifier cache. artifact sync.Map @@ -563,22 +558,6 @@ type Cache struct { inExec atomic.Bool } -// IsStrict returns whether the [Cache] strictly verifies checksums. -func (c *Cache) IsStrict() bool { return c.strict } - -// SetStrict sets whether the [Cache] strictly verifies checksums, even when -// the implementation promises to validate them internally. This significantly -// reduces performance and is not recommended outside of testing. -// -// This method is not safe for concurrent use with any other method. -func (c *Cache) SetStrict(strict bool) { c.strict = strict } - -// SetThreshold imposes a maximum size on the dependency graph, checked on every -// call to Cure. The zero value disables this check entirely. -// -// This method is not safe for concurrent use with any other method. -func (c *Cache) SetThreshold(threshold uintptr) { c.threshold = threshold } - // extIdent is a [Kind] concatenated with [ID]. type extIdent [wordSize + len(ID{})]byte @@ -1229,14 +1208,6 @@ func (e InvalidArtifactError) Error() string { return "artifact " + Encode(e) + " cannot be cured" } -// DependencyError refers to an artifact with a dependency tree larger than the -// threshold specified by a previous call to [Cache.SetThreshold]. -type DependencyError struct{ A Artifact } - -func (e DependencyError) Error() string { - return "artifact has too many dependencies" -} - // Cure cures the [Artifact] and returns its pathname and [Checksum]. Direct // calls to Cure are not subject to the cures limit. func (c *Cache) Cure(a Artifact) ( @@ -1252,18 +1223,6 @@ func (c *Cache) Cure(a Artifact) ( default: } - if c.threshold > 0 { - var n uintptr - for range Flood(a) { - if n == c.threshold { - err = DependencyError{a} - return - } - n++ - } - c.msg.Verbosef("visited %d artifacts", n) - } - return c.cure(a, true) } @@ -1586,7 +1545,7 @@ func (c *Cache) cure(a Artifact, curesExempt bool) ( } r, err = f.Cure(&RContext{common{c}}) if err == nil { - if checksumPathname == nil || c.IsStrict() { + if checksumPathname == nil || c.flags&CValidateKnown != 0 { h := sha512.New384() hbw := c.getWriter(h) _, err = io.Copy(w, io.TeeReader(r, hbw)) @@ -1603,7 +1562,7 @@ func (c *Cache) cure(a Artifact, curesExempt bool) ( if checksumPathname == nil { checksum = unique.Make(Checksum(buf[:])) checksums = Encode(Checksum(buf[:])) - } else if c.IsStrict() { + } else if c.flags&CValidateKnown != 0 { if got := Checksum(buf[:]); got != checksum.Value() { err = &ChecksumMismatchError{ Got: got, @@ -1841,10 +1800,10 @@ func (c *Cache) Close() { func Open( ctx context.Context, msg message.Msg, - cures int, + flags, cures int, base *check.Absolute, ) (*Cache, error) { - return open(ctx, msg, cures, base, true) + return open(ctx, msg, flags, cures, base, true) } // open implements Open but allows omitting the [lockedfile] lock when called @@ -1852,7 +1811,7 @@ func Open( func open( ctx context.Context, msg message.Msg, - cures int, + flags, cures int, base *check.Absolute, lock bool, ) (*Cache, error) { @@ -1874,6 +1833,7 @@ func open( c := Cache{ cures: make(chan struct{}, cures), + flags: flags, msg: msg, base: base, diff --git a/internal/pkg/pkg_test.go b/internal/pkg/pkg_test.go index 6b5d5fc4..82f260e7 100644 --- a/internal/pkg/pkg_test.go +++ b/internal/pkg/pkg_test.go @@ -33,7 +33,7 @@ import ( func unsafeOpen( ctx context.Context, msg message.Msg, - cures int, + flags, cures int, base *check.Absolute, lock bool, ) (*pkg.Cache, error) @@ -228,7 +228,7 @@ func TestIdent(t *testing.T) { var cache *pkg.Cache if a, err := check.NewAbs(t.TempDir()); err != nil { t.Fatal(err) - } else if cache, err = pkg.Open(t.Context(), msg, 0, a); err != nil { + } else if cache, err = pkg.Open(t.Context(), msg, 0, 0, a); err != nil { t.Fatal(err) } t.Cleanup(cache.Close) @@ -252,6 +252,7 @@ func TestIdent(t *testing.T) { // on test completion. type cacheTestCase struct { name string + flags int early func(t *testing.T, base *check.Absolute) f func(t *testing.T, base *check.Absolute, c *pkg.Cache) want pkg.Checksum @@ -289,7 +290,7 @@ func checkWithCache(t *testing.T, testCases []cacheTestCase) { msg.SwapVerbose(testing.Verbose()) var scrubFunc func() error // scrub after hashing - if c, err := pkg.Open(t.Context(), msg, 1<<4, base); err != nil { + if c, err := pkg.Open(t.Context(), msg, tc.flags, 1<<4, base); err != nil { t.Fatalf("Open: error = %v", err) } else { t.Cleanup(c.Close) @@ -468,9 +469,7 @@ func TestCache(t *testing.T) { }() testCases := []cacheTestCase{ - {"file", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) - + {"file", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { identifier := (pkg.ID)(bytes.Repeat([]byte{ 0x75, 0xe6, 0x9d, 0x6d, 0xe7, 0x9f, }, 8)) @@ -593,7 +592,7 @@ func TestCache(t *testing.T) { if c0, err := unsafeOpen( t.Context(), message.New(nil), - 0, base, false, + 0, 0, base, false, ); err != nil { t.Fatalf("open: error = %v", err) } else { @@ -627,7 +626,7 @@ func TestCache(t *testing.T) { } }, pkg.MustDecode("St9rlE-mGZ5gXwiv_hzQ_B8bZP-UUvSNmf4nHUZzCMOumb6hKnheZSe0dmnuc4Q2")}, - {"directory", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"directory", 0, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { id := pkg.MustDecode( "HnySzeLQvSBZuTUcvfmLEX_OmH4yJWWH788NxuLuv7kVn8_uPM6Ks4rqFWM2NZJY", ) @@ -804,9 +803,7 @@ func TestCache(t *testing.T) { }) }, pkg.MustDecode("WVpvsVqVKg9Nsh744x57h51AuWUoUR2nnh8Md-EYBQpk6ziyTuUn6PLtF2e0Eu_d")}, - {"pending", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { - c.SetStrict(true) - + {"pending", pkg.CValidateKnown, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { wantErr := stub.UniqueError(0xcafe) n, ready := make(chan struct{}), make(chan struct{}) go func() { @@ -876,7 +873,7 @@ func TestCache(t *testing.T) { <-wCureDone }, pkg.MustDecode("E4vEZKhCcL2gPZ2Tt59FS3lDng-d_2SKa2i5G_RbDfwGn6EemptFaGLPUDiOa94C")}, - {"scrub", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"scrub", 0, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { cureMany(t, c, []cureStep{ {"bad measured file", newStubFile( pkg.KindHTTPGet, @@ -1182,7 +1179,7 @@ func (a earlyFailureF) Cure(*pkg.FContext) error { func TestDependencyCureErrorEarly(t *testing.T) { checkWithCache(t, []cacheTestCase{ - {"early", nil, func(t *testing.T, _ *check.Absolute, c *pkg.Cache) { + {"early", 0, nil, func(t *testing.T, _ *check.Absolute, c *pkg.Cache) { _, _, err := c.Cure(earlyFailureF(8)) if !errors.Is(err, stub.UniqueError(0xcafe)) { t.Fatalf("Cure: error = %v", err) @@ -1205,7 +1202,7 @@ func TestNew(t *testing.T) { if _, err := pkg.Open( t.Context(), message.New(nil), - 0, check.MustAbs(container.Nonexistent), + 0, 0, check.MustAbs(container.Nonexistent), ); !reflect.DeepEqual(err, wantErr) { t.Errorf("Open: error = %#v, want %#v", err, wantErr) } @@ -1233,7 +1230,7 @@ func TestNew(t *testing.T) { if _, err := pkg.Open( t.Context(), message.New(nil), - 0, tempDir.Append("cache"), + 0, 0, tempDir.Append("cache"), ); !reflect.DeepEqual(err, wantErr) { t.Errorf("Open: error = %#v, want %#v", err, wantErr) } diff --git a/internal/pkg/tar_test.go b/internal/pkg/tar_test.go index c604dc78..49861d2e 100644 --- a/internal/pkg/tar_test.go +++ b/internal/pkg/tar_test.go @@ -21,7 +21,7 @@ func TestTar(t *testing.T) { t.Parallel() checkWithCache(t, []cacheTestCase{ - {"http", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"http", 0, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { checkTarHTTP(t, base, c, fstest.MapFS{ ".": {Mode: fs.ModeDir | 0700}, @@ -42,7 +42,7 @@ func TestTar(t *testing.T) { )) }, pkg.MustDecode("NQTlc466JmSVLIyWklm_u8_g95jEEb98PxJU-kjwxLpfdjwMWJq0G8ze9R4Vo1Vu")}, - {"http expand", nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { + {"http expand", 0, nil, func(t *testing.T, base *check.Absolute, c *pkg.Cache) { checkTarHTTP(t, base, c, fstest.MapFS{ ".": {Mode: fs.ModeDir | 0700}, diff --git a/internal/rosa/rosa_test.go b/internal/rosa/rosa_test.go index c0b01a31..18d7b99b 100644 --- a/internal/rosa/rosa_test.go +++ b/internal/rosa/rosa_test.go @@ -60,7 +60,7 @@ func getCache(t *testing.T) *pkg.Cache { msg := message.New(log.New(os.Stderr, "rosa: ", 0)) msg.SwapVerbose(true) - if buildTestCache, err = pkg.Open(ctx, msg, 0, a); err != nil { + if buildTestCache, err = pkg.Open(ctx, msg, 0, 0, a); err != nil { t.Fatal(err) } }