verify and cleanup read the manifest of a run of any size

.github/race-weights

@@ -54,7 +54,6 @@ unlisted 0.58
 16.2 TestABuildWithNoWindowInItSaysSoAndKeepsStandardOutputEmpty
 16.0 TestEveryLanguageDescribesEverythingTheProgramCanProduce
 15.4 TestAvifStillFitsWhenTheQualityIsNotTheOneTheCeilingsWereMeasuredAt
-15.2 TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack
 15.1 TestABuildWithoutTheBuildTagsRefusesAndSaysWhy
 14.9 TestAShortcutCannotStartASecondRunDuringTheFirst
 14.6 TestTheLicenceCommandListsWhatTheBinaryLinks

CHANGELOG.md

@@ -33,6 +33,11 @@ because it turns other people's test suites red.
   The same run finished where the container had swap, so it is warned rather
   than refused.
 
+- **`tfg verify` and `tfg cleanup` take `--memory`**, the setting `generate`
+  takes. A manifest listing more files than it lets them check is refused
+  before any file is looked at, with the number of files and how much memory
+  checking them takes.
+
 - **A Docker image.** `ghcr.io/donislawdev/tfg` holds the Linux command line
   of a release and nothing else, for x64 and Arm, and runs with no network. It
   is made from the published archive once its checksum and attestation check
@@ -50,6 +55,19 @@ because it turns other people's test suites red.
   licences are the same. `THIRD-PARTY-NOTICES.md` names the new files and
   their sums.
 
+- **`tfg verify` and `tfg cleanup` refuse a manifest that names a key twice
+  or has anything after it but white space.** This tool never writes either.
+  A manifest with two lists of files used to be acted on by the second one,
+  and now nothing in it is acted on. The refusal names the key, and ends with
+  exit code 5 like any manifest that cannot be read.
+
+### Removed
+
+- **The warning that a manifest will be too large to read back**, on the
+  command line and in the window, and `summary.too_large_to_read_back` in new
+  manifests. `tfg verify` and `tfg cleanup` read a manifest of any size now.
+  The field was optional, and `manifest_version` does not change.
+
 ### Fixed
 
 - **Cancel answers at once, and closing the window during a run no longer
@@ -124,13 +142,16 @@ because it turns other people's test suites red.
   than the size the manifest records. `verify` now reports a file it could not
   even look up, for want of permission or a name too long for the system, as
   `unreadable` with the reason rather than as `missing`.
-- **A run is warned when its manifest will be too large to read back,
-  whatever the format.** The warning came from a fixed cost per file that was
-  low for every format, so a run of about 20 000 small files wrote a manifest
-  `tfg verify` and `tfg cleanup` refuse, with no warning and no
-  `too_large_to_read_back` in it. The manifest's length is now counted, and
-  after the run `generate` corrects what it said before when the two differ.
-  Counting adds about seven seconds to a run or preview of a million files.
+- **`tfg verify` and `tfg cleanup` read the manifest of a run of any size.**
+  They refused one larger than 16 MB, about 20 000 files, so `tfg cleanup`
+  could never remove the files of a larger run. One value in it, such as one
+  file's entry, may still take at most 16 MB. In a container of 1 GB both
+  read the manifest of 600 000 files holding at most about 280 MB. Ctrl+C
+  while it is read stops at once. The files of a large run sharing the
+  directory are no longer called `extra`.
+- **`generate --json` no longer holds a second copy of the manifest in memory
+  to print it.** It is written out a part at a time, the same bytes as the
+  saved manifest.
 - **The free space check counts the manifest and the instructions.** A disk
   with room for the files and not for the manifest took the files and then
   refused the manifest, which left files `tfg cleanup` cannot remove. The

internal/audit/audit.go

