Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,33 @@ because it turns other people's test suites red.

### Fixed

- **Two runs writing into one directory can no longer write over each other's
files.** A run holds the directory it is writing into for as long as it is
writing. A second run that starts meanwhile is refused before it writes a
byte, with exit code `5` and a message saying so.

That protection was already there and was attached to the wrong thing. A run
takes its manifest name at the start and keeps it until it ends, so two runs
both writing `manifest.json` into one directory have always been refused. A
recipe that points `output.manifest` at a name of its own had nothing holding
it. Measured with two runs started on the same instant, eight times: twice
both ended `0`, both reported sixty files produced, and sixty files existed -
every one of the first run's replaced by the second run's, without either run
saying anything. `tfg verify` against the first manifest then reported sixty
changed files for a run that had been told it succeeded.

What holds the directory is a file called `.tfg-run-lock`. The run removes it
when it ends, including when you stop it with Ctrl+C. A run killed outright
cannot remove it, so it stays behind and the next run into that directory is
refused until you delete it - the refusal names the file and says exactly
that. `tfg verify` also names it, as a leftover of ours rather than as a file
somebody else put there, and `tfg cleanup` will not remove it, because it
removes only what a manifest lists.

The cost is worth stating plainly: two runs can no longer fill one directory
at the same time, even when the files they write have different names. For
every run that does not set `output.manifest` that was already true.

- **The About screen now shows the support address, so the Donate button is no
longer the only way to reach it.** Pressing Donate asks your desktop to open
the page. On a machine with no browser registered that quietly does nothing,
Expand Down
29 changes: 23 additions & 6 deletions internal/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ func (d Difference) String() string {
case Unreadable:
return fmt.Sprintf("unreadable %s - %s", d.Path, d.Got)
case Leftover:
// The lock a run holds while it writes into a directory, and it is the
// one of these that may belong to something still going on. So the
// sentence says both endings rather than asserting the run is dead:
// verify cannot tell, and telling somebody to delete the mark a live
// run is holding would let a second run in behind it.
if core.IsRunLockName(filepath.Base(d.Path)) {
return fmt.Sprintf(
"leftover %s\n the mark a run holds while it writes into this directory. If a run is going "+
"on it will remove this itself when it ends. If none is, it was killed before it could tidy up - "+
"no files were lost, but no new run will start here until this is deleted by hand",
d.Path)
}
// Two markers reach this, and one sentence cannot serve both. The
// first is a file that was being produced, so nothing is lost. The
// second is a RECORD that was being saved, so the useful thing to say
Expand Down Expand Up @@ -262,12 +274,17 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string)
kind := Extra
want := ""
switch {
case core.IsPartialName(filepath.Base(p)), core.IsWritingName(filepath.Base(p)):
// Both markers, because both name a file this tool started and did
// not finish. Only the first was recognised until 2026-09-06, so a
// half written manifest was reported as "extra" - the word that
// means somebody else put it here. They get different sentences in
// String, because what a reader should do about them differs.
case core.IsPartialName(filepath.Base(p)),
core.IsWritingName(filepath.Base(p)),
core.IsRunLockName(filepath.Base(p)):
// All three markers, because each names something this tool put
// here and did not take away. Only the first was recognised until
// 2026-09-06, so a half written manifest was reported as "extra" -
// the word that means somebody else put it here - and the third
// arrived with the run lock on 2026-09-07. They get different
// sentences in String, because what a reader should do about them
// differs, and about the lock it differs most: it is the only one
// that may belong to a run that is still going.
kind = Leftover
default:
// One file under two spellings reads as a polluted directory
Expand Down
8 changes: 8 additions & 0 deletions internal/cli/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,14 @@ func classifyReading(err error) (int, bool) {
if errors.As(err, &collision) {
return ExitIO, true
}
// A directory somebody else's run is holding. The same code as a name that
// is taken, because it is the same kind of answer - the disk would not have
// this run - and the frozen table has one row for that. What differs is the
// sentence, and that lives on the error.
var inProgress *engine.RunInProgressError
if errors.As(err, &inProgress) {
return ExitIO, true
}
// A manifest we cannot read is a reading failure, not a bug in the tool.
// Falling through to RUNTIME would tell CI to file a report against us for
// a file somebody handed in.
Expand Down
38 changes: 38 additions & 0 deletions internal/core/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,44 @@ func IsWritingName(name string) bool {
return strings.HasSuffix(name, WritingMarker)
}

// RunLockName is the name a run holds for as long as it is writing into a
// directory.
//
// The third of these, and the only one that is a whole name rather than a
// suffix on somebody else's: it belongs to the run, not to a file. A run takes
// it exclusively before the first byte and gives it back when it ends, so a
// second run starting into the same directory is refused rather than allowed
// to write over what the first one is producing.
//
// Why it had to exist, measured on 2026-09-07 with two runs started on the
// same wall clock instant, eight times: twice both runs ended 0, each said it
// had produced sixty files, and sixty files were on the disk - every one of
// the first run's belonging to the second. Five times the runs ended 8, and
// that was luck rather than a defence: Windows refuses to rename onto a file
// another process holds open, which is not something Linux does.
//
// The protection this restores already existed and was keyed to the wrong
// thing. A run claims its manifest name for its whole length, so two runs
// writing manifest.json into one directory have always been refused - measured
// the same day, four times out of four. output.manifest was the one way out of
// that, and it was never meant to be a way out of this.
//
// Declared here beside the other two for the reason written above them: two
// parts of the tool have to agree on the spelling. The engine writes it, and
// verify has to recognise one that outlived its run rather than call it a file
// somebody else put there.
const RunLockName = ".tfg-run-lock"

// IsRunLockName says whether a name is that lock.
//
// A whole name rather than a suffix, so this is equality rather than a search.
// Written as a function anyway, because every reader of the other two markers
// asks through one and a reader that compares the constant itself is a reader
// that will not be found when the spelling changes.
func IsRunLockName(name string) bool {
return name == RunLockName
}

// AddSizes adds one file size to a running total and says when the total has
// left the range it is measured in.
//
Expand Down
50 changes: 50 additions & 0 deletions internal/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,31 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error)
return res, fmt.Errorf("cannot create the output directory %s: %w", opt.OutDir, err)
}

