diff --git a/.github/osv-scanner.toml b/.github/osv-scanner.toml index 293960de..a3384e70 100644 --- a/.github/osv-scanner.toml +++ b/.github/osv-scanner.toml @@ -25,3 +25,22 @@ id = "GHSA-84m7-p3x7-pcfv" ignoreUntil = 2026-11-15 reason = "semgrep pins mcp==1.29.0 in every release, and the advisory is about an MCP HTTP server, which semgrep scan never starts" + +# opentelemetry-api 1.37.0, in requirements-semgrep.txt for the scanner and +# nothing else. +# +# semgrep 1.180.0 and 1.181.0 both ask for opentelemetry-api~=1.37.0, read +# from their wheels on 2026-10-26, so no set pip can install holds the fixed +# 1.45.1. certifi, found by the same scan, had no such bound and was raised. +# +# The advisory is about TraceState.from_header taking quadratic time on an +# inbound tracestate header - reached through propagate.extract() in the +# instrumentation of a server receiving requests. semgrep scan sends its +# telemetry out and receives no request, on a runner that lives for one scan. +# +# The same date as the entry above, on purpose: both wait on a semgrep that +# moves its pins, and both come back on one reminder. +[[IgnoredVulns]] +id = "GHSA-xcq7-p3pf-p88h" +ignoreUntil = 2026-11-15 +reason = "semgrep pins opentelemetry-api~=1.37.0 in every release, and the advisory is about parsing an inbound tracestate header in a server, which semgrep scan never runs" diff --git a/.github/requirements-semgrep.txt b/.github/requirements-semgrep.txt index 9bdf43ae..2caef892 100644 --- a/.github/requirements-semgrep.txt +++ b/.github/requirements-semgrep.txt @@ -25,7 +25,7 @@ boltons==21.0.0 # semgrep bracex==3.1 # via wcmatch -certifi==2026.7.22 +certifi==2026.10.23 # via # httpcore # httpx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 129f0115..11e6cdf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ env: # The exact toolchain used for tests and releases. go.mod declares a # minimum - this is the pin. Raising it can change generated bytes, so the # byte stability guard has to be green before it moves. - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" jobs: test: diff --git a/.github/workflows/dev-build.yml b/.github/workflows/dev-build.yml index 25b53c15..2d3a9539 100644 --- a/.github/workflows/dev-build.yml +++ b/.github/workflows/dev-build.yml @@ -51,7 +51,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" # Fourteen days rather than the default ninety. These are throwaway builds # handed to one person, and an unsigned binary should not sit for a quarter of # a year behind a link somebody can pass on as if it were official. diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index efe4759d..3851a5a3 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -42,7 +42,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" # Built as the test job builds on Linux, so a verdict here is about the # guards CI actually runs. Entries on files only a cgo build or only # Windows compiles come back UNJUDGED, said out loud in the report. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c4b14d09..a6349ea9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -29,7 +29,7 @@ concurrency: env: # The same pin CI carries. A release built on a different toolchain than the # one the byte stability guards ran under is a release nobody measured. - GO_VERSION: "1.27.2" + GO_VERSION: "1.27.3" jobs: check: diff --git a/CHANGELOG.md b/CHANGELOG.md index e52c4e17..055dfea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,37 @@ because it turns other people's test suites red. `--user "$(id -u):$(id -g)"` to write there. README also shows a step that installs `tfg` on Windows and macOS runners in GitHub Actions. +### Fixed + +- **`tfg cleanup --yes` keeps a file that changed after it was checked.** + Cleanup checks every file before it removes any, and a file changed in that + time was removed without `--force`. It now looks again just before it + removes each file and keeps one that was written to or replaced, saying so. + A write that keeps the length and lands within the same tick of a coarse + clock can still go unseen - FAT32 counts in two seconds. +- **`tfg verify` and `tfg cleanup` stop part way through a large file on + Ctrl+C**, never wait on a pipe put where a listed file was, and read no more + 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. +- **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 + refusal now says how much the run needs with its manifest. + +### Security + +- **Built with Go 1.27.3.** It fixes a vulnerability in `crypto/x509` of the + Go standard library, which only the desktop window links. The command line + tool was not affected. Building from source now needs Go 1.27.3. + ## [0.5.0] - 2026-10-20 ### Breaking diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 042121e9..e77176b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,7 +22,7 @@ reached yet. ## Building it -Needs Go 1.27.2 or newer, and nothing else for the command line tool. +Needs Go 1.27.3 or newer, and nothing else for the command line tool. ``` git clone https://github.com/donislawdev/TestingFilesGenerator diff --git a/README.md b/README.md index 88689af3..cff2f7ab 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ instead of writing files that disappear with the container. go install -tags noasm github.com/donislawdev/TestingFilesGenerator/cmd/tfg@latest ``` -**From source.** Needs Go 1.27.2 or newer, and nothing else: +**From source.** Needs Go 1.27.3 or newer, and nothing else: ``` git clone https://github.com/donislawdev/TestingFilesGenerator diff --git a/go.mod b/go.mod index 9eea3188..77e84f92 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ module github.com/donislawdev/TestingFilesGenerator // which version it is. Kept honest by TestTheBuildFloorIsThePinnedToolchainAndTheReadmeSaysSo, // which also holds the sentence in README.md against this number. See // docs/STACK.md. -go 1.27.2 +go 1.27.3 // How the line above got to the number it carries. There used to be a separate // "toolchain" directive here saying which compiler produces our binaries, and @@ -75,6 +75,12 @@ go 1.27.2 // guards were run under 1.27.2 the same day - all 104 golden values and every // pinned standard library path - and none shifted, so D11 holds and no major // version is owed. +// +// Raised to 1.27.3 on 2026-10-26 because govulncheck reported GO-2026-6809 in +// crypto/x509, fixed in 1.27.3, and the known vulnerabilities check of every +// pull request went red on it. Only the window binary links crypto/x509 (go +// list -deps, the same day). The byte stability guards were run under 1.27.3 +// before the move, as they were for 1.27.2. require ( fyne.io/fyne/v2 v2.8.2 diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 123a4d20..03c002d6 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -11,11 +11,7 @@ package audit import ( "context" - "crypto/sha256" - "encoding/hex" "fmt" - "io" - "os" "path" "path/filepath" "sort" @@ -38,9 +34,10 @@ const ( WrongSize Kind = "wrong-size" // WrongHash is a file of the right length and the wrong content. WrongHash Kind = "wrong-hash" - // Unreadable is a file that is there and could not be read. It is not - // agreement and it is not a mismatch, and calling it either would be a - // guess. + // Unreadable is a file that is there and could not be read, or a name the + // system would not even look up - no permission, a name too long for it. + // It is not agreement and it is not a mismatch, and calling it either + // would be a guess. Got carries the system's own reason. Unreadable Kind = "unreadable" // Leftover is a file this tool started and never finished, from a run that // was killed before it could tidy up. @@ -294,7 +291,7 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) } found, stopped := InOrder(ctx, len(claimed), func(i int, scratch []byte) Difference { - return compare(claimed[i], full[i], scratch) + return compare(ctx, claimed[i], full[i], scratch) }) for _, d := range found { if d.Kind != "" { @@ -456,31 +453,40 @@ 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(f manifest.File, full string, scratch []byte) Difference { - info, statErr := os.Stat(full) - if statErr != nil { +func compare(ctx context.Context, f manifest.File, 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 + // reported as gone - a sentence about the wrong thing, and the one a + // person acts on (docs/STABILITY-REVIEW-2026-10-26.md, R04). cleanup has + // always told the two apart. + if absent(err) { return Difference{Kind: Missing, Path: f.Path} } + if err != nil { + return Difference{Kind: Unreadable, Path: f.Path, Got: err.Error()} + } // Size first. It is free, it catches the common failure, and it names the - // cause more precisely than a hash mismatch would. - if info.Size() != f.Bytes { + // cause more precisely than a hash mismatch would. A file that grew while + // it was read is the same answer, reached later: at least as long as what + // was read of it, and longer if it went on growing. + size := got.last.Size() + if got.grew(f.Bytes) { + size = max(size, got.read) + } + if size != f.Bytes { return Difference{ Kind: WrongSize, Path: f.Path, Want: fmt.Sprintf("%d B", f.Bytes), - Got: fmt.Sprintf("%d B", info.Size()), + Got: fmt.Sprintf("%d B", size), } } - - sum, hashErr := hashFile(full, scratch) - if hashErr != nil { - return Difference{Kind: Unreadable, Path: f.Path, Got: hashErr.Error()} - } - if sum != f.Hashes.SHA256 { + if got.sum != f.Hashes.SHA256 { return Difference{ Kind: WrongHash, Path: f.Path, Want: f.Hashes.SHA256, - Got: sum, + Got: got.sum, } } return Difference{} @@ -494,41 +500,3 @@ func compare(f manifest.File, full string, scratch []byte) Difference { // told apart, so this is the size that buys it without asking every worker for // a megabyte. const hashScratch = 256 << 10 - -// onlyRead hides everything but Read from io.CopyBuffer. -// -// This is not a wrapper for its own sake and without it the buffer below does -// nothing at all. os.File implements io.WriterTo, so io.CopyBuffer hands the -// whole job to the file and THROWS THE BUFFER AWAY - measured on 2026-09-05, -// passing 128 KiB, 256 KiB or 1 MiB with a plain file on the other end all take -// the same 167-172 ms that io.Copy takes, because all four are the same 32 KiB -// path inside. Hidden behind this, a 256 KiB buffer takes 150-161 ms. -// -// The report that suggested the buffer said it would "cut the syscall count 32 -// fold" and would have changed nothing as written. That is the whole reason -// this type exists rather than a comment saying the buffer is bigger. -type onlyRead struct{ io.Reader } - -// hashFile streams the file rather than reading it in. A run of this tool -// produces files measured in gigabytes, and verify has to survive its own -// output. -// -// scratch belongs to the caller and is reused across every file that caller -// answers for. It may be nil, and then this asks for one of its own - which is -// what a sequential caller wants and what every test that reaches here gets. -func hashFile(path string, scratch []byte) (string, error) { - f, err := os.Open(path) - if err != nil { - return "", err - } - defer func() { _ = f.Close() }() - - if scratch == nil { - scratch = make([]byte, hashScratch) - } - h := sha256.New() - if _, err := io.CopyBuffer(h, onlyRead{f}, scratch); err != nil { - return "", err - } - return hex.EncodeToString(h.Sum(nil)), nil -} diff --git a/internal/audit/cleanup.go b/internal/audit/cleanup.go index c5c5aac3..432d09d4 100644 --- a/internal/audit/cleanup.go +++ b/internal/audit/cleanup.go @@ -44,6 +44,12 @@ type Candidate struct { Path string Disposition Disposition Detail string + + // seen is the file as it was when its content was found to be ours, from + // the handle it was read through. Remove asks the disk again just before + // it deletes and leaves the file alone when the answer differs - see + // changedSince. + seen os.FileInfo } // Removable says whether this candidate is one cleanup will delete. @@ -80,36 +86,71 @@ func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate // In order, because this list is what cleanup removes from and what it // printed to a person beforehand. return InOrder(ctx, len(claimed), func(i int, scratch []byte) Candidate { - return look(claimed[i], full[i], scratch) + return look(ctx, claimed[i], full[i], scratch) }) } // 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(f manifest.File, full string, scratch []byte) Candidate { - info, err := os.Stat(full) - if errors.Is(err, fs.ErrNotExist) { +func look(ctx context.Context, f manifest.File, full string, scratch []byte) Candidate { + got, err := readListed(ctx, full, f.Bytes, scratch) + if absent(err) { return Candidate{Path: f.Path, Disposition: Absent} } if err != nil { return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} } - // Size before hash, so a file of obviously the wrong length does not cost - // a read of however many gigabytes it is. - if info.Size() != f.Bytes { + // Size before hash - readListed reads nothing of a file of the wrong + // length, so one of obviously the wrong length does not cost a read of + // however many gigabytes it is. + if size := got.seen.Size(); size != f.Bytes { return Candidate{Path: f.Path, Disposition: Changed, - Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", info.Size(), f.Bytes)} + Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", size, f.Bytes)} } - sum, err := hashFile(full, scratch) - if err != nil { - return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} + // Changed under the read is changed, whatever the hash came to. Part of + // what was hashed may be what this run wrote and part what somebody wrote + // since, and a match on that says nothing about what is there now. + if got.grew(f.Bytes) || got.moved() { + return Candidate{Path: f.Path, Disposition: Changed, + Detail: "it changed while it was being checked"} } - if sum != f.Hashes.SHA256 { + if got.sum != f.Hashes.SHA256 { return Candidate{Path: f.Path, Disposition: Changed, Detail: "its content is not the content this run wrote"} } - return Candidate{Path: f.Path, Disposition: Ready} + return Candidate{Path: f.Path, Disposition: Ready, seen: got.last} +} + +// changedSince says whether the file at full is no longer the file seen when +// it was checked, and why, in the words a person needs. +// +// Asked just before the file is deleted, and the reason is R01 of the review +// of 2026-10-21 (docs/STABILITY-REVIEW-2026-10-26.md). Inspect reads every +// file before Remove deletes any, so the first file was judged ours and then +// waited for the hashing of all the others - reproduced there with 257 files +// and a change 400 ms in: the changed file went, without --force, under exit +// 0. Asking again closes that wait down to the moment between this look and +// the deletion, which a file somebody is working on is very unlikely to fall +// into and which no portable call closes completely. +// +// Three questions, and each catches what the others do not. The identity is a +// file put in its place. The size is a file appended to. The time is a file +// written in place at the same length, measured on 2026-10-26 on NTFS to move +// at once, even while the writer still holds the file open. What it cannot +// see is a write in place that keeps the length and lands within the same tick +// of a coarse clock - FAT32 counts in two seconds. +func changedSince(full string, seen os.FileInfo) (string, bool) { + now, err := os.Lstat(full) + if err != nil { + // Gone, or no longer answering. Either way os.Remove would have the + // last word, and it is given it - this only stops a deletion. + return "", false + } + if !os.SameFile(seen, now) || now.Size() != seen.Size() || !now.ModTime().Equal(seen.ModTime()) { + return "it changed after it was checked, so it may not be ours. Run cleanup again to check it afresh", true + } + return "", false } // Outcome is what happened to one file. @@ -199,6 +240,14 @@ func Remove(ctx context.Context, dir string, cands []Candidate, force bool) ([]O }) continue } + // Only for a file judged ours. --force already covers a changed file, + // and a file found changed and forced has nothing to compare against. + if c.Disposition == Ready { + if why, changed := changedSince(full, c.seen); changed { + out = append(out, Outcome{Path: c.Path, Blocked: true, Reason: why}) + continue + } + } if err := os.Remove(full); err != nil { if errors.Is(err, fs.ErrNotExist) { // It went away between the two passes. That is the state the diff --git a/internal/audit/parallel.go b/internal/audit/parallel.go index 5469895a..1f14ddbe 100644 --- a/internal/audit/parallel.go +++ b/internal/audit/parallel.go @@ -133,12 +133,19 @@ func drain[T any](ctx context.Context, next *atomic.Int64, out []T, done []bool, for { i := int(next.Add(1)) - 1 // Cancellation is asked per item rather than per pass, so a Ctrl+C - // during a long hash is noticed at the next file rather than at the - // end of the run. + // between files is noticed at the next one - and verify and cleanup + // ask it inside a file as well, while they read it. if i >= len(out) || ctx.Err() != nil { return } out[i] = one(i, scratch) + // An item whose answer was cut short by the cancellation is not + // answered. A read stopped half way comes back as the context's error, + // and counting that would put "could not be read" into the prefix a + // cancelled pass reports, about a file nobody failed to read. + if ctx.Err() != nil { + return + } done[i] = true } } diff --git a/internal/audit/read.go b/internal/audit/read.go new file mode 100644 index 00000000..4d1ade61 --- /dev/null +++ b/internal/audit/read.go @@ -0,0 +1,150 @@ +package audit + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "io" + "io/fs" + "os" + "syscall" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// Reading a file the manifest lists. One way in for verify and cleanup, so +// the two cannot come to read the same file differently - and the way in +// carries three things the plain os.Open it replaced did not, each found by +// review on 2026-10-21 (docs/STABILITY-REVIEW-2026-10-26.md, R01 and R04): +// +// - The file is opened without waiting and only a file is read +// (core.OpenRegular). A name the manifest lists for an empty file and +// somebody swapped for a pipe had the right size, 0, and os.Open then +// waited on it for good. +// - Ctrl+C is asked between pieces, not only between files. A run of this +// tool writes files measured in gigabytes, and the pool asked only at the +// next file, so stopping verify meant waiting out the largest one. +// - No more is read than the manifest records, plus one byte to tell that +// there is more. A file somebody keeps appending to was read until they +// stopped. + +// listed is what reading one listed file found. +type listed struct { + // seen is the file as its handle described it before the first byte was + // read. Taken from the handle rather than the name, because on Windows + // os.Stat reads the identity of a file lazily, by name, at the moment it + // is compared - so two looks by name both describe whatever is there at + // the end and agree about a file that was replaced in between (the review + // of #157, point 4, read from stat_windows.go). Measured on 2026-10-26 on + // NTFS: this side against os.Lstat tells a replaced file of the same size + // and time apart, and a rewrite in place moves the time at once, even + // while the writer still holds the file open. + seen os.FileInfo + // last is the file as its handle described it when the reading ended, and + // the same as seen when nothing was read. + last os.FileInfo + // sum is the SHA-256 of what was read, empty when nothing was. + sum string + // read is how many bytes were read. + read int64 +} + +// absent says an open failed because nothing is at the name. +// +// One answer for every system, and that is what it is for. A listed file +// under a directory that has since become a file is ENOTDIR on Linux and +// macOS and "the path is not found" on Windows, which Go already counts as +// not existing - so without this, verify called the same directory "missing" +// on one machine and "unreadable" on the other (untouchable rule 24). +func absent(err error) bool { + return errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) +} + +// grew says more was read than the manifest records. +func (l listed) grew(want int64) bool { return l.read > want } + +// moved says the file was not the same when the reading ended as when it +// began - its size or its time changed under the read. +func (l listed) moved() bool { + return l.last.Size() != l.seen.Size() || !l.last.ModTime().Equal(l.seen.ModTime()) +} + +// readListed opens a listed file and hashes it, when its size is the size the +// manifest records. A file of another size is not read at all, so a file of +// obviously the wrong length does not cost a read of however many gigabytes it +// is. +// +// scratch belongs to the caller and is reused across every file that caller +// answers for, for the reason written at drain. It may be nil, and then this +// asks for one of its own. +// +// A cancelled read comes back as the context's error and nothing else. It is +// not an answer about the file, and the pool drops it rather than letting it +// stand as one (drain). +func readListed(ctx context.Context, path string, want int64, scratch []byte) (listed, error) { + f, seen, err := core.OpenRegular(path) + if err != nil { + return listed{}, err + } + defer func() { _ = f.Close() }() + + out := listed{seen: seen, last: seen} + if seen.Size() != want { + return out, nil + } + if scratch == nil { + scratch = make([]byte, hashScratch) + } + // One past what is recorded, so a file that is still being written to + // shows as longer rather than as the right length with the wrong content. + h := sha256.New() + if out.read, err = HashAtMost(ctx, f, want+1, h, scratch); err != nil { + return listed{}, err + } + out.sum = hex.EncodeToString(h.Sum(nil)) + + if out.last, err = f.Stat(); err != nil { + return listed{}, err + } + return out, nil +} + +// HashAtMost feeds up to limit bytes of r to h, a buffer at a time, and asks +// about Ctrl+C before every one. +// +// Exported for the guard that asks both of those of it directly. Reaching them +// through verify needs a read that is still going when the cancellation lands, +// and the only way to arrange that from outside is a clock - which is a guard +// that is slow on a slow machine and green for the wrong reason on a fast one. +// +// A loop of its own rather than io.CopyBuffer. The buffer size is measured - +// hashing a 256 MB file, median of five on 2026-09-05: io.Copy 199 ms, 64 KiB +// 182 ms, 256 KiB 161 ms, 1 MiB 162 ms - and io.CopyBuffer threw it away for an +// os.File, which implements io.WriterTo and takes the copy over with 32 KiB of +// its own. So the loop is what makes the buffer count, as well as what lets a +// read be stopped part way. +func HashAtMost(ctx context.Context, r io.Reader, limit int64, h io.Writer, scratch []byte) (int64, error) { + var read int64 + for read < limit { + if err := ctx.Err(); err != nil { + return read, err + } + piece := scratch + if left := limit - read; left < int64(len(piece)) { + piece = piece[:left] + } + n, err := r.Read(piece) + if n > 0 { + _, _ = h.Write(piece[:n]) // a hash never refuses a write + read += int64(n) + } + if errors.Is(err, io.EOF) { + return read, nil + } + if err != nil { + return read, err + } + } + return read, nil +} diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 56b3ede3..8c76a9a1 100644 --- a/internal/cli/generate.go +++ b/internal/cli/generate.go @@ -423,7 +423,8 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g core.ExactBytes(engine.TotalBytes(planned))) echoBoundaries(targets, planned, errOut) - echoManifestReach(planned, errOut) + forecast, warned := echoManifestReach(ctx, planned, opt, errOut) + opt.Record = forecast if g.dryRun { fmt.Fprintln(errOut, "dry run - nothing was written.") @@ -455,13 +456,13 @@ func produce(ctx context.Context, targets []engine.Target, opt engine.Options, g // Writing one would replace the record of whatever was already in the // directory, and that record is the only thing cleanup can work from. if !g.dryRun && res.Started { - saveManifest(res, opt, errOut) + saveManifest(res, opt, warned, errOut) } return classify(runErr) } if !g.dryRun { - if code := saveManifest(res, opt, errOut); code != ExitOK { + if code := saveManifest(res, opt, warned, errOut); code != ExitOK { return code } } diff --git a/internal/cli/record.go b/internal/cli/record.go index 324b9db5..69c0982a 100644 --- a/internal/cli/record.go +++ b/internal/cli/record.go @@ -80,26 +80,61 @@ func whatSurvived(runErr error, res *engine.Result) string { // today. What was missing was that nobody was told. It is printed before the // first byte and on a dry run, which is the step this project tells people to // take before anything large. -func echoManifestReach(planned []engine.PlannedFile, errOut io.Writer) { - noted := 0 - for _, f := range planned { - if len(f.Plan.Notes) > 0 { - noted++ - } +// +// It hands back the forecast, for the run to use rather than work out a second +// time, and whether it said anything, so that what the run really wrote can +// correct it afterwards (echoManifestOutcome). The forecast is the manifest of +// a run in which every file is produced - since 2026-10-26 counted entry by +// entry rather than from a constant that was low for every format, so a run of +// 20 500 text files is no longer told nothing and then left with a manifest +// verify refuses (docs/STABILITY-REVIEW-2026-10-26.md, section 2.1). +func echoManifestReach(ctx context.Context, planned []engine.PlannedFile, opt engine.Options, errOut io.Writer) (*engine.RecordSize, bool) { + record, err := engine.PredictRecord(ctx, planned, opt) + if err != nil { + // A forecast that could not be made is not a reason to stop the run + // here. The run works it out again and answers for it, and what was + // written is measured again after the save and said then. + return nil, false } - size, over := manifest.TooLargeToReadBack(len(planned), noted) - if !over { - return + if !record.TooLarge { + return &record, false } fmt.Fprintf(errOut, "note: this run will write a manifest of roughly %s and this build reads at most %s, "+ "so tfg verify and tfg cleanup will not be able to read it. Split the run to keep each manifest readable.\n", - core.HumanBytes(size), core.HumanBytes(manifest.MaxBytes)) + core.HumanBytes(record.Manifest), core.HumanBytes(manifest.MaxBytes)) + return &record, true +} + +// echoManifestOutcome says what the manifest came to, when that is not what +// the run was told before its first byte. +// +// The forecast is right for a run in which every file is produced. A file that +// fails gains a note in its entry, so the manifest can grow past the ceiling +// after the forecast said it would not - and then this is the only sentence +// that says so. The other way round, a run warned and then within the ceiling, +// is corrected too: an alarm left standing after the fact is a sentence the +// person acts on, and it would be false. +func echoManifestOutcome(written engine.RecordSize, warned bool, errOut io.Writer) { + switch { + case written.TooLarge && !warned: + fmt.Fprintf(errOut, + "note: this run wrote a manifest of %s and this build reads at most %s, "+ + "so tfg verify and tfg cleanup will not be able to read it. Split the run to keep each manifest readable.\n", + core.HumanBytes(written.Manifest), core.HumanBytes(manifest.MaxBytes)) + case !written.TooLarge && warned: + fmt.Fprintf(errOut, + "note: the manifest came to %s, within the %s this build reads, so tfg verify and tfg cleanup can read it after all.\n", + core.HumanBytes(written.Manifest), core.HumanBytes(manifest.MaxBytes)) + } } // saveManifest saves the record of a run - the manifest, and the instructions // beside it when any file was given a purpose - and says where each went. -func saveManifest(res *engine.Result, opt engine.Options, errOut io.Writer) int { +// +// warned is whether the run was told before its first byte that its manifest +// would be too large to read back - see echoManifestOutcome. +func saveManifest(res *engine.Result, opt engine.Options, warned bool, errOut io.Writer) int { // Asked of the engine rather than joined here. The engine claimed this // exact name before the first file was written, so working it out a second // way is a chance for the saver and the claim to mean different files. @@ -137,5 +172,6 @@ func saveManifest(res *engine.Result, opt engine.Options, errOut io.Writer) int "tfg: cannot write the instructions to %s: %s. The manifest was saved and holds the same facts.\n", core.Shown(rec.Missed.Path), describeError(rec.Missed.Err)) } + echoManifestOutcome(rec.Size, warned, errOut) return ExitOK } diff --git a/internal/tool/open.go b/internal/core/openregular.go similarity index 62% rename from internal/tool/open.go rename to internal/core/openregular.go index 1f5484f7..87de9d73 100644 --- a/internal/tool/open.go +++ b/internal/core/openregular.go @@ -1,17 +1,21 @@ -package tool +package core import "os" -// Opening a file a tool reads. One way in for every tool, so a file named on -// its own, the thousandth file of a folder and an archive somebody was sent -// are opened with the same care - the review of #157 found a pipe swapped in -// between two looks at a name, and the answer to that is here rather than in -// each tool (docs/NARZEDZIA-SUMY-2026-09-29.md §13.4). Moved out of the -// checksum tool on 2026-10-19, when a second tool came to read files. +// Opening a file that is read rather than written. One way in for every +// reader, so a file named on its own, the thousandth file of a folder, an +// archive somebody was sent and a file a manifest lists are opened with the +// same care - the review of #157 found a pipe swapped in between two looks at a +// name, and the answer to that is here rather than in each reader +// (docs/NARZEDZIA-SUMY-2026-09-29.md §13.4). Moved out of the checksum tool on +// 2026-10-19, when a second tool came to read files, and out of internal/tool +// into core on 2026-10-26, when verify and cleanup came to need it: they sit +// a layer below the tools and could not reach it there +// (docs/STABILITY-REVIEW-2026-10-26.md, R04). -// NotRegularError is a path that names something other than a file. Each tool -// says it in its own words, wrapping this, so a check that every tool refuses -// a pipe asks for this one type. +// NotRegularError is a path that names something other than a file. Each +// reader says it in its own words, wrapping this, so a check that every reader +// refuses a pipe asks for this one type. type NotRegularError struct { Path string Directory bool diff --git a/internal/tool/open_unix.go b/internal/core/openregular_unix.go similarity index 98% rename from internal/tool/open_unix.go rename to internal/core/openregular_unix.go index a3b5e9bf..d4cb437c 100644 --- a/internal/tool/open_unix.go +++ b/internal/core/openregular_unix.go @@ -1,6 +1,6 @@ //go:build !windows -package tool +package core import ( "os" diff --git a/internal/tool/open_windows.go b/internal/core/openregular_windows.go similarity index 97% rename from internal/tool/open_windows.go rename to internal/core/openregular_windows.go index b8143393..f3b83a67 100644 --- a/internal/tool/open_windows.go +++ b/internal/core/openregular_windows.go @@ -1,6 +1,6 @@ //go:build windows -package tool +package core import "os" diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 9df4428f..ad84d819 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -9,14 +9,12 @@ import ( "io/fs" "os" "path/filepath" - "runtime" "strings" "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/damage" "github.com/donislawdev/TestingFilesGenerator/internal/format" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" - "github.com/donislawdev/TestingFilesGenerator/internal/version" ) // Target is one group of files to produce. @@ -147,6 +145,14 @@ type Options struct { // on a handful of files, and the real number is asked about separately. MaxPlanBytes int64 + // Record is the forecast of this run's record, when the caller has already + // worked it out for these same files with PredictRecord - the command line + // does, to warn before the first byte. A forecast is a pass over every + // entry, about seven seconds for a million files measured on 2026-10-26, + // and asked twice it doubled a dry run of that size. Nil means the run + // works it out. + Record *RecordSize + // OnProgress is called as the run advances. Nil means silence, which is // what every caller that has nobody to show it to should pass. // @@ -201,6 +207,12 @@ type Result struct { // the notes in its own language from here, and the manifest stays English. NotesSaid map[string]core.Said + // Record is how large the record of this run was worked out to be before + // its first byte, the manifest and the instructions (PredictRecord). For a + // dry run that is the answer about the run it previews. A run that is + // saved knows the real length afterwards, in Record of SaveRecord. + Record RecordSize + // reservation is the run's hold on its manifest name, taken before the // first file and handed to SaveRecord, which saves through it. Nil for a // dry run and for a run refused before it reserved anything. @@ -468,21 +480,19 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF // it may start at all, and the reading back of the answers in the order the // plan lists them. func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) { - m := manifest.New( - "testing-files-generator", version.Version, - runID(opt.Seed), opt.Command, opt.Seed, - runtime.GOOS, runtime.GOARCH, - ) - m.Run.RecipeHash = opt.RecipeHash - m.Run.Overrides = opt.Overrides - m.Run.Preset = opt.Preset + m := newManifest(opt) res := &Result{Manifest: m} // Whether this run may start at all is settled before anything is written // and before a dry run returns. A dry run exists to count and show before // the disk is touched, so reporting success for a run that refuses to // start on the very next line answers the wrong question. - if err := preflight(ctx, files, opt); err != nil { + record, err := opt.forecast(ctx, files) + if err != nil { + return res, err + } + res.Record = record + if err := preflight(ctx, files, opt, record); err != nil { return res, err } @@ -495,6 +505,10 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) m.Add(res.entryFor(f, "", false, nil)) } m.Run.Complete = true + // The preview's document has no hashes and is shorter than the one the + // run writes, so what it says about being read back is said about the + // run, from the record worked out for it. + m.Summary.TooLargeToReadBack = record.TooLarge return res, nil } diff --git a/internal/engine/errors.go b/internal/engine/errors.go index f36ac80c..8a227662 100644 --- a/internal/engine/errors.go +++ b/internal/engine/errors.go @@ -214,7 +214,7 @@ func (e *SpaceError) Error() string { return e.Said().String() } // Said is the refusal, for a window that says it in its own language. func (e *SpaceError) Said() core.Said { return core.Says("engine.NoSpace", - "this run needs %d B and %s has %d B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory", + "this run needs %d B with its manifest and %s has %d B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory", core.A("Needed", e.Needed), core.A("Dir", core.Shown(e.Path)), core.A("Free", e.Available)) } diff --git a/internal/engine/preflight.go b/internal/engine/preflight.go index 668b3a56..2621bf8f 100644 --- a/internal/engine/preflight.go +++ b/internal/engine/preflight.go @@ -32,11 +32,19 @@ const DefaultManifestName = "manifest.json" // // Both checks used to sit after the dry run had already returned, so // --dry-run reported success for runs that would refuse to start. -func preflight(ctx context.Context, files []PlannedFile, opt Options) error { +func preflight(ctx context.Context, files []PlannedFile, opt Options, record RecordSize) error { // Free space first. Finding out at file five thousand of ten thousand // leaves a half written set and a full disk on a machine somebody works on. - needed := TotalBytes(files) - if available, err := opt.availableBytes(opt.OutDir); err == nil { + // + // The record counts as well as the files, since 2026-10-26. A disk with + // room for the files and not for the manifest took the files and refused + // the manifest, which left files nothing can remove - the manifest is the + // only authority over that (docs/STABILITY-REVIEW-2026-10-26.md, R08). + // What it cannot promise is the space somebody else takes while the run + // goes. Rounding every file up to a cluster is NOT added: NTFS keeps a small + // file inside its directory record, so it would refuse runs that fit. + needed, sumErr := core.AddSizes(TotalBytes(files), record.Bytes()) + if available, err := opt.availableBytes(opt.OutDir); err == nil && sumErr == nil { if available < needed { return &SpaceError{Needed: needed, Available: available, Path: opt.OutDir} } diff --git a/internal/engine/record.go b/internal/engine/record.go index 6dd375f4..3a07de53 100644 --- a/internal/engine/record.go +++ b/internal/engine/record.go @@ -19,6 +19,10 @@ type Record struct { // not failed by it - the manifest was saved and holds the same facts - and // the caller says it out loud rather than letting it pass. Missed *InstructionsError + // Size is what was written, measured on the way rather than worked out + // beforehand: Result.Record is the forecast, this is the fact. Its + // TooLarge is the field the saved manifest carries. + Size RecordSize } // InstructionsError is instructions that were due and could not be written. @@ -60,6 +64,7 @@ func SaveRecord(res *Result, opt Options) (Record, error) { } else { instructions = own rec.Instructions = path + rec.Size.Instructions = int64(len(text)) m.Run.Instructions = filepath.Base(path) } } @@ -68,9 +73,11 @@ func SaveRecord(res *Result, opt Options) (Record, error) { // The file this call wrote, and nothing that took its name since. _ = core.RemoveOwn(rec.Instructions, instructions) rec.Instructions, m.Run.Instructions = "", "" + rec.Size.Instructions = 0 } return rec, err } + rec.Size.Manifest, rec.Size.TooLarge = m.SavedBytes(), m.Summary.TooLargeToReadBack return rec, nil } diff --git a/internal/engine/recordsize.go b/internal/engine/recordsize.go new file mode 100644 index 00000000..8036704a --- /dev/null +++ b/internal/engine/recordsize.go @@ -0,0 +1,131 @@ +package engine + +import ( + "context" + "path/filepath" + "runtime" + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" + "github.com/donislawdev/TestingFilesGenerator/internal/version" +) + +// How large the record of a run will be, worked out from the plan before a +// byte is written (docs/STABILITY-REVIEW-2026-10-26.md, R03 and R08). +// +// Two things ask it. The free space check: until 2026-10-26 it held the free +// space against the files alone, so a disk with room for the files and not +// for the manifest took the files and refused the manifest - files nothing can +// remove, because the manifest is the only authority over that (untouchable +// rule 7). And the warning that a manifest will be too large to read back, +// which came from a constant per entry that was low for every format. + +// RecordSize is the record a run will write beside its files. +type RecordSize struct { + // Manifest is the length of the manifest. + Manifest int64 + // TooLarge says this build would refuse to read that manifest back. + TooLarge bool + // Instructions is the length of the instructions, nought when no file was + // given a purpose and none are written. + Instructions int64 +} + +// Bytes is the whole record. +func (r RecordSize) Bytes() int64 { return r.Manifest + r.Instructions } + +// hashPlaceholder stands in for a hash not worked out yet. Every hash a run +// records is this long, so the document is as long as the one the run writes. +var hashPlaceholder = strings.Repeat("0", 64) + +// PredictRecord is the record a run of these files writes when every file is +// produced. A file that fails gains a note in its entry, so a run with +// failures writes a little more than this says - which is why the surfaces say +// again, from what was written, when the two disagree. +// +// Entries are counted one at a time and not kept (manifest.Projection). The +// instructions are the exception: what they say depends on every file at once, +// so for a run that gives files a purpose the document is built whole to write +// them. Runs that do are presets and recipes written by hand, not the runs of +// a million files that a whole document would weigh on. +// +// Ctrl+C is asked as it goes. It runs before the first byte, where stopping is +// expected to be immediate, and over a million files it is a pass worth +// stopping - the shape R05 of the review of 2026-10-21 found in the drawing of +// sizes, which this must not repeat. +func PredictRecord(ctx context.Context, files []PlannedFile, opt Options) (RecordSize, error) { + scratch := &Result{} + header := newManifest(opt) + header.Run.Complete = true + counted := header.Projection() + explains := false + for i, f := range files { + if err := askedToStop(ctx, i); err != nil { + return RecordSize{}, err + } + e := scratch.entryFor(f, hashPlaceholder, true, nil) + explains = explains || e.Purpose != "" + counted.Add(e) + } + var out RecordSize + var err error + if explains { + header.Run.Instructions = filepath.Base(InstructionsPath(opt)) + if out.Instructions, err = predictInstructions(ctx, files, opt, scratch); err != nil { + return RecordSize{}, err + } + } + out.Manifest, out.TooLarge, err = counted.Bytes() + return out, err +} + +// predictInstructions is the length of the instructions a run of these files +// writes. They are written from the whole document, so it is built here. +func predictInstructions(ctx context.Context, files []PlannedFile, opt Options, scratch *Result) (int64, error) { + whole := newManifest(opt) + for i, f := range files { + if err := askedToStop(ctx, i); err != nil { + return 0, err + } + whole.Add(scratch.entryFor(f, hashPlaceholder, true, nil)) + } + return int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))), nil +} + +// askedToStop is Ctrl+C, asked before the first entry and then once every +// askEvery entries. +func askedToStop(ctx context.Context, i int) error { + if i%askEvery != 0 { + return nil + } + return ctx.Err() +} + +// forecast is the record the caller handed in, or the one worked out here +// when it handed in none. +func (o Options) forecast(ctx context.Context, files []PlannedFile) (RecordSize, error) { + if o.Record != nil { + return *o.Record, nil + } + return PredictRecord(ctx, files, o) +} + +// askEvery is how many entries go by between two questions about Ctrl+C. An +// entry costs a few microseconds, so this answers within a few milliseconds +// and asks a thousand times less often than there are entries. +const askEvery = 1024 + +// newManifest is the empty manifest of a run with these options. One place, +// so the record a run writes and the record PredictRecord measures start from +// the same header. +func newManifest(opt Options) *manifest.Manifest { + m := manifest.New( + "testing-files-generator", version.Version, + runID(opt.Seed), opt.Command, opt.Seed, + runtime.GOOS, runtime.GOARCH, + ) + m.Run.RecipeHash = opt.RecipeHash + m.Run.Overrides = opt.Overrides + m.Run.Preset = opt.Preset + return m +} diff --git a/internal/guard/auditpipe_unix_test.go b/internal/guard/auditpipe_unix_test.go new file mode 100644 index 00000000..094d71f8 --- /dev/null +++ b/internal/guard/auditpipe_unix_test.go @@ -0,0 +1,71 @@ +//go:build !windows + +package guard + +import ( + "context" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// TestVerifyAndCleanupNeverWaitOnAPipe puts a pipe where a run left an empty +// file. The size the manifest records is 0 and a pipe's size is 0, so the size +// check let it through, and os.Open then waited on it until something wrote to +// it - which may be never (docs/STABILITY-REVIEW-2026-10-26.md, R04). Both +// commands read listed files through core.OpenRegular since 2026-10-26, which +// opens without waiting and turns away what is not a file. +// +// An empty file is the case and not a convenience: every preset that probes +// "nothing at all" lists one, and any other size the pipe fails at once. +func TestVerifyAndCleanupNeverWaitOnAPipe(t *testing.T) { + out := filepath.Join(t.TempDir(), "out") + if code, _, errOut := run(t, "generate", "--format", "txt", "--size", "0B", "--clean", "--out", out); code != 0 { + t.Fatalf("generate gave %d:\n%s", code, errOut) + } + mf := filepath.Join(out, "manifest.json") + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + if len(m.Files) != 1 || m.Files[0].Bytes != 0 { + t.Fatalf("the run is not one empty file, so this is not the case it is for: %+v", m.Files) + } + full := filepath.Join(out, m.Files[0].Path) + if err := os.Remove(full); err != nil { + t.Fatalf("removing the empty file: %v", err) + } + if err := syscall.Mkfifo(full, 0o600); err != nil { + t.Fatalf("making a pipe in its place: %v", err) + } + + answersWithoutWaiting(t, "verify", func() bool { + diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + return err == nil && len(diffs) == 1 && diffs[0].Kind == audit.Unreadable + }) + answersWithoutWaiting(t, "cleanup", func() bool { + cands, err := audit.Inspect(context.Background(), out, m) + return err == nil && len(cands) == 1 && cands[0].Disposition == audit.Unreachable + }) +} + +// answersWithoutWaiting holds one command to an answer within ten seconds, and +// to the right one. +func answersWithoutWaiting(t *testing.T, name string, ask func() bool) { + t.Helper() + answered := make(chan bool, 1) + go func() { answered <- ask() }() + select { + case right := <-answered: + if !right { + t.Errorf("%s answered about the pipe, but not that it cannot read it", name) + } + case <-time.After(10 * time.Second): + t.Fatalf("%s is still waiting on the pipe after ten seconds - it opened it", name) + } +} diff --git a/internal/guard/cleanuprecheck_test.go b/internal/guard/cleanuprecheck_test.go new file mode 100644 index 00000000..d0e11292 --- /dev/null +++ b/internal/guard/cleanuprecheck_test.go @@ -0,0 +1,252 @@ +package guard + +import ( + "context" + "crypto/sha256" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// Cleanup judges every file before it removes any, so the first file judged +// ours then waited for the hashing of all the others - and a change made to it +// in that wait was deleted without --force, under exit 0. Reproduced by an +// outside review on 2026-10-21 with 257 files and a change 400 ms in +// (docs/STABILITY-REVIEW-2026-10-26.md, R01). +// +// These ask it at the seam that wait lives in: Inspect has answered, Remove +// has not begun. No clock is involved, so the answer does not depend on how +// fast the machine hashes. + +// inspected runs a small generate and judges it for cleanup, and hands back the +// directory, the candidates and the name of the first file. +func inspected(t *testing.T) (string, []audit.Candidate, string) { + t.Helper() + out, mf := generated(t) + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + victim := firstGenerated(t, out) + // Pushed an hour back before it is judged, so the write below lands on a + // later time even where the clock moves in steps of several milliseconds - + // a write in the same step is the limit changedSince names, and a guard + // that sometimes landed in it would be red for no fault. + past := time.Now().Add(-time.Hour) + if err := os.Chtimes(filepath.Join(out, victim), past, past); err != nil { + t.Fatalf("moving the time of %s back: %v", victim, err) + } + cands, err := audit.Inspect(context.Background(), out, m) + if err != nil { + t.Fatalf("inspecting: %v", err) + } + // Asserted rather than assumed: a candidate that is not ready is never + // asked again, so the question below would be answered by the old rule. + for _, c := range cands { + if c.Disposition != audit.Ready { + t.Fatalf("%s was judged %s before anything was changed, so the guard is not where it means to be", c.Path, c.Disposition) + } + } + return out, cands, victim +} + +// keptAsChanged asks that Remove left the victim on the disk with a reason that +// says why, and took the rest - the rest is what shows Remove ran at all. +func keptAsChanged(t *testing.T, out, victim string, outcomes []audit.Outcome) { + t.Helper() + if _, err := os.Stat(filepath.Join(out, victim)); err != nil { + t.Fatalf("%s changed after it was checked and cleanup removed it anyway: %v", victim, err) + } + removed := 0 + for _, o := range outcomes { + switch { + case o.Path == victim: + if !o.Blocked || !strings.Contains(o.Reason, "changed after it was checked") { + t.Errorf("%s was kept but the report does not say so and why: %+v", victim, o) + } + case o.Removed: + removed++ + } + } + if removed != len(outcomes)-1 { + t.Errorf("%d of the %d files nobody touched were removed", removed, len(outcomes)-1) + } +} + +// A file written over in place, at its own length, after it was judged ours. +func TestCleanupLeavesAFileThatChangedAfterItWasChecked(t *testing.T) { + out, cands, victim := inspected(t) + full := filepath.Join(out, victim) + + body, err := os.ReadFile(full) + if err != nil { + t.Fatalf("reading: %v", err) + } + body[0] ^= 0xFF + if err := os.WriteFile(full, body, 0o644); err != nil { + t.Fatalf("writing: %v", err) + } + + outcomes, err := audit.Remove(context.Background(), out, cands, false) + if err != nil { + t.Fatalf("removing: %v", err) + } + keptAsChanged(t, out, victim, outcomes) +} + +// A file put in the place of the one judged ours, with its size and its time. +// Only the identity tells the two apart, and on Windows only an identity read +// from the handle does - os.Stat there reads it lazily, by name, so two looks +// by name agree about whatever is there at the end (the review of #157, +// point 4). +func TestCleanupLeavesAFileReplacedAfterItWasChecked(t *testing.T) { + out, cands, victim := inspected(t) + full := filepath.Join(out, victim) + + before, err := os.Lstat(full) + if err != nil { + t.Fatalf("looking at %s: %v", victim, err) + } + body, err := os.ReadFile(full) + if err != nil { + t.Fatalf("reading: %v", err) + } + body[0] ^= 0xFF + // Made before the original goes, so the filesystem cannot hand the new + // file the number the old one had. + swap := filepath.Join(out, "swap.tmp") + if err := os.WriteFile(swap, body, 0o644); err != nil { + t.Fatalf("writing the replacement: %v", err) + } + if err := os.Chtimes(swap, before.ModTime(), before.ModTime()); err != nil { + t.Fatalf("giving the replacement the old time: %v", err) + } + if err := os.Remove(full); err != nil { + t.Fatalf("removing the original: %v", err) + } + if err := os.Rename(swap, full); err != nil { + t.Fatalf("putting the replacement in its place: %v", err) + } + // Asserted, so a replacement that differed in size or time cannot make + // this green for a reason other than the identity. + after, err := os.Lstat(full) + if err != nil { + t.Fatalf("looking at the replacement: %v", err) + } + if after.Size() != before.Size() || !after.ModTime().Equal(before.ModTime()) { + t.Fatalf("the replacement is %d B at %v and the original was %d B at %v, so this is not the case it is for", + after.Size(), after.ModTime(), before.Size(), before.ModTime()) + } + + outcomes, err := audit.Remove(context.Background(), out, cands, false) + if err != nil { + t.Fatalf("removing: %v", err) + } + keptAsChanged(t, out, victim, outcomes) +} + +// A name the system would not even look up is not a file that is missing. +// Every failure to look was called "missing" until 2026-10-26, so a file +// somebody had taken the permissions off read as gone (R04, the third of the +// further points). A name longer than any filesystem holds is the one way to +// that failure that is the same on every system: ENAMETOOLONG on Linux and +// macOS, "the syntax is incorrect" on Windows - measured on Windows the same +// day, where the build before this said missing. +func TestVerifyCallsANameItCouldNotLookUpUnreadableRatherThanMissing(t *testing.T) { + out, mf := generated(t) + m, err := manifest.Load(mf) + if err != nil { + t.Fatalf("loading the manifest: %v", err) + } + long := strings.Repeat("a", 300) + ".txt" + m.Files[0].Path, m.Files[0].Name = long, long + if err := os.Remove(mf); err != nil { + t.Fatalf("making room for the edited manifest: %v", err) + } + if err := m.Save(mf); err != nil { + t.Fatalf("saving the edited manifest: %v", err) + } + + diffs, err := audit.Verify(context.Background(), out, m, filepath.Base(mf)) + if err != nil { + t.Fatalf("verifying: %v", err) + } + for _, d := range diffs { + if d.Path != long { + continue + } + if d.Kind != audit.Unreadable || d.Got == "" { + t.Errorf("a name the system would not look up was reported as %s with %q, not as unreadable with the reason", d.Kind, d.Got) + } + return + } + t.Fatalf("the edited entry was not reported at all: %v", diffs) +} + +// cancellingReader stands for a large file. It hands out bytes for ever and +// calls the cancellation the first time it is read from. +type cancellingReader struct { + cancel func() + reads int +} + +func (r *cancellingReader) Read(p []byte) (int, error) { + r.reads++ + if r.reads == 1 { + r.cancel() + } + return len(p), nil +} + +// Ctrl+C in the middle of a large file stops the reading of it. The pool asked +// only between files, so stopping verify meant waiting out the largest one +// (R04). One read after the cancellation and no more: the question is put +// before every buffer. +func TestHashingAListedFileStopsPartWayWhenCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + r := &cancellingReader{cancel: cancel} + + _, err := audit.HashAtMost(ctx, r, 64<<20, sha256.New(), make([]byte, 256<<10)) + if !errors.Is(err, context.Canceled) { + t.Errorf("a cancelled read came back with %v, not the cancellation", err) + } + if r.reads != 1 { + t.Errorf("the file was read %d times after Ctrl+C had landed on the first read, and 256 is all of it", r.reads) + } +} + +// endlessReader stands for a file somebody keeps appending to. It refuses to go +// past a point no reasonable limit reaches, so a read with no limit ends with +// that refusal rather than never. +type endlessReader struct{ read int64 } + +func (r *endlessReader) Read(p []byte) (int, error) { + if r.read > 64<<20 { + return 0, errors.New("read past any limit this guard sets") + } + r.read += int64(len(p)) + return len(p), nil +} + +// No more is read than asked. A file that went on growing was read until it +// stopped (R04), and a listed file is asked for its recorded size and one byte +// more - the byte that says it is longer. +func TestHashingAListedFileReadsNoMoreThanItIsAskedFor(t *testing.T) { + const limit = 1000 + 1 + r := &endlessReader{} + read, err := audit.HashAtMost(context.Background(), r, limit, io.Discard, make([]byte, 256<<10)) + if err != nil { + t.Fatalf("reading: %v", err) + } + if read != limit || r.read != limit { + t.Errorf("asked for %d B, it took %d and the file handed out %d", limit, read, r.read) + } +} diff --git a/internal/guard/manifestreach_test.go b/internal/guard/manifestreach_test.go index d1588325..b33b4c49 100644 --- a/internal/guard/manifestreach_test.go +++ b/internal/guard/manifestreach_test.go @@ -4,12 +4,14 @@ import ( "bytes" "context" "encoding/json" + "errors" "os" "path/filepath" "strings" "testing" "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) @@ -18,8 +20,7 @@ import ( // // Two ceilings were chosen independently and never held against each other. // core.MaxFilesPerRun is a million files. manifest.MaxBytes is sixteen -// megabytes, and its comment works out that this "holds a run of roughly twenty -// thousand files". +// megabytes. // // Measured on 2026-08-26: a run of 25 000 files wrote a manifest of 25 220 640 // B, and from that moment "tfg verify" and "tfg cleanup" both exited 5 on it @@ -31,19 +32,22 @@ import ( // The owner's decision on the same day was a note rather than a refusal: the // run works, and refusing it would take away something this tool does today. // What was missing was that nobody was told. -func TestARunSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { - // Just over the ceiling, worked out from the estimate rather than from a - // number written here - a guard carrying its own copy of a limit goes - // stale the day somebody changes the real one and says nothing while it - // does. - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { - t.Fatalf("%d entries was not judged too large, so this proved nothing", over) - } - said := runCLI(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(over), "--dry-run", "--out", t.TempDir()) +// inTheSilentBand is a run that was told nothing until 2026-10-26. +// +// The warning came from 786 B an entry, and a text file of one byte with no +// label costs about 838 - so 20 500 of them came to 16 113 000 B by the +// estimate, under the ceiling, and wrote 17 170 219 B, over it +// (docs/STABILITY-REVIEW-2026-10-26.md, section 2.1). A run here is a run the +// old arithmetic let through in silence, which is the run these guards are for. +var inTheSilentBand = []string{"generate", "--format", "txt", "--size", "1b", "--clean", "--count", "20500", "--dry-run"} +func TestARunSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { + said := runCLI(t, append(inTheSilentBand, "--out", t.TempDir())...) + + if !strings.Contains(said, "will not be able to read") { + t.Fatalf("a run whose manifest will be past the ceiling was told nothing:\n%s", firstLines(said, 6)) + } if !strings.Contains(said, "verify") || !strings.Contains(said, "cleanup") { t.Errorf("the note does not name the two commands that stop working:\n%s", said) } @@ -66,46 +70,54 @@ func TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling(t *testing.T) { } } -// The estimate has to stay near what a manifest really weighs. +// The length worked out for a manifest is the length Encode writes, to the +// byte. // -// An estimate nobody checks is a number nobody measured, and this one decides -// whether the warning above appears at all. It is held to a wide band on -// purpose: the exact size moves with how long the names are and what the notes -// say, and a guard demanding the byte would go red for a reason that does not -// matter. -func TestTheManifestEstimateIsNearWhatAManifestWeighs(t *testing.T) { - dir := t.TempDir() - const files = 300 +// This replaces a guard that held an estimate to "a quarter either way", and +// a quarter was wide enough to let a constant 6% low for text and half of what +// an e-mail entry costs stay green while a run of 20 500 files went unwarned. +// The answer is counted entry by entry now, so it is held to the byte: a +// document of no entries, one entry, and many unlike each other - notes, a +// failure, words outside ASCII, characters JSON escapes, properties nested +// several deep. +func TestTheManifestSizeIsTheLengthOfTheManifest(t *testing.T) { + documents := map[string]*manifest.Manifest{"no entries": newDocument()} - runCLI(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(files), "--out", dir) + one := newDocument() + one.Add(manifest.File{ID: "a", Path: "a.txt", Name: "a.txt", Format: "txt", Bytes: 1}) + documents["one entry"] = one - info, err := os.Stat(filepath.Join(dir, "manifest.json")) - if err != nil { - t.Fatalf("the run wrote no manifest: %v", err) - } - real := info.Size() - guess := manifest.EstimatedBytes(files, 0) + many := newDocument() + many.Run.Preset = &manifest.Preset{ID: "upload-validation", Parameters: map[string]string{"limit": "1mb"}} + many.Add(manifest.File{ID: "plain", Path: "plain.txt", Name: "plain.txt", Format: "txt", Bytes: 200, Materialized: true, + Hashes: manifest.Hashes{SHA256: strings.Repeat("ab", 32)}, TargetID: "t"}) + many.Add(manifest.File{ID: "noted", Path: "zażółć 日本.txt", Name: "zażółć 日本.txt", Format: "txt", + Notes: []manifest.Note{{Code: "label_omitted", Detail: "it said & \"left\" \\ alone"}}, + Expected: manifest.Expected{Outcome: "reject", Reason: "size_limit"}, Purpose: "a & c", Group: "g"}) + many.Add(manifest.File{ID: "deep", Path: "deep.zip", Name: "deep.zip", Format: "zip", + Properties: map[string]any{"contains": []any{map[string]any{"format": "txt", "count": 3}}, "depth": 2, "flat": true, "ratio": 0.5}}) + many.Add(manifest.File{ID: "failed", Path: "failed.png", Name: "failed.png", Format: "png", Failed: true, + Error: "a generator stopped", Damage: []manifest.Damage{{Type: "zero-head", Settings: map[string]string{"bytes": "4"}}}}) + documents["many entries"] = many - // Within a quarter either way. Wider than the measurement, narrow enough - // that the constant drifting by a factor would be caught. - low, high := real*3/4, real*5/4 - if guess < low || guess > high { - t.Errorf("the estimate for %d files is %d B and the manifest is %d B, which is outside a quarter either way", - files, guess, real) + for name, m := range documents { + counted, err := m.EncodedBytes() + if err != nil { + t.Fatalf("%s: counting: %v", name, err) + } + var written bytes.Buffer + if err := m.Encode(&written); err != nil { + t.Fatalf("%s: encoding: %v", name, err) + } + if counted != int64(written.Len()) { + t.Errorf("%s: the manifest was counted at %d B and Encode wrote %d B", name, counted, written.Len()) + } } } -// runCLI runs one command and gives back everything it said, on both channels. -// -// Both, because the line under test goes to the error channel - a report on -// stdout has to stay machine readable - and a guard reading only one of them -// would be measuring which channel it picked. -func runCLI(t *testing.T, args ...string) string { - t.Helper() - var out, errOut bytes.Buffer - cli.Run(context.Background(), args, &out, &errOut) - return out.String() + errOut.String() +// newDocument is an empty manifest with the header a run gives it. +func newDocument() *manifest.Manifest { + return manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") } // And it says so where a script can read it, not only in prose. @@ -130,19 +142,9 @@ func runCLI(t *testing.T, args ...string) string { // produced" and point at a manifest nothing can read. Exit codes are a frozen // contract. func TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads(t *testing.T) { - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - - // Asked of the estimator first, so a build whose ceiling moved above this - // count fails here rather than passing the whole test by never being over - // it at all. - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { - t.Fatalf("%d entries was not judged too large, so this proved nothing", over) - } - // stdout only. The summary line goes to the error stream, and a reader that // took both would be parsing JSON with a sentence stuck to the end of it. - code, said, errOut := run(t, "generate", "--format", "txt", "--size", "200b", - "--count", itoa(over), "--dry-run", "--json", "--out", t.TempDir()) + code, said, errOut := run(t, append(inTheSilentBand, "--json", "--out", t.TempDir())...) if code != cli.ExitOK { t.Fatalf("the run ended with %d, so there is no report to read:\n%s", code, errOut) } @@ -200,6 +202,132 @@ func TestAnOrdinaryRunSaysNothingAboutTheCeilingInItsReport(t *testing.T) { } } +// A saved manifest carries the field exactly when this build will refuse it, +// asked one byte either side of the ceiling. +// +// The field is written into the document whose length it describes, so the +// edge is where a rule that is nearly right goes wrong: a manifest of exactly +// manifest.MaxBytes is read and must not say otherwise, and one byte more is +// refused and must say so. Saved for real and read back, so the answer is about +// the bytes on the disk rather than about the arithmetic that predicted them. +func TestASavedManifestSaysItIsTooBigExactlyWhenItIs(t *testing.T) { + for _, past := range []int64{0, 1} { + m := documentOfLength(t, manifest.MaxBytes+past) + path := filepath.Join(t.TempDir(), "manifest.json") + if err := m.Save(path); err != nil { + t.Fatalf("saving: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("looking at the saved manifest: %v", err) + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading the saved manifest: %v", err) + } + says := bytes.Contains(raw, []byte(`"too_large_to_read_back": true`)) + _, loadErr := manifest.Load(path) + var refused *manifest.TooLargeError + isRefused := errors.As(loadErr, &refused) + + switch { + case past == 0 && (info.Size() != manifest.MaxBytes || says || isRefused): + t.Errorf("a manifest of the ceiling's length: %d B on the disk, field present %v, refused %v (%v) - it is read and must not say it is not", + info.Size(), says, isRefused, loadErr) + case past == 1 && (info.Size() <= manifest.MaxBytes || !says || !isRefused): + t.Errorf("a manifest one byte longer: %d B on the disk, field present %v, refused %v - it is refused and must say so", + info.Size(), says, isRefused) + } + if m.SavedBytes() != info.Size() { + t.Errorf("Save says it wrote %d B and the disk holds %d B", m.SavedBytes(), info.Size()) + } + } +} + +// documentOfLength is a manifest whose document, without the field, is exactly +// length bytes long - one entry's purpose takes up the difference. +func documentOfLength(t *testing.T, length int64) *manifest.Manifest { + t.Helper() + m := newDocument() + m.Add(manifest.File{ID: "a", Path: "a.txt", Name: "a.txt", Format: "txt", Purpose: "x"}) + now, err := m.EncodedBytes() + if err != nil { + t.Fatalf("counting: %v", err) + } + m.Files[0].Purpose += strings.Repeat("x", int(length-now)) + // Measured as it stands, before Save has settled the field - so this is the + // length the rule is asked about, and nothing here has set the field yet + // for Save to find already set. + if got, _ := m.EncodedBytes(); got != length || m.Summary.TooLargeToReadBack { + t.Fatalf("the document is %d B (field set: %v) and was built to be %d B without it, so the edge is not where this asks", + got, m.Summary.TooLargeToReadBack, length) + } + return m +} + +// What a run is told about its record before its first byte is what it then +// writes, when every file is produced - the manifest to the byte, and the +// instructions beside it. +// +// The forecast is what the free space is held against and what the warning +// above is said from. A forecast that drifted from the record would put both +// back where the old constant had them, wrong in a direction nobody sees. +func TestTheRecordARunIsToldBeforehandIsTheRecordItWrites(t *testing.T) { + dir := t.TempDir() + opt := engine.Options{OutDir: dir, Seed: 7, Command: "test"} + told := txtTarget("told", 3, 100) + told.Purpose, told.Group = "A file with a purpose, zażółć <&>", "with a purpose" + planned, err := engine.Plan([]engine.Target{txtTarget("plain", 300, 200), told}, opt) + if err != nil { + t.Fatalf("planning: %v", err) + } + + forecast, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting: %v", err) + } + res, err := engine.Run(context.Background(), planned, opt) + if err != nil { + t.Fatalf("running: %v", err) + } + rec, err := engine.SaveRecord(res, opt) + if err != nil { + t.Fatalf("saving the record: %v", err) + } + + if rec.Instructions == "" || forecast.Instructions == 0 { + t.Fatalf("no instructions were written or forecast (%q, %d B), so half of this asked nothing", rec.Instructions, forecast.Instructions) + } + onDisk := map[string]int64{} + for name, path := range map[string]string{"manifest": rec.Manifest, "instructions": rec.Instructions} { + info, err := os.Stat(path) + if err != nil { + t.Fatalf("looking at the %s: %v", name, err) + } + onDisk[name] = info.Size() + } + if forecast.Manifest != onDisk["manifest"] || rec.Size.Manifest != onDisk["manifest"] { + t.Errorf("the manifest was forecast at %d B, recorded as %d B and is %d B on the disk", + forecast.Manifest, rec.Size.Manifest, onDisk["manifest"]) + } + if forecast.Instructions != onDisk["instructions"] || rec.Size.Instructions != onDisk["instructions"] { + t.Errorf("the instructions were forecast at %d B, recorded as %d B and are %d B on the disk", + forecast.Instructions, rec.Size.Instructions, onDisk["instructions"]) + } +} + +// runCLI runs one command and gives back everything it said, on both channels. +// +// Both, because the line under test goes to the error channel - a report on +// stdout has to stay machine readable - and a guard reading only one of them +// would be measuring which channel it picked. +func runCLI(t *testing.T, args ...string) string { + t.Helper() + var out, errOut bytes.Buffer + cli.Run(context.Background(), args, &out, &errOut) + return out.String() + errOut.String() +} + // firstLines trims a long document down for a failure message. func firstLines(s string, n int) string { lines := strings.Split(s, "\n") diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index d4dadca3..7878888f 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -30,7 +30,7 @@ ( "the contributing guide names a Go version this module does not admit", "CONTRIBUTING.md", - "Needs Go 1.27.2 or newer", + "Needs Go 1.27.3 or newer", "Needs Go 1.21 or newer", "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), @@ -53,30 +53,30 @@ ( "the preview stops warning that the record will be too big to read back", "internal/gui/window/run.go", - "r.say(append([]string{said}, manifestReachNote(res)...)...)", + "r.say(append([]string{said}, manifestReachNote(res.Record)...)...)", "r.say(said)", "TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack", ), ( "a finished run stops warning, so somebody who never pressed Preview is told nothing", "internal/gui/window/run.go", - "said := append([]string{outcome}, manifestReachNote(res)...)", + "said := append([]string{outcome}, manifestReachNote(saved.Size)...)", "said := []string{outcome}", "TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack", ), ( "the window warns about every run, which teaches people to skip the line", "internal/gui/window/runreport.go", - "if !over {", - "if false && !over {", + "if !record.TooLarge {", + "if false && !record.TooLarge {", "TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling", ), ( - "the document answers a different question than the plan, so one run is warned about on one surface only", - "internal/manifest/manifest.go", - "return TooLargeToReadBack(len(m.Files), m.notedFiles)", - "return TooLargeToReadBack(len(m.Files)+1000, m.notedFiles)", - "TestBothSurfacesJudgeTheSameRunTheSameWay", + "the forecast of a run's record counts a hash one character short, so it drifts from what the run writes", + "internal/engine/recordsize.go", + "strings.Repeat(\"0\", 64)", + "strings.Repeat(\"0\", 63)", + "TestTheRecordARunIsToldBeforehandIsTheRecordItWrites", ), # --- verify says whose the other files are (2026-09-07, T8) --- ( @@ -242,9 +242,9 @@ # --- a manifest too big to read back says so where a script reads (2026-09-06) --- ( "the report stops saying the manifest cannot be read back", - "internal/manifest/manifest.go", - "_, m.Summary.TooLargeToReadBack = TooLargeToReadBack(len(m.Files), m.notedFiles)", - "_, _ = TooLargeToReadBack(len(m.Files), m.notedFiles)", + "internal/engine/engine.go", + "m.Summary.TooLargeToReadBack = record.TooLarge", + "_ = record.TooLarge", "TestAManifestTooBigToReadBackSaysSoInTheReportAScriptReads", ), ( @@ -334,14 +334,14 @@ ( "the README goes back to naming a floor the module no longer admits", "README.md", - "**From source.** Needs Go 1.27.2 or newer, and nothing else:", + "**From source.** Needs Go 1.27.3 or newer, and nothing else:", "**From source.** Needs Go 1.26.5 or newer, and nothing else:", "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), ( "CI builds with a compiler older than the floor go.mod admits", ".github/workflows/ci.yml", - 'GO_VERSION: "1.27.2"', + 'GO_VERSION: "1.27.3"', 'GO_VERSION: "1.26.8"', "TestTheBuildFloorIsThePinnedToolchainAndTheDocumentsSaySo", ), @@ -821,8 +821,8 @@ ( "a dry run skips the checks and promises a run that would be refused", "internal/engine/engine.go", - "if err := preflight(ctx, files, opt); err != nil {", - "if err := preflight(ctx, files, opt); err != nil && !opt.DryRun {", + "if err := preflight(ctx, files, opt, record); err != nil {", + "if err := preflight(ctx, files, opt, record); err != nil && !opt.DryRun {", "TestADryRunReachesTheSameVerdictAsTheRealRun", ), ( @@ -1453,7 +1453,7 @@ ( "verify never looks at content, so an edited file passes", "internal/audit/audit.go", - "if sum != f.Hashes.SHA256 {", + "if got.sum != f.Hashes.SHA256 {", "if false {", "TestVerifyCatchesAMissingAnExtraAndAChangedFile", ), @@ -6137,16 +6137,16 @@ ( "a run stops saying that its manifest will be too big for verify and cleanup to read", "internal/cli/record.go", - "\tif !over {", - "\tif true || !over {", + "if !record.TooLarge {", + "if true || !record.TooLarge {", "TestARunSaysWhenItsManifestWillBeTooBigToReadBack", ), ( - "the manifest size estimate stops matching what a manifest really weighs", - "internal/manifest/manifest.go", - "\tBytesPerEntry = 786", - "\tBytesPerEntry = 79", - "TestTheManifestEstimateIsNearWhatAManifestWeighs", + "the counted length of a manifest leaves out the line that closes its list of files", + "internal/manifest/size.go", + "between*int64(n-1) + closing", + "between*int64(n-1)", + "TestTheManifestSizeIsTheLengthOfTheManifest", ), ( "the size-boundaries preset stops refusing a limit with no number above it", @@ -6179,8 +6179,8 @@ ( "every run starts warning that its manifest is too big to read back", "internal/cli/record.go", - "\tif !over {", - "\tif !over && false {", + "if !record.TooLarge {", + "if !record.TooLarge && false {", "TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling", ), ( @@ -6254,10 +6254,10 @@ "TestAContainerRefusesASizeItsArithmeticCannotSee", ), ( - "preflight stops noticing that it was asked to stop", + "the forecast of the record and the preflight both stop noticing that they were asked to stop", "internal/engine/engine.go", - "\tif err := preflight(ctx, files, opt); err != nil {", - "\tif err := preflight(context.Background(), files, opt); err != nil {", + "record, err := opt.forecast(ctx, files)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tres.Record = record\n\tif err := preflight(ctx, files, opt, record); err != nil {", + "record, err := opt.forecast(context.Background(), files)\n\tif err != nil {\n\t\treturn res, err\n\t}\n\tres.Record = record\n\tif err := preflight(context.Background(), files, opt, record); err != nil {", "TestWorkingOutTheCostCanBeStopped", ), ( @@ -6660,8 +6660,8 @@ "the engine says something on stdout on the SUCCESS path, where the " "guard that a failed run prints nothing cannot see it", "internal/engine/engine.go", - "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tm := manifest.New(", - "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tfmt.Println(\"running\")\n\tm := manifest.New(", + "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tm := newManifest(opt)", + "func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error) {\n\tfmt.Println(\"running\")\n\tm := newManifest(opt)", "TestNothingBelowASurfaceReachesTheConsoleDirectly", ), ( @@ -12781,8 +12781,8 @@ ( 'a manifest that could not be written leaves its empty claim behind, so the files it should have named cannot be cleaned up', 'internal/manifest/reservation.go', - '\t\t_ = core.RemoveOwn(r.tmp, r.own)\n\t\treturn err\n\t}\n\treturn nil', - '\t\treturn err\n\t}\n\treturn nil', + '\t\t_ = core.RemoveOwn(r.tmp, r.own)\n\t\treturn err\n\t}\n\tm.saved = size\n\treturn nil', + '\t\treturn err\n\t}\n\tm.saved = size\n\treturn nil', 'TestAManifestThatCouldNotBeWrittenLeavesNoManifest', ), ( @@ -13223,7 +13223,7 @@ # through - read from the source, so a file built only off Windows # is held on the Windows machine too. "the checksum tool opens its file for writing as well, and the one claim does not notice", - "internal/tool/open_unix.go", + "internal/core/openregular_unix.go", "os.O_RDONLY|syscall.O_NONBLOCK, 0)", "os.O_RDWR|syscall.O_NONBLOCK, 0)", "TestEveryFileThisToolWritesIsCreatedThroughOneClaim", @@ -15542,4 +15542,102 @@ '"version": "9', "TestEveryVersionTheReadmeWritesOutIsThisOne", ), + ( + # R01 of docs/STABILITY-REVIEW-2026-10-26.md: a file judged ours and + # changed before the deletion reached it went, without --force. + "cleanup deletes a file that changed after it was judged ours, without asking the disk again", + "internal/audit/cleanup.go", + "if why, changed := changedSince(full, c.seen); changed {", + "if why, changed := changedSince(full, c.seen); false && changed {", + "TestCleanupLeavesAFileThatChangedAfterItWasChecked", + ), + ( + "a file put in the place of the one judged ours is taken for it when its size and time match", + "internal/audit/cleanup.go", + "if !os.SameFile(seen, now) || now.Size()", + "if now.Size()", + "TestCleanupLeavesAFileReplacedAfterItWasChecked", + ), + ( + "a file written over in place at its own length is still taken for the one judged ours", + "internal/audit/cleanup.go", + "now.Size() != seen.Size() || !now.ModTime().Equal(seen.ModTime()) {", + "now.Size() != seen.Size() {", + "TestCleanupLeavesAFileThatChangedAfterItWasChecked", + ), + ( + "verify calls every failure to look a missing file again", + "internal/audit/audit.go", + "if absent(err) {\n\t\treturn Difference{Kind: Missing", + "if err != nil {\n\t\treturn Difference{Kind: Missing", + "TestVerifyCallsANameItCouldNotLookUpUnreadableRatherThanMissing", + ), + ( + "a listed file is read to its end after Ctrl+C", + "internal/audit/read.go", + "if err := ctx.Err(); err != nil {\n\t\t\treturn read, err", + "if err := ctx.Err(); false && err != nil {\n\t\t\treturn read, err", + "TestHashingAListedFileStopsPartWayWhenCancelled", + ), + ( + "a listed file is read past its size and one byte, for as long as it grows", + "internal/audit/read.go", + "piece = piece[:left]", + "_ = left", + "TestHashingAListedFileReadsNoMoreThanItIsAskedFor", + ), + ( + # Unix only: a pipe exists nowhere else. The mutation runner of the CI + # is Linux, so it is judged there. + "verify and cleanup open a listed file with a plain open again, which waits on a pipe", + "internal/audit/read.go", + "f, seen, err := core.OpenRegular(path)", + "f, seen, err := func() (*os.File, os.FileInfo, error) { _ = core.OpenRegular; f, err := os.Open(path); if err != nil { return nil, nil, err }; i, err := f.Stat(); return f, i, err }()", + "TestVerifyAndCleanupNeverWaitOnAPipe", + ), + ( + # R03 of docs/STABILITY-REVIEW-2026-10-26.md: a manifest past the + # ceiling went out without the field that says so. + "a saved manifest past the ceiling no longer says it cannot be read back", + "internal/manifest/size.go", + "header.Summary.TooLargeToReadBack = true", + "header.Summary.TooLargeToReadBack = false", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "a manifest of exactly the ceiling's length says it cannot be read back, though it is read", + "internal/manifest/size.go", + "size <= MaxBytes {", + "size < MaxBytes {", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "Save writes the manifest without settling the field from its real length", + "internal/manifest/reservation.go", + "size, err := m.settleReadBack()", + "size, err := m.SavedBytes(), error(nil)", + "TestASavedManifestSaysItIsTooBigExactlyWhenItIs", + ), + ( + "the counted length of a manifest leaves out the indentation of the first line of every entry", + "internal/manifest/size.go", + "return int64(len(entryIndent) + len(b)), nil", + "return int64(len(b)), nil", + "TestTheManifestSizeIsTheLengthOfTheManifest", + ), + ( + # R08: the free space was held against the files alone. + "the free space is held against the files alone again, so a disk with no room for the manifest takes the files", + "internal/engine/preflight.go", + "core.AddSizes(TotalBytes(files), record.Bytes())", + "core.AddSizes(TotalBytes(files), 0)", + "TestARunWithRoomForItsFilesButNotItsRecordIsRefused", + ), + ( + "the forecast of a run's record leaves out the instructions written beside the manifest", + "internal/engine/recordsize.go", + "return int64(len(whole.Instructions(filepath.Base(ManifestPath(opt))))), nil", + "return 0, nil", + "TestTheRecordARunIsToldBeforehandIsTheRecordItWrites", + ), ] diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index f7ecae88..bf66796a 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -37,7 +37,7 @@ var notProvenByMutation = map[string]bool{ // - the substitution would be applied and the test would not exist to // catch it. It runs on every Linux and macOS runner of the CI. The two // breaks it is for - asking IsDir where IsRegular stands in - // tool.OpenRegular, and an open without O_NONBLOCK in tool/open_unix.go. + // core.OpenRegular, and an open without O_NONBLOCK in core/openregular_unix.go. // The first was read, not run (2026-09-29). The second was run by hand in // a Linux container on 2026-09-30, through the tool rather than through // this test: without the flag a pipe was still waited on after five diff --git a/internal/guard/recordspace_test.go b/internal/guard/recordspace_test.go new file mode 100644 index 00000000..ff50bd71 --- /dev/null +++ b/internal/guard/recordspace_test.go @@ -0,0 +1,60 @@ +package guard + +import ( + "context" + "errors" + "os" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/engine" +) + +// A disk with room for a run's files and not for its record is refused before +// the first byte. +// +// The free space was held against the files alone until 2026-10-26, so such a +// disk took every file and then refused the manifest - and a file no manifest +// lists is a file no command of this tool can remove, because the manifest is +// the only authority over that (untouchable rule 7, R08 in +// docs/STABILITY-REVIEW-2026-10-26.md). The free space is handed in, so the +// guard asks at the byte rather than filling a disk. +func TestARunWithRoomForItsFilesButNotItsRecordIsRefused(t *testing.T) { + plan := func(t *testing.T) ([]engine.PlannedFile, engine.Options) { + opt := engine.Options{OutDir: t.TempDir(), Seed: 7, Command: "test"} + planned, err := engine.Plan([]engine.Target{txtTarget("t", 3, 1000)}, opt) + if err != nil { + t.Fatalf("planning: %v", err) + } + return planned, opt + } + planned, opt := plan(t) + record, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting: %v", err) + } + files := engine.TotalBytes(planned) + if record.Manifest <= 0 { + t.Fatalf("the record was forecast at %d B, so there is nothing for the space to fall short of", record.Manifest) + } + + // One byte short of the files and their record. + opt.AvailableBytes = func(string) (int64, error) { return files + record.Bytes() - 1, nil } + _, err = engine.Run(context.Background(), planned, opt) + var noRoom *engine.SpaceError + if !errors.As(err, &noRoom) { + t.Fatalf("room for the files and all but one byte of the record gave %v, not the refusal about space", err) + } + if noRoom.Needed != files+record.Bytes() { + t.Errorf("the refusal says %d B are needed, and the files and their record are %d B", noRoom.Needed, files+record.Bytes()) + } + if entries, _ := os.ReadDir(opt.OutDir); len(entries) != 0 { + t.Errorf("a refused run left %d entries in the directory", len(entries)) + } + + // Exactly enough is enough - the other side of the same line. + planned, opt = plan(t) + opt.AvailableBytes = func(string) (int64, error) { return files + record.Bytes(), nil } + if _, err := engine.Run(context.Background(), planned, opt); err != nil { + t.Errorf("room for exactly the files and their record was refused: %v", err) + } +} diff --git a/internal/guard/tools_unix_test.go b/internal/guard/tools_unix_test.go index f32fe65a..6c03ffa8 100644 --- a/internal/guard/tools_unix_test.go +++ b/internal/guard/tools_unix_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/tool" ) @@ -21,7 +22,7 @@ import ( // of nothing instead of the refusal. // // Since 2026-09-30 this is asked of the open file rather than of the name -// (tool.OpenRegular), which is what closed the moment between the two looks +// (core.OpenRegular), which is what closed the moment between the two looks // that a review of #157 found - so this one guard holds both halves. // // Every tool that reads a file, walked from the registry since 2026-10-19, @@ -56,7 +57,7 @@ func refusesThePipe(t *testing.T, d tool.Descriptor, pipe string) { }() select { case err := <-answered: - var notAFile *tool.NotRegularError + var notAFile *core.NotRegularError if !errors.As(err, ¬AFile) { t.Errorf("a pipe was named to %s and the answer was %v, not the refusal saying it is not a file", d.ID, err) } diff --git a/internal/guard/windowmanifestreach_test.go b/internal/guard/windowmanifestreach_test.go index fc135109..cbcbd2eb 100644 --- a/internal/guard/windowmanifestreach_test.go +++ b/internal/guard/windowmanifestreach_test.go @@ -1,9 +1,11 @@ package guard import ( + "context" "strings" "testing" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" "github.com/donislawdev/TestingFilesGenerator/internal/manifest" @@ -24,22 +26,38 @@ import ( // looks for, but something one surface SAYS and the other does not. // // The note was written down as a question about the manifest schema, on the -// grounds that notes are per file and this one is per run. It is not. The -// command line does not read this off the manifest either - it works it out -// from the plan and prints it before the first byte - and manifest.TooLarge- -// ToReadBack was put where it is exactly so the two surfaces could not come to -// different conclusions about one run. What was missing was a caller. - -// overTheCeiling is a file count whose manifest this build would refuse. +// grounds that notes are per file and this one is per run. It is not. Both +// surfaces take the answer from the engine - the record a run is forecast to +// write, and after it the record it wrote (engine.RecordSize) - so the two +// cannot come to different conclusions about one run. What was missing was a +// caller. + +// overTheCeiling is a file count whose manifest this build would refuse, for +// files of 200 B of text - the run the screens below are filled in for. // -// Worked out from the estimate rather than written here, for the reason the -// command line guard beside it gives: a guard carrying its own copy of a limit -// goes stale the day somebody changes the real one, and says nothing while it -// does. +// Worked out from the engine's own forecast rather than written here, for the +// reason the command line guard beside it gives: a guard carrying its own copy +// of a limit goes stale the day somebody changes the real one, and says +// nothing while it does. A thousand files past the count the forecast puts at +// the ceiling, and then asked again, so a forecast that moved cannot leave +// this under it. func overTheCeiling(t *testing.T) int { t.Helper() - over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 - if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { + forecast := func(n int) engine.RecordSize { + opt := engine.Options{OutDir: t.TempDir(), Seed: 1, Command: "test"} + planned, err := engine.Plan([]engine.Target{txtTarget("t", n, 200)}, opt) + if err != nil { + t.Fatalf("planning %d files: %v", n, err) + } + r, err := engine.PredictRecord(context.Background(), planned, opt) + if err != nil { + t.Fatalf("forecasting %d files: %v", n, err) + } + return r + } + each := forecast(2).Manifest - forecast(1).Manifest + over := int(manifest.MaxBytes/each) + 1000 + if !forecast(over).TooLarge { t.Fatalf("%d entries was not judged too large, so this guard would prove nothing", over) } return over @@ -189,32 +207,3 @@ func TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling(t *testing.T) { t.Errorf("a hundred files drew the warning about the record being too big:\n%s", said) } } - -// Both surfaces judge the same run the same way. -// -// The window reads the answer off the document a dry run builds, and the -// command line works it out from the plan before anything is written. Two paths -// to one number, and what makes two paths acceptable is that they go through -// one predicate. Asked at the boundary, which is the only place a disagreement -// would show. -func TestBothSurfacesJudgeTheSameRunTheSameWay(t *testing.T) { - for _, entries := range []int{ - int(manifest.MaxBytes / manifest.BytesPerEntry), - int(manifest.MaxBytes/manifest.BytesPerEntry) + 1, - } { - _, fromThePlan := manifest.TooLargeToReadBack(entries, 0) - - m := manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") - for i := 0; i < entries; i++ { - m.Add(manifest.File{Path: "f.txt", Materialized: true}) - } - _, fromTheDocument := m.ReadBackReach() - - if fromThePlan != fromTheDocument { - t.Errorf("at %d entries the plan says %v and the document says %v.\n"+ - "The command line answers from the first and the window from the second, so one run "+ - "would be warned about on one surface and not on the other", - entries, fromThePlan, fromTheDocument) - } - } -} diff --git a/internal/guard/writeescape_test.go b/internal/guard/writeescape_test.go index 018710a7..48833bcc 100644 --- a/internal/guard/writeescape_test.go +++ b/internal/guard/writeescape_test.go @@ -133,7 +133,7 @@ func TestEveryFileThisToolWritesIsCreatedThroughOneClaim(t *testing.T) { // readingFlags are the flags of an open that only reads - os.Open with // something os.Open cannot ask for. O_NONBLOCK is why this exists: the -// tools open without waiting on a pipe (tool.OpenRegular), which +// tools open without waiting on a pipe (core.OpenRegular), which // takes os.OpenFile, and it creates nothing and writes nothing. var readingFlags = map[string]bool{ "O_RDONLY": true, "O_NONBLOCK": true, "O_CLOEXEC": true, "O_NOFOLLOW": true, "O_NOCTTY": true, diff --git a/internal/gui/text/locale/said/en.json b/internal/gui/text/locale/said/en.json index 5924a3df..7e36b6c8 100644 --- a/internal/gui/text/locale/said/en.json +++ b/internal/gui/text/locale/said/en.json @@ -1140,8 +1140,8 @@ }, "engine.NoSpace": { "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run. Carries {{.Needed}}, {{.Dir}}, {{.Free}}, written the way the program writes them - keep each spelled exactly that way.", - "hash": "sha256-0e6457cf7a3b", - "other": "this run needs {{.Needed}} B and {{.Dir}} has {{.Free}} B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory" + "hash": "sha256-064c75842c91", + "other": "this run needs {{.Needed}} B with its manifest and {{.Dir}} has {{.Free}} B free - nothing was written. Ask for fewer files or a smaller size, or write to another disk by changing the output directory" }, "engine.NotProduced": { "description": "Said by internal/engine - a refusal under the box it is about or at the foot of the form, or a note after a run.", diff --git a/internal/gui/text/locale/said/pl.json b/internal/gui/text/locale/said/pl.json index f9c3600a..42d710a3 100644 --- a/internal/gui/text/locale/said/pl.json +++ b/internal/gui/text/locale/said/pl.json @@ -226,7 +226,7 @@ "engine.NoIDWhy": { "hash": "sha256-363605924eb4", "other": "każda partia potrzebuje stałej nazwy, bo od niej zależy ziarno i to ona łączy partię z manifestem" }, "engine.NoOutDir": { "hash": "sha256-9ff19de0e58b", "other": "katalog wyjściowy jest pusty" }, "engine.NoOutDirFix": { "hash": "sha256-c14426ffc389", "other": "Podaj katalog, na przykład ./fixtures" }, - "engine.NoSpace": { "hash": "sha256-0e6457cf7a3b", "other": "ten przebieg potrzebuje {{.Needed}} B, a w {{.Dir}} jest wolne {{.Free}} B - nic nie zostało zapisane. Poproś o mniej plików albo mniejszy rozmiar albo zapisz na inny dysk, zmieniając katalog wyjściowy" }, + "engine.NoSpace": { "hash": "sha256-064c75842c91", "other": "ten przebieg potrzebuje {{.Needed}} B razem z manifestem, a w {{.Dir}} jest wolne {{.Free}} B - nic nie zostało zapisane. Poproś o mniej plików albo mniejszy rozmiar albo zapisz na inny dysk, zmieniając katalog wyjściowy" }, "engine.NotProduced": { "hash": "sha256-46fb82f6985b", "other": "Ten plik nie powstał. Przebieg poszedł dalej i skończył się kodem wyjścia oznaczającym częściowy wynik." }, "engine.OutDirIsAFile": { "hash": "sha256-55a340f0b67d", "other": "katalog wyjściowy {{.Dir}} jest plikiem, a nie katalogiem" }, "engine.OutDirIsAFileFix": { "hash": "sha256-f9e5f71f3822", "other": "Wskaż jako katalog wyjściowy katalog albo taki, którego jeszcze nie ma - zostanie utworzony" }, diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index c0266611..206bedfd 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -515,9 +515,9 @@ func roomOn(dir string) diskRoom { // previewFinished is the end of a preview, back on the interface thread. // // The result is carried across as well as the plan, and that is what lets a -// preview warn about a record too big to read back. A dry run builds the whole -// document - see manifestReachNote - so the answer is there for the asking -// rather than something the window would have to work out for itself. +// preview warn about a record too big to read back. A dry run works out the +// record the run would write - see manifestReachNote - so the answer is there +// for the asking rather than something the window works out for itself. func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFile, opt engine.Options, room diskRoom, runErr error) { r.busy.set(false, busyFace{}) if runErr != nil { @@ -531,7 +531,7 @@ func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFil r.line.measured(opt.OutDir, room.free) } said := r.line.said(exactly(planned), opt.OutDir) + text.AndNothingWrittenYet() - r.say(append([]string{said}, manifestReachNote(res)...)...) + r.say(append([]string{said}, manifestReachNote(res.Record)...)...) } // formatsOf is what kinds of file the run would produce, each named once. @@ -716,7 +716,7 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error, room dis // said only how many files - so the one thing this tool makes that others // do not was, from a window, something you found in the folder afterwards. outcome := text.SaidWithManifest(outcomeText(res, runErr), manifestNameOf(saved.Manifest)) - said := append([]string{outcome}, manifestReachNote(res)...) + said := append([]string{outcome}, manifestReachNote(saved.Size)...) // Instructions that were due and are not there are said, not failed: the // manifest was saved and holds the same facts. Silence would leave a button // missing with no reason given. diff --git a/internal/gui/window/runreport.go b/internal/gui/window/runreport.go index bafa24ea..15a73123 100644 --- a/internal/gui/window/runreport.go +++ b/internal/gui/window/runreport.go @@ -27,25 +27,20 @@ import ( // quality rather than in what the engine can do, which is the kind D1 is // easiest to lose. // -// Read off the document rather than worked out here, and off the SAME predicate -// the command line uses, which is what manifest.TooLargeToReadBack exists for. -// The two surfaces cannot come to different conclusions about one run. -// -// A preview reaches this too. engine.Run with DryRun adds an entry for every -// planned file, so the document a preview produces is the document the run -// would produce, minus the bytes on the disk. That is why one shape serves -// both, and why the window can answer before anything is written even though it -// cannot say a word in the middle of a run. -func manifestReachNote(res *engine.Result) []string { - if res == nil || res.Manifest == nil { - return nil - } - size, over := res.Manifest.ReadBackReach() - if !over { +// Read off the engine rather than worked out here, from the same count the +// command line uses, so the two surfaces cannot come to different conclusions +// about one run. A preview hands in the record worked out for the run it +// previews (engine.Result.Record), and a finished run the record it wrote +// (engine.Record.Size) - the fact, since 2026-10-26, rather than an estimate +// that was low for every format and let a run of 20 500 files end with no word +// about a manifest Verify then refused (docs/STABILITY-REVIEW-2026-10-26.md). +// A run whose record was not written hands in nothing, and nothing is said. +func manifestReachNote(record engine.RecordSize) []string { + if !record.TooLarge { return nil } return []string{text.ManifestTooLargeToRead( - text.HumanBytes(size), text.HumanBytes(manifest.MaxBytes))} + text.HumanBytes(record.Manifest), text.HumanBytes(manifest.MaxBytes))} } // progressText is the line under the bar. Bytes rather than files, because one diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index d8c98047..5b263137 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -35,15 +35,10 @@ type Manifest struct { Summary Summary `json:"summary"` Files []File `json:"files"` - // notedFiles is how many entries carry a note, which the size estimate - // needs and no reader does. - // - // Unexported, so it is not written and not read back. A manifest decoded - // from disk carries the ANSWER in Summary.TooLargeToReadBack rather than - // the working needed to reach it, which is what a reader wants and is also - // what keeps the two from disagreeing - a decoded manifest has no Add - // calls to count. - notedFiles int + // saved is how long this document was when Save wrote it, and nought + // before that or for a document read back from the disk. Unexported, so it + // is not written and not read back. + saved int64 } // Tool records what produced these bytes. Without it a hash mismatch after an @@ -188,6 +183,11 @@ type Summary struct { // written stays byte for byte what it was. An added field, which // docs/MANIFEST.md section 10 allows without moving manifest_version - // the same shape as tool.go, added for review item S1. + // + // Save sets it from the length of the document it writes, since + // 2026-10-26. It came from an estimate until then, and a manifest of + // 17 170 219 B went out without it (size.go). A preview's document carries + // it for the run it previews, from the same count. TooLargeToReadBack bool `json:"too_large_to_read_back,omitempty"` } @@ -365,6 +365,13 @@ func New(toolName, toolVersion, runID, command string, seed int64, os, arch stri // one place is what stops the two from drifting apart. func (m *Manifest) Add(f File) { m.Files = append(m.Files, f) + m.count(f) +} + +// count folds one file into the summary. Apart from Add so that a Projection +// counts a file exactly as Add does without keeping it - still one place that +// counts. +func (m *Manifest) count(f File) { m.Summary.FileCount++ m.Summary.ByFormat[f.Format]++ if f.TargetID != "" { @@ -380,14 +387,6 @@ func (m *Manifest) Add(f File) { if _, ok := m.Tool.Generators[f.Format]; !ok && f.Generator.Version != "" { m.Tool.Generators[f.Format] = f.Generator.Version } - // Kept in step here for the reason the comment above this function gives: - // counting anywhere else is a second place for the two to disagree. The - // estimate is arithmetic over the counts, so recomputing it per file costs - // nothing and is always right about the document as it stands. - if len(f.Notes) > 0 { - m.notedFiles++ - } - _, m.Summary.TooLargeToReadBack = TooLargeToReadBack(len(m.Files), m.notedFiles) } // noteExamples is how many file names a grouped note shows before it stops @@ -552,76 +551,16 @@ func (e *SchemaError) Said() core.Said { // an asymmetry rather than a decision. // // Sixteen megabytes rather than the recipe's one. A manifest is generated -// rather than typed and it grows with the run: measured on 2026-08-03, an entry -// costs about 700 B, so this holds a run of roughly twenty thousand files - -// twice the largest preset in docs/PRESETS.md and well above the ten thousand -// this tool is designed around. +// rather than typed and it grows with the run: measured on 2026-10-26, an entry +// costs from about 840 B (txt) to about 1580 B (eml), so this holds a run of +// roughly ten to twenty thousand files - above the largest preset in +// docs/PRESETS.md and around the ten thousand this tool is designed around. A +// run past it is told so, from the document's real length (size.go). // // What it does not do, stated as plainly as the recipe's: it does not defend // against a manifest that fits and is still expensive. const MaxBytes = 16 << 20 -// What an entry really costs, and what a run of a given size will come to. -// -// The comment above says "an entry costs about 700 B, so this holds a run of -// roughly twenty thousand files". Re-measured on 2026-08-26 against real runs: -// 786 B an entry for files carrying no notes, and 1009 B where every file -// carries one. So the ceiling is reached at about 21 300 files, or about 16 600 -// when every file has something to say about itself. -// -// The number matters because nothing compared it with the other ceiling. -// core.MaxFilesPerRun is a million. Measured the same day: a run of 25 000 -// files wrote a manifest of 25 220 640 B, and from that moment neither "tfg -// verify" nor "tfg cleanup" would read it - both exit 5 - so the files sat on -// the disk with nothing in this toolset able to remove them. Two ceilings were -// chosen independently and never held against each other. -// -// This is what lets a run say so before it starts rather than after. It is -// deliberately an estimate and named as one: the exact size depends on how long -// the names are and what the notes say, and a run that guessed slightly low -// would still be better off than one that said nothing. -const ( - // BytesPerEntry is a file with nothing to report. - BytesPerEntry = 786 - // BytesPerNotedEntry is the surcharge for a file that carries a note. - BytesPerNotedEntry = 223 -) - -// EstimatedBytes is roughly what the manifest of a run this shape will weigh. -func EstimatedBytes(entries, withNotes int) int64 { - return int64(entries)*BytesPerEntry + int64(withNotes)*BytesPerNotedEntry -} - -// TooLargeToReadBack says whether this build would refuse its own manifest for -// a run of this shape. -// -// Asked here rather than worked out by each surface, so the command line and -// the window cannot come to different conclusions about the same run. -func TooLargeToReadBack(entries, withNotes int) (int64, bool) { - n := EstimatedBytes(entries, withNotes) - return n, n > MaxBytes -} - -// ReadBackReach is what this document will weigh and whether this build would -// refuse to read it. -// -// The same question TooLargeToReadBack answers, asked of a manifest that -// exists rather than of a plan that has not run. It is here because the two -// callers had nowhere else to get the count of entries carrying a note from - -// that is kept while entries are added, so working it out again would mean -// walking every file a second time and getting it subtly wrong when a failed -// entry gains a note of its own. -// -// The window needs this and the command line does not. The command line says -// its piece BEFORE the first byte, from the plan, where no manifest exists yet. -// A window cannot say anything in the middle of a run - a widget touched from a -// worker is a race, and two of those were found on CI - so it says it when the -// run ends, off the document the run actually produced. Both go through -// TooLargeToReadBack, which is where that answer is settled for both surfaces. -func (m *Manifest) ReadBackReach() (int64, bool) { - return TooLargeToReadBack(len(m.Files), m.notedFiles) -} - // TooLargeError is returned for a manifest past MaxBytes. type TooLargeError struct { Path string diff --git a/internal/manifest/reservation.go b/internal/manifest/reservation.go index cc4369f7..c48cd13b 100644 --- a/internal/manifest/reservation.go +++ b/internal/manifest/reservation.go @@ -110,6 +110,14 @@ func (r *Reservation) Save(m *Manifest) error { return core.Defect(fmt.Errorf("the reservation of %s was already used or given back", core.Shown(r.final))) } r.used = true + // Whether this build will read the document back is settled from the + // length it is about to have, before the file is opened - so the field + // says what is true of the bytes below it (size.go). + size, err := m.settleReadBack() + if err != nil { + _ = core.RemoveOwn(r.tmp, r.own) + return err + } f, err := core.OpenOwn(r.tmp, r.own) if err != nil { // A reservation that could not be opened is still this run's, unless @@ -126,9 +134,14 @@ func (r *Reservation) Save(m *Manifest) error { _ = core.RemoveOwn(r.tmp, r.own) return err } + m.saved = size return nil } +// SavedBytes is how long this document was when Save wrote it, and nought for +// one not saved by this process. +func (m *Manifest) SavedBytes() int64 { return m.saved } + // Release gives the name back, for a run that reserved it and then had nothing // to record. Without it a refused run would leave its reservation behind and the // next run into that directory would be told a run is going. diff --git a/internal/manifest/size.go b/internal/manifest/size.go new file mode 100644 index 00000000..d23ef3b7 --- /dev/null +++ b/internal/manifest/size.go @@ -0,0 +1,183 @@ +package manifest + +import "encoding/json" + +// How long a manifest is, to the byte, and whether this build would read it +// back. +// +// Counted rather than estimated since 2026-10-26. Until then a constant stood +// here - 786 B an entry, measured on txt files on 2026-08-26 - and the warning +// that a manifest would be too large to read came from it. Measured again on +// 2026-10-26 (docs/STABILITY-REVIEW-2026-10-26.md, section 6.0): an entry +// costs 838 B for txt, 893 for csv and 1577 for eml, so the constant was low +// for every format, and a run of 20 500 text files wrote a manifest of +// 17 170 219 B with no warning and no too_large_to_read_back in it - which +// verify and cleanup then refused. No single number is right for every format, +// names and notes, so the document is measured instead. +// +// Measured an entry at a time rather than by encoding the whole document, so +// knowing the size costs one entry's worth of memory. A dry run of a million +// files already peaks near three gigabytes (measured the same day), and a +// second copy of the document to count it would be a large share of that. +// +// The arithmetic below is Encode's layout written out: the document with an +// empty "files" list, and then the entries in place of the brackets. A guard +// holds it to the length of Encode's output, byte for byte, for documents of +// none, one and many entries (TestTheManifestSizeIsTheLengthOfTheManifest). + +// entryIndent is what Encode puts in front of every line of an entry: the +// entry is two levels into the document. +const entryIndent = " " + +// entryBytes is how long one entry is inside the document, its indentation +// included. +func entryBytes(f File) (int64, error) { + b, err := json.MarshalIndent(f, entryIndent, " ") + if err != nil { + return 0, err + } + // MarshalIndent puts the prefix in front of every line but the first, and + // in the document the first line is indented like the rest. + return int64(len(entryIndent) + len(b)), nil +} + +// documentBytes joins an empty document and its entries. With none, the +// document is what it is - "files": [] - and with some, the two brackets +// open onto a line each, the entries are separated by a comma and a line +// break, and the list closes on a line of its own one level in. +func documentBytes(empty, entries int64, n int) int64 { + if n == 0 { + return empty + } + const open, between, closing int64 = int64(len("[\n") - len("[]")), int64(len(",\n")), int64(len("\n ]")) + return empty + open + entries + between*int64(n-1) + closing +} + +// emptyBytes is how long this document is with no entries in it. +func (m *Manifest) emptyBytes() (int64, error) { + shell := *m + if len(shell.Files) > 0 { + shell.Files = []File{} + } + var n counted + if err := shell.Encode(&n); err != nil { + return 0, err + } + return int64(n), nil +} + +// counted is a writer that keeps the count and nothing else. +type counted int64 + +func (c *counted) Write(p []byte) (int, error) { + *c += counted(len(p)) + return len(p), nil +} + +// settleReadBack sets Summary.TooLargeToReadBack from the length of the +// document as it is about to be written, and says how long that is. +// +// The field is counted in its own document, so whether it is there changes +// the length it describes. Settled the one way that cannot be wrong: present +// exactly when the document WITHOUT it is past the ceiling. Then a document +// that carries it is longer still and is refused, and one that does not +// carry it is within the ceiling and is read. +func (m *Manifest) settleReadBack() (int64, error) { + entries, err := m.entriesBytes() + if err != nil { + return 0, err + } + size, _, err := settle(m, entries, len(m.Files)) + return size, err +} + +// entriesBytes is how long the entries of this document are, together. +func (m *Manifest) entriesBytes() (int64, error) { + var entries int64 + for _, f := range m.Files { + b, err := entryBytes(f) + if err != nil { + return 0, err + } + entries += b + } + return entries, nil +} + +// settle is the rule above, for a header and the entries counted for it. +func settle(header *Manifest, entries int64, n int) (int64, bool, error) { + header.Summary.TooLargeToReadBack = false + empty, err := header.emptyBytes() + if err != nil { + return 0, false, err + } + if size := documentBytes(empty, entries, n); size <= MaxBytes { + return size, false, nil + } + header.Summary.TooLargeToReadBack = true + if empty, err = header.emptyBytes(); err != nil { + return 0, false, err + } + return documentBytes(empty, entries, n), true, nil +} + +// EncodedBytes is exactly how long Encode makes this document as it stands, +// and changes nothing in it. Save settles Summary.TooLargeToReadBack first, so +// asked of a document that is about to be saved, this can be short by the +// field. +func (m *Manifest) EncodedBytes() (int64, error) { + entries, err := m.entriesBytes() + if err != nil { + return 0, err + } + empty, err := m.emptyBytes() + if err != nil { + return 0, err + } + return documentBytes(empty, entries, len(m.Files)), nil +} + +// Projection is the length of a document a run has not written yet, counted +// as its entries are offered and none of them kept. +// +// A run says before its first byte whether its manifest will be readable, and +// a preview says what the run would write. Both have the plan and no document, +// and building the document a million entries long to measure it would hold +// it all at once. This counts each entry the moment it is offered and keeps +// only the summary, the same summary Add keeps. +type Projection struct { + shell *Manifest + entries int64 + n int + err error +} + +// Projection measures the document m would become with entries added to it. +// m is taken over as the header - its run, its tool - and must hold no files. +func (m *Manifest) Projection() *Projection { + return &Projection{shell: m} +} + +// Add counts one entry the document would hold. +func (p *Projection) Add(f File) { + if p.err != nil { + return + } + b, err := entryBytes(f) + if err != nil { + p.err = err + return + } + p.entries += b + p.n++ + p.shell.count(f) +} + +// Bytes is how long the document would be, and whether this build would refuse +// to read it back. +func (p *Projection) Bytes() (int64, bool, error) { + if p.err != nil { + return 0, false, p.err + } + return settle(p.shell, p.entries, p.n) +} diff --git a/internal/tool/archivescan/archivescan.go b/internal/tool/archivescan/archivescan.go index cc943bff..61c3212d 100644 --- a/internal/tool/archivescan/archivescan.go +++ b/internal/tool/archivescan/archivescan.go @@ -110,8 +110,8 @@ func run(ctx context.Context, in tool.Request, progress tool.Progress) (tool.Res limit, _ = core.ParseSize(raw) } path := in.Inputs[InputArchive] - f, before, err := tool.OpenRegular(path) - var not *tool.NotRegularError + f, before, err := core.OpenRegular(path) + var not *core.NotRegularError if errors.As(err, ¬) { return tool.Result{}, &NotAFileError{Path: path, Directory: not.Directory} } diff --git a/internal/tool/archivescan/refusals.go b/internal/tool/archivescan/refusals.go index bb5c9024..13e1d03a 100644 --- a/internal/tool/archivescan/refusals.go +++ b/internal/tool/archivescan/refusals.go @@ -36,7 +36,7 @@ func (e *NotAFileError) Error() string { return e.Said().String() } // Unwrap is the refusal every tool shares. func (e *NotAFileError) Unwrap() error { - return &tool.NotRegularError{Path: e.Path, Directory: e.Directory} + return &core.NotRegularError{Path: e.Path, Directory: e.Directory} } // Said is the whole refusal, for a window that says it in its own language. diff --git a/internal/tool/checksum/read.go b/internal/tool/checksum/read.go index 7a83df5c..27a46f40 100644 --- a/internal/tool/checksum/read.go +++ b/internal/tool/checksum/read.go @@ -8,7 +8,7 @@ import ( "io" "os" - "github.com/donislawdev/TestingFilesGenerator/internal/tool" + "github.com/donislawdev/TestingFilesGenerator/internal/core" ) // Reading files, for all three tools of this package. One way in, so a file @@ -67,11 +67,11 @@ func digest(ctx context.Context, path string, chosen map[string]bool, scratch [] return out, before, nil } -// openRegular is tool.OpenRegular with its refusal in the words of these +// openRegular is core.OpenRegular with its refusal in the words of these // tools - a checksum of a directory, not just a directory. func openRegular(path string) (*os.File, os.FileInfo, error) { - f, info, err := tool.OpenRegular(path) - var not *tool.NotRegularError + f, info, err := core.OpenRegular(path) + var not *core.NotRegularError if errors.As(err, ¬) { return nil, nil, &NotAFileError{Path: path, Directory: not.Directory} } diff --git a/internal/tool/checksum/refusals.go b/internal/tool/checksum/refusals.go index a2de117b..abdfa360 100644 --- a/internal/tool/checksum/refusals.go +++ b/internal/tool/checksum/refusals.go @@ -126,7 +126,7 @@ func (e *NotAFileError) Error() string { return e.Said().String() } // Unwrap is the refusal every tool shares, so a check that asks any tool // whether it turned a pipe away finds this one too. func (e *NotAFileError) Unwrap() error { - return &tool.NotRegularError{Path: e.Path, Directory: e.Directory} + return &core.NotRegularError{Path: e.Path, Directory: e.Directory} } // Said is the whole refusal, for a window that says it in its own language.