@@ -185,7 +185,7 @@ func (d Difference) String() string {
 // EscapeError is a manifest entry that leaves the directory once the links on
 // the way have been followed.
 //
-// Separate from the check manifest.Load already makes, and it has to be. That
+// Separate from the check the manifest reader already makes, and it has to be. That
 // one reads the text of the path and this one asks the filesystem, so a path
 // with no climb in it - "jn/VICTIM.txt" - passes the first and is caught here
 // when "jn" turns out to point somewhere else.
@@ -215,35 +215,15 @@ func (e *EscapeError) Error() string {
 // resolving it per entry impossible to write by accident: there is no directory
 // string in scope to resolve. See core.Boundary for what that is worth in
 // seconds, and observation O117 for the measurement.
-func resolved(b core.Boundary, f manifest.File) (string, error) {
-	full := filepath.Join(b.Dir(), filepath.FromSlash(f.Path))
+func resolved(b core.Boundary, p string) (string, error) {
+	full := filepath.Join(b.Dir(), filepath.FromSlash(p))
 	if b.Escapes(full) {
-		return "", &EscapeError{Dir: b.Dir(), Path: f.Path}
+		return "", &EscapeError{Dir: b.Dir(), Path: p}
 	}
 	return full, nil
 }
 
-// Claimed lists the files a manifest says are on the disk.
-//
-// Two kinds of entry are deliberately not claimed. An entry marked failed
-// describes a file that was never written - reporting it missing would be a
-// false alarm about the one thing the manifest was honest about. And an entry
-// that was not materialised was never meant to reach the disk at all.
-//
-// This is the single reading of the manifest. Both commands use it, so neither
-// can drift into claiming something the other does not.
-func Claimed(m *manifest.Manifest) []manifest.File {
-	var out []manifest.File
-	for _, f := range m.Files {
-		if f.Failed || !f.Materialized {
-			continue
-		}
-		out = append(out, f)
-	}
-	return out
-}
-
-// Verify compares a directory against a manifest.
+// Verify compares a directory against what a manifest claims.
 //
 // skip is the name of the manifest file itself. It is never reported as extra:
 // the manifest usually sits in the directory it describes, and a tool that
@@ -255,8 +235,8 @@ func Claimed(m *manifest.Manifest) []manifest.File {
 // A run that is cancelled reports what it managed to compare and says so
 // through the context error. Reporting "sound" on the strength of half a
 // directory is the one answer this must never give.
-func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) ([]Difference, error) {
-	claimed := Claimed(m)
+func Verify(ctx context.Context, dir string, c manifest.Claims, skip string) ([]Difference, error) {
+	claimed := c.Files
 	boundary := core.NewBoundary(dir)
 
 	present, err := walk(ctx, dir)
@@ -315,7 +295,7 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string)
 		// decided. walk builds these with filepath.Rel, which returns a clean
 		// path, so a comparablePath here is a call that cannot be wrong -
 		// removing it left this guard green. See the comment on comparablePath.
-		if seen[p] || filepath.Base(p) == skip || (m.Run.Instructions != "" && filepath.Base(p) == m.Run.Instructions) {
+		if seen[p] || filepath.Base(p) == skip || (c.Instructions != "" && filepath.Base(p) == c.Instructions) {
 			continue
 		}
 		unclaimed = append(unclaimed, p)
@@ -430,14 +410,14 @@ func comparablePath(p string) string {
 // Shared by Verify and Inspect because both have to refuse the same manifest
 // for the same reason, and because both then hand the answers to workers that
 // must not be able to refuse anything themselves. See parallel.go.
-func claimedPaths(b core.Boundary, files []manifest.File) ([]string, error) {
+func claimedPaths(b core.Boundary, files []manifest.ClaimedFile) ([]string, error) {
 	full := make([]string, len(files))
 	for i, f := range files {
 		// The raw path, never the compared one. Cleaning resolves a parent step
 		// against the text of the path rather than against the disk, and the
 		// two differ exactly where a link sits in the middle - which is the
 		// case core.Boundary exists for.
-		p, err := resolved(b, f)
+		p, err := resolved(b, f.Path)
 		if err != nil {
 			return nil, err
 		}
@@ -453,7 +433,7 @@ func claimedPaths(b core.Boundary, files []manifest.File) ([]string, error) {
 // agreement on its own. No sentinel anybody has to remember, and no pointer
 // per file - a run of ten thousand would allocate ten thousand of them to say
 // "nothing to report" ten thousand times.
-func compare(ctx context.Context, f manifest.File, full string, scratch []byte) Difference {
+func compare(ctx context.Context, f manifest.ClaimedFile, full string, scratch []byte) Difference {
 	got, err := readListed(ctx, full, f.Bytes, scratch)
 	// Only an absent file is missing. Every failure to look used to be called
 	// that, so a file somebody had locked or taken the permissions off was
@@ -482,10 +462,10 @@ func compare(ctx context.Context, f manifest.File, full string, scratch []byte)
 			Got:  fmt.Sprintf("%d B", size),
 		}
 	}
-	if got.sum != f.Hashes.SHA256 {
+	if got.sum != f.SHA256 {
 		return Difference{
 			Kind: WrongHash, Path: f.Path,
-			Want: f.Hashes.SHA256,
+			Want: f.SHA256,
 			Got:  got.sum,
 		}
 	}

internal/audit/cleanup.go

@@ -66,9 +66,9 @@ func (c Candidate) Removable(force bool) bool {
 // It is a separate pass because the default run of cleanup deletes nothing and
 // prints this list. A person gets to read what is about to disappear before it
 // does, and CLI.md section 9 rules out asking them interactively.
-func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate, error) {
+func Inspect(ctx context.Context, dir string, c manifest.Claims) ([]Candidate, error) {
 	boundary := core.NewBoundary(dir)
-	claimed := Claimed(m)
+	claimed := c.Files
 
 	// Nothing is inspected and nothing is offered when one of these points
 	// outside the directory. A list that points outside is not a list this tool
@@ -92,7 +92,7 @@ func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate
 
 // look is what one claimed file comes to for cleanup: whether it may be
 // removed, and if not, why not in the words a person needs.
-func look(ctx context.Context, f manifest.File, full string, scratch []byte) Candidate {
+func look(ctx context.Context, f manifest.ClaimedFile, full string, scratch []byte) Candidate {
 	got, err := readListed(ctx, full, f.Bytes, scratch)
 	if absent(err) {
 		return Candidate{Path: f.Path, Disposition: Absent}
@@ -115,7 +115,7 @@ func look(ctx context.Context, f manifest.File, full string, scratch []byte) Can
 		return Candidate{Path: f.Path, Disposition: Changed,
 			Detail: "it changed while it was being checked"}
 	}
-	if got.sum != f.Hashes.SHA256 {
+	if got.sum != f.SHA256 {
 		return Candidate{Path: f.Path, Disposition: Changed,
 			Detail: "its content is not the content this run wrote"}
 	}
@@ -214,7 +214,7 @@ func Remove(ctx context.Context, dir string, cands []Candidate, force bool) ([]O
 		// separated by however long a person spends reading the preview, and
 		// this is the one operation in the tool that destroys data - so the
 		// question is put to the filesystem in the state it is in now.
-		full, err := resolved(boundary, manifest.File{Path: c.Path})
+		full, err := resolved(boundary, c.Path)
 		if err != nil {
 			return out, err
 		}

internal/audit/neighbours.go

@@ -54,11 +54,22 @@ type neighbourClaims struct {
 // about it here and it keeps whatever verify would have called it. Guessing on
 // a file we could not read is how a tool starts accounting for files nobody
 // wrote.
+//
+// A record is read a piece at a time and only its claims among the candidates
+// are kept, so the record of a run of a million files costs a pass over it and
+// what it says about the files in front of us. It used to be read whole under
+// the sixteen megabytes Load allows, and a record past that was refused - so
+// the files of a run of more than about twenty thousand were called extra in
+// the directory they shared with ours.
 func findNeighbours(ctx context.Context, dir string, candidates []string) neighbourClaims {
 	found := neighbourClaims{
 		records:   map[string]bool{},
 		claimedBy: map[string]string{},
 	}
+	unclaimed := make(map[string]bool, len(candidates))
+	for _, rel := range candidates {
+		unclaimed[rel] = true
+	}
 	for _, rel := range candidates {
 		if ctx.Err() != nil {
 			// A cancelled pass stops looking. The caller reports what it
@@ -68,34 +79,51 @@ func findNeighbours(ctx context.Context, dir string, candidates []string) neighb
 		if !couldBeNamedLikeARecord(rel) {
 			continue
 		}
-		full := filepath.Join(dir, filepath.FromSlash(rel))
-		m, err := manifest.Load(full)
-		if err != nil {
+		claims, ok := claimsAmong(ctx, filepath.Join(dir, filepath.FromSlash(rel)), unclaimed)
+		if !ok {
 			continue
 		}
 		found.records[rel] = true
-		base := filepath.Base(rel)
-		found.take(m, base)
+		found.take(claims, filepath.Base(rel))
 	}
 	return found
 }
 
+// claimsAmong is the files the record at full claims that are among the
+// unclaimed, and whether it is a record this build reads at all. What it claims
+// is kept only once the whole record has been read and found sound, because a
+// record refused at its last byte is no record, whatever its first entries
+// said.
+//
+// Each file once, however many times the record lists it, so what is kept is
+// never more than the files in front of us. A record of a million entries
+// naming one of them would otherwise be a list a million long.
+func claimsAmong(ctx context.Context, full string, unclaimed map[string]bool) ([]string, bool) {
+	var hits []string
+	kept := map[string]bool{}
+	_, err := manifest.EachClaim(ctx, full, func(c manifest.ClaimedFile) {
+		// The path is put through the same spelling both sides of the
+		// comparison use. Without it a neighbour writing "./a.txt" would claim
+		// nothing, which is the fault comparablePath was written for.
+		if key := comparablePath(c.Path); unclaimed[key] && !kept[key] {
+			kept[key] = true
+			hits = append(hits, key)
+		}
+	})
+	return hits, err == nil
+}
+
 // take records what one neighbour says it wrote.
 //
 // Its own function rather than a loop inside a loop, which is the ceiling on
 // how deeply this project nests talking. It is also the better shape to read:
 // the caller decides WHICH files are records, and this decides what a record
 // accounts for.
-func (n neighbourClaims) take(m *manifest.Manifest, base string) {
-	for _, f := range Claimed(m) {
+func (n neighbourClaims) take(claims []string, base string) {
+	for _, key := range claims {
 		// First record wins, and the walk that produced the candidates is in a
 		// fixed order, so two neighbours claiming one file name the same one of
 		// themselves every time rather than whichever was read first on the day.
-		//
-		// The path is put through the same spelling both sides of the
-		// comparison use. Without it a neighbour writing "./a.txt" would claim
-		// nothing, which is the fault comparablePath was written for.
-		key := comparablePath(f.Path)
 		if _, taken := n.claimedBy[key]; !taken {
 			n.claimedBy[key] = base
 		}
@@ -128,13 +156,14 @@ func (n neighbourClaims) take(m *manifest.Manifest, base string) {
 // same day. That one read the first half kilobyte of every candidate and looked
 // for the first key of a manifest, to keep a large JSON document that is not one
 // from being read in full. Nothing could make it fail: a document that got past
-// it was refused by manifest.Load's schema check anyway, so removing it changed
+// it was refused by the manifest's schema check anyway, so removing it changed
 // no answer and no test could tell. A defence nothing can redden is not a
 // defence, and this project takes those out rather than keeping them.
 //
-// What is left is the name, and then manifest.Load, which has a ceiling of its
-// own - so the worst an unclaimed JSON document can cost is one read of at most
-// that ceiling.
+// What is left is the name, and then the manifest reader, which holds at most
+// one value of manifest.MaxValueBytes at a time - so the worst an unclaimed
+// JSON document can cost is one pass over it, and one that is not an object
+// is let go at its first byte.
 func couldBeNamedLikeARecord(rel string) bool {
 	return strings.EqualFold(filepath.Ext(rel), ".json")
 }

internal/cli/claims.go

@@ -0,0 +1,38 @@
+// Part of package cli. See cli.go.
+package cli
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+
+	"github.com/donislawdev/TestingFilesGenerator/internal/core"
+	"github.com/donislawdev/TestingFilesGenerator/internal/engine"
+	"github.com/donislawdev/TestingFilesGenerator/internal/manifest"
+)
+
+// claimsOf reads what the manifest at path claims, for verify and cleanup,
+// under the memory setting, and says on errOut why not when it cannot. When ok
+// is false the command ends here with code.
+//
+// One function for both commands, so they read a manifest under the same
+// limit and refuse it in the same words. stopped is the sentence for Ctrl+C
+// arriving while the manifest is read, which is the one ending the two put
+// differently - one checked nothing, the other removed nothing - and both say
+// so, because a manifest of a million files takes seconds to read and a person
+// who stopped it wants to know what happened to their directory.
+func claimsOf(ctx context.Context, path string, memory core.Memory, stopped string, errOut io.Writer) (claims manifest.Claims, code int, ok bool) {
+	limits := memory.On(core.MachineMemory)
+	engine.HandToRuntime(limits)
+	claims, err := manifest.ReadClaims(ctx, path, limits.Run)
+	if err == nil {
+		return claims, ExitOK, true
+	}
+	if errors.Is(err, context.Canceled) {
+		fmt.Fprintf(errOut, "tfg: %s\n", stopped)
+		return manifest.Claims{}, ExitInterrupted, false
+	}
+	fmt.Fprintf(errOut, "tfg: %s\n", describeError(err))
+	return manifest.Claims{}, classify(err), false
+}