// The directory is taken before the manifest name is, and the two are not
// the same claim. This one does not depend on what the manifest is called,
// so a recipe that points output.manifest somewhere of its own is held by
// it exactly as a default run is.
//
// That difference is the whole of it. Measured on 2026-09-07 with two runs
// started on the same instant, eight times: twice both ended 0, both said
// sixty files, and sixty files existed - the second run's, over the first
// run's names. verify against the first manifest then reported sixty wrong
// hashes for a run that had been told it succeeded. With the same manifest
// name the claim below already refused that, four times out of four, which
// is why this is the same mechanism rather than a new one.
lockPath := RunLockPath(opt.OutDir)
if err := claimRunLock(lockPath); err != nil {
if errors.Is(err, fs.ErrExist) {
return res, &RunInProgressError{Path: lockPath, Dir: opt.OutDir}
}
return res, fmt.Errorf("cannot start a run in %s: %w", opt.OutDir, err)
}
// Given back however this run ends, including one stopped part way: the
// signal cancels the context, Run returns, and this runs. What it cannot
// cover is the process being killed outright, and that is why the refusal
// above names the file to remove.
defer releaseRunLock(lockPath)

// The manifest name is taken before the first file, not after the last one.
//
// Claiming it at save time already stopped two runs from both writing a
Expand Down Expand Up @@ -611,6 +636,31 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error)
return res, nil
}

// claimRunLock takes the name that says this directory has a run in it.
//
// core.CreateNew rather than os.Create, and that is the whole claim: it refuses
// a name something already holds and it believes the refusal only when Lstat
// finds something there, so a directory reached through a link still works.
// The file stays empty - the same shape as the manifest claim, and for the same
// reason. Nothing reads it, so there is nothing in it to be read half written.
func claimRunLock(path string) error {
fh, err := core.CreateNew(path, 0o666)
if err != nil {
return err
}
return fh.Close()
}

// releaseRunLock gives the name back.
//
// The failure is dropped on purpose. A run that finished and could not remove
// its own lock has nothing useful to say to the person - the files are written
// and the manifest is saved - and the next run into that directory will name
// the file and say what to do about it.
func releaseRunLock(path string) {
_ = os.Remove(path)
}

func entryFor(f PlannedFile, sha string, materialized bool, failure error) manifest.File {
var notes []manifest.Note
for _, n := range f.Plan.Notes {
Expand Down
23 changes: 23 additions & 0 deletions internal/engine/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,29 @@ func (e *SpaceError) Error() string {
e.Needed, e.Path, e.Available)
}

// RunInProgressError is refusing to start because another run holds this
// directory.
//
// Its own type rather than a third state on CollisionError, because it is a
// different fault with a different remedy. A collision is a name that is taken
// and the answer is to move that file or write somewhere else. This is a run
// that has not finished, and the answer is usually to wait.
//
// The last sentence is there because a run killed outright cannot give the
// name back, and a person who is told only "another run is writing here" about
// a machine where nothing is running has been given a dead end. It names the
// file so that clearing it is one command rather than a hunt.
type RunInProgressError struct {
Path string
Dir string
}

func (e *RunInProgressError) Error() string {
return fmt.Sprintf(
"another run is already writing into %s, so this one will not start. Two runs writing into one directory can write over each other's files without either of them saying so. Wait for it to finish, or generate into a different directory. If nothing is running, that run was killed before it could tidy up - remove %s and try again",
e.Dir, e.Path)
}

// CollisionError is refusing to write over something that is already there.
//
// This tool runs in directories that belong to the user. Overwriting without
Expand Down
30 changes: 30 additions & 0 deletions internal/engine/preflight.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ func preflight(ctx context.Context, files []PlannedFile, opt Options) error {
Remedy: "Point the output directory at a directory, or at one that does not exist yet and it will be created"}
}

// Whether anybody else is writing here comes before every other question
// about a name, and the order is the message rather than tidiness. A run in
// flight has already claimed its manifest name, so without this check a
// second run into the same directory was told "manifest.json already exists
// ... it is the only record of what an earlier run wrote" - a sentence
// about a run that finished, said about one that is still going, which
// sends somebody looking through a directory rather than waiting a minute.
//
// Asked here rather than only at the claim below because a dry run stops
// before the claim. A preview that says a run would succeed, while another
// run is filling the directory it would write into, is answering a question
// nobody asked.
if path := RunLockPath(opt.OutDir); exists(path) {
return &RunInProgressError{Path: path, Dir: opt.OutDir}
}

// The manifest is checked with the files it would describe, and leaving it
// out cost exactly what it protects. A second run into the same directory
// wrote a fresh manifest over the old one, so every file the old one listed
Expand Down Expand Up @@ -205,3 +221,17 @@ func manifestNameOf(opt Options) string {
func ManifestPath(opt Options) string {
return filepath.Join(opt.OutDir, manifestNameOf(opt))
}

// RunLockPath is the name a run holds while it writes into a directory.
//
// Exported for the same reason ManifestPath is: more than one part of the tool
// has to mean the same file. The check above asks whether it is taken, the run
// takes it, and a guard has to be able to put one there and watch a second run
// refuse.
//
// It does not depend on the manifest name, and that independence is the whole
// point. Two runs pointing output.manifest at different files are two runs
// writing into one directory, which is the case the manifest claim cannot see.
func RunLockPath(outDir string) string {
return filepath.Join(outDir, core.RunLockName)
}
Loading
Loading