Skip to content

Commit c6b3970

Browse files
donislawdevclaude
andcommitted
engine: give one answer about a name on every machine
Five findings from going through the environment questions - filesystems, locales, encodings, permissions, shells - on Windows, on exFAT and on Linux. TWO NAMES DIFFERING ONLY IN CASE WERE TWO FILES The collision check read "one name" as "the same string". Most filesystems people run this on disagree: NTFS, APFS and exFAT keep the case somebody typed and match without it, ext4 matches exactly. Measured on Windows: report.txt and REPORT.TXT exit 0, "2 file(s) in 2 target(s)" on disk one file the manifest two entries tfg verify wrong-size report.txt, exit 7 One file written over by the other, a manifest describing something that is not there, and the tool's own check failing on its own output a second later - which is the defect the collision check exists to stop, reached by changing the case of a letter. Refused everywhere now, including ext4 where they really are two files, for the reason the separator rule already gives: a recipe travels between machines and one that quietly loses a file on somebody else's is worse than one refused on both. A COLON WAS LEFT TO THE LOCAL MACHINE TO INTERPRET Measured: "a:b.txt" was accepted on Linux and refused on Windows from one recipe, because filepath.VolumeName answers for the system it runs on. And "AB:c.txt" on Windows is not a drive at all - it names an alternate data stream, so the run reported the file as not produced, ended with the partial code, and left an empty file called AB in the directory: not in the manifest, reported by verify as something nobody asked for, beyond the reach of cleanup for good. Refused on every system now, like a separator, and the volume test no longer asks the local machine. A RECIPE IN ANY ENCODING WAS READ AS BEST IT COULD BE Measured: a recipe saved as cp1250 with Polish letters in a file name was accepted with exit 0, and the file arrived called "za" followed by four replacement characters. The manifest recorded the same mangled name, so verify agreed with the disk and nothing said the name was not the one asked for. Notepad still offers ANSI when saving and this tool is aimed at testers on Windows. It is refused now, with what to do about it. A DIRECTORY WE CANNOT WRITE IN SAID THE MANIFEST ALREADY EXISTED My own regression from earlier today: every failure to claim the manifest name was reported as a collision, so an empty directory with writes denied produced "manifest.json already exists ... it is the only record of what an earlier run wrote". Untrue, and it sends somebody looking for a run that never happened. A second message about failing to write the manifest followed it, for one fault, because Started was set before the claim rather than after it. WHAT WAS CHECKED AND FOUND CORRECT Worth recording, because these were the questions and the answer was yes: - the manifest records UTC with the Z on it, unmoved by TZ (checked in three zones) - numbers and messages do not change with LC_ALL (checked C, pl_PL, de_DE) - a system error under a Polish locale still arrives in English with its number - names Windows will not take - star, question mark, pipe, over 255 characters - end with the partial code and a manifest entry saying so, which is D10 working rather than failing - exFAT takes generate, verify and cleanup end to end, and the new case rule fires there, which is where it matters most - a dot file, brackets, spaces, ampersands and emoji all survive as typed - the whole guard suite passes on Linux ext4 as well as on Windows Six guards, each seen red first, and seven mutations. The permission one skips on Windows, where denying a write needs an ACL rather than a mode, and was proven on Linux. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 08c64b5 commit c6b3970

5 files changed

Lines changed: 411 additions & 12 deletions

File tree

‎internal/core/filename.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,15 @@ func ContainmentProblem(p string) string {
6060
return ""
6161
}
6262

63-
// hasVolumeName spots a Windows volume prefix such as "C:". filepath.VolumeName
64-
// answers for the system this build runs on, which is exactly what must not
65-
// decide it here.
63+
// HasVolumeName spots a Windows volume prefix such as "C:".
64+
//
65+
// filepath.VolumeName answers for the system this build runs on, which is
66+
// exactly what must not decide it. Measured on 2026-08-04: the name "a:b.txt"
67+
// was accepted on Linux and refused on Windows as an absolute path, from one
68+
// recipe - so a fixture set written on one machine failed on the next, which
69+
// is the thing the separator rule beside it exists to prevent.
70+
func HasVolumeName(p string) bool { return hasVolumeName(p) }
71+
6672
func hasVolumeName(p string) bool {
6773
if len(p) < 2 || p[1] != ':' {
6874
return false

‎internal/engine/engine.go‎

Lines changed: 81 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"errors"
1212
"fmt"
1313
"io"
14+
"io/fs"
1415
"os"
1516
"path/filepath"
1617
"runtime"
@@ -254,7 +255,7 @@ func settleTarget(t *Target, opt Options, seen map[string]bool) (format.Descript
254255
func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
255256
var out []PlannedFile
256257
seen := map[string]bool{}
257-
names := map[string]string{}
258+
names := map[string]nameOwner{}
258259

259260
// The running total of the whole run, kept here rather than worked out
260261
// afterwards. Both of these used to be settled too late to help: the file
@@ -318,12 +319,33 @@ func Plan(targets []Target, opt Options) ([]PlannedFile, error) {
318319
// destroyed by the other, and the manifest would still describe
319320
// both. A manifest that quietly lost a file looks complete and
320321
// reaches the test suite as a false truth.
321-
if owner, clash := names[name]; clash {
322-
return nil, &RecipeError{Detail: fmt.Sprintf(
323-
"targets %q and %q both produce a file named %s - give one of them a --name template containing {index:04}",
324-
owner, t.ID, name)}
322+
//
323+
// "One name" is not "the same string", and reading it that way cost
324+
// exactly what this check exists to prevent. Most filesystems people
325+
// run this on keep the case somebody typed and match without it -
326+
// NTFS, APFS and exFAT do, ext4 does not - so "report.txt" and
327+
// "REPORT.TXT" are two files on one machine and one file on the
328+
// next. Measured on Windows, 2026-08-03: exit 0, two entries in the
329+
// manifest, one file on the disk, and "tfg verify" failing on the
330+
// tool's own output a second later.
331+
//
332+
// Refused everywhere rather than only where it bites, the same as a
333+
// path separator and for the same reason: a recipe travels between
334+
// machines by design, and one that quietly loses a file on somebody
335+
// else's is worse than one refused on both. Producing such a pair on
336+
// purpose belongs to the name laboratory and its archive mode, D10.
337+
key := strings.ToLower(name)
338+
if owner, clash := names[key]; clash {
339+
detail := fmt.Sprintf("targets %q and %q both produce a file named %s", owner.id, t.ID, name)
340+
if owner.name != name {
341+
detail = fmt.Sprintf(
342+
"targets %q and %q produce the names %s and %s, which differ only in case. Most filesystems treat those as one file, so one would be written over the other and the manifest would describe both",
343+
owner.id, t.ID, owner.name, name)
344+
}
345+
return nil, &RecipeError{Detail: detail +
346+
" - give one of them a --name template containing " + indexToken}
325347
}
326-
names[name] = t.ID
348+
names[key] = nameOwner{id: t.ID, name: name}
327349

328350
if totalBytes, err = core.AddSizes(totalBytes, p.Bytes); err != nil {
329351
return nil, &RecipeError{Detail: fmt.Sprintf(
@@ -447,6 +469,13 @@ func tempPathFor(outDir, name string) string {
447469
return fmt.Sprintf("%s%s%d", filepath.Join(outDir, name), core.PartialMarker, os.Getpid())
448470
}
449471

472+
// nameOwner remembers who took a name and how it was spelled, so a refusal can
473+
// show both names as they were written rather than as they were compared.
474+
type nameOwner struct {
475+
id string
476+
name string
477+
}
478+
450479
func exists(path string) bool {
451480
_, err := os.Stat(path)
452481
return err == nil
@@ -478,12 +507,12 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error)
478507
if err := preflight(files, opt); err != nil {
479508
return res, err
480509
}
481-
res.Started = true
482510

483511
if opt.DryRun {
484512
// Nothing is written. Every entry is what would have been produced,
485513
// which is the same planning path the real run uses rather than a
486514
// separate one that can drift away from it.
515+
res.Started = true
487516
for _, f := range files {
488517
m.Add(entryFor(f, "", false, nil))
489518
}
@@ -505,8 +534,26 @@ func Run(ctx context.Context, files []PlannedFile, opt Options) (*Result, error)
505534
// Taking the name here turns that into a refusal before anything is written.
506535
manifestPath := filepath.Join(opt.OutDir, manifestNameOf(opt))
507536
if err := manifest.Claim(manifestPath); err != nil {
508-
return res, &CollisionError{Path: manifestPath, Manifest: true}
537+
// Only a name that is genuinely taken is a collision. Reporting every
538+
// failure that way said "manifest.json already exists ... it is the
539+
// only record of what an earlier run wrote" about an empty directory
540+
// the user simply had no permission to write in - a sentence that is
541+
// untrue and sends somebody looking for a run that never happened.
542+
// Measured on 2026-08-04 with write denied on the output directory.
543+
if errors.Is(err, fs.ErrExist) {
544+
return res, &CollisionError{Path: manifestPath, Manifest: true}
545+
}
546+
return res, fmt.Errorf("cannot start a run in %s: %w", opt.OutDir, err)
509547
}
548+
549+
// Past this point the run owns the name and may write. Started says so, and
550+
// it is what tells the caller a manifest is worth saving - set here rather
551+
// than before the claim, because a run that could not take the name has
552+
// written nothing and has nothing to record. It used to be set earlier, so
553+
// a refused claim was followed by a second message about failing to write
554+
// the manifest it had just been told it could not have.
555+
res.Started = true
556+
510557
// Given back if this run ends without writing one, or the name would stay
511558
// taken by a run that never happened.
512559
defer func() {
@@ -743,6 +790,25 @@ func checkFileName(where, name string) error {
743790
"%s produces the name %q, which is a path rather than a file name. Names stay inside the output directory, and a separator is refused on every system so that a recipe works everywhere. Use --out or the output section to choose the directory",
744791
where, name)}
745792

793+
// A colon, on every system, for the same reason as a separator.
794+
//
795+
// Windows reads it as the start of an alternate data stream, so
796+
// "AB:c.txt" names a stream called c.txt inside a file called AB.
797+
// Measured on 2026-08-04: the run reported the file as not produced and
798+
// ended with the partial code, and an empty file called AB was left in the
799+
// directory anyway - not in the manifest, reported by verify as something
800+
// nobody asked for, and beyond the reach of cleanup for good. A single
801+
// letter in front of it is read as a drive instead, which is how the same
802+
// recipe came to be accepted on Linux and refused on Windows.
803+
//
804+
// Legal in a name on Linux and macOS, and refused there too. A recipe
805+
// travels between machines by design, and one that quietly leaves debris on
806+
// somebody else's is worse than one refused on all of them.
807+
case strings.Contains(name, ":"):
808+
return &RecipeError{Detail: fmt.Sprintf(
809+
"%s produces the name %q, which holds a colon. Windows reads that as a drive or as an alternate data stream rather than as part of the name, so the file arrives called something else or not at all. It is refused on every system so that a recipe means one thing everywhere - take the colon out, or ask for the file inside an archive where the name survives",
810+
where, name)}
811+
746812
case name == "." || name == "..":
747813
return &RecipeError{Detail: fmt.Sprintf(
748814
"%s produces the name %q, which names a directory rather than a file", where, name)}
@@ -765,7 +831,13 @@ func checkFileName(where, name string) error {
765831
"%s produces the name %q, which ends in a dot or a space. Windows stores such a name without it, so the file on disk would not be the file the manifest describes and verify would report both. Take the last character off, or ask for the file inside an archive where the name survives",
766832
where, name)}
767833

768-
case filepath.IsAbs(name) || filepath.VolumeName(name) != "":
834+
// Judged the same way on every system, like the separator above. Using
835+
// filepath here asks the machine this build runs on, and the answer
836+
// differs: measured on 2026-08-04, "a:b.txt" was accepted on Linux and
837+
// refused on Windows from one recipe, so a fixture set written on one
838+
// machine failed on the next. That is the failure this rule exists to
839+
// prevent, arriving through the rule itself.
840+
case filepath.IsAbs(name) || core.HasVolumeName(name):
769841
return &RecipeError{Detail: fmt.Sprintf(
770842
"%s produces the absolute path %q. A recipe carries no absolute paths, because then it only works on the machine it was written on. Use --out or the output section to choose the directory",
771843
where, name)}

‎internal/guard/environment_test.go‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
package guard
2+
3+
import (
4+
"encoding/json"
5+
"os"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
"time"
11+
"unicode/utf8"
12+
13+
"github.com/donislawdev/TestingFilesGenerator/internal/cli"
14+
)
15+
16+
// What the machine underneath is allowed to change about a run, and what it is
17+
// not. Every case here was measured on 2026-08-04 while going through the
18+
// environment questions the owner raised.
19+
20+
// A directory somebody cannot write in is a permission problem and says so.
21+
//
22+
// It used to say "manifest.json already exists ... it is the only record of
23+
// what an earlier run wrote" about an empty directory, because every failure
24+
// to claim the manifest name was reported as a collision. That sends a person
25+
// looking for a run that never happened. A second message about failing to
26+
// write the manifest followed it, for one fault.
27+
func TestADirectoryWeCannotWriteInIsAPermissionProblem(t *testing.T) {
28+
if runtime.GOOS == "windows" {
29+
// os.Chmod on Windows only toggles the read only bit, which does not
30+
// stop a file being created in a directory. Denying it needs an ACL,
31+
// which is not something a test should be installing. Measured by hand
32+
// there instead, and this runs on the other two systems in the matrix.
33+
t.Skip("a directory that refuses writes needs an ACL on Windows")
34+
}
35+
dir := t.TempDir()
36+
out := filepath.Join(dir, "ro")
37+
if err := os.Mkdir(out, 0o555); err != nil {
38+
t.Fatalf("making a directory nobody can write in: %v", err)
39+
}
40+
t.Cleanup(func() { _ = os.Chmod(out, 0o755) })
41+
42+
code, stdout, errOut := run(t, "generate", "--format", "txt", "--size", "1kb", "--out", out)
43+
44+
if code == cli.ExitOK {
45+
t.Fatal("a run into a directory it cannot write in reported success")
46+
}
47+
if strings.Contains(errOut, "already exists") {
48+
t.Errorf("a permission problem is reported as a name that is taken:\n%s", errOut)
49+
}
50+
if n := strings.Count(errOut, "tfg: "); n != 1 {
51+
t.Errorf("one fault produced %d messages:\n%s", n, errOut)
52+
}
53+
if stdout != "" {
54+
t.Errorf("a failed run wrote to stdout:\n%s", stdout)
55+
}
56+
}
57+
58+
// The time the manifest records does not move with the clock of the machine.
59+
//
60+
// It is written as UTC with the Z on it, so a run in Tokyo and a run in
61+
// California describe the same instant the same way. A local time would make
62+
// two manifests of the same run look like different runs, and a reader
63+
// comparing them has no way to tell which.
64+
func TestTheRecordedTimeDoesNotDependOnTheLocalZone(t *testing.T) {
65+
dir := t.TempDir()
66+
code, stdout, errOut := run(t,
67+
"generate", "--format", "txt", "--size", "1kb", "--json", "--out", dir)
68+
if code != cli.ExitOK {
69+
t.Fatalf("exit %d:\n%s", code, errOut)
70+
}
71+
var m struct {
72+
GeneratedAt string `json:"generated_at"`
73+
}
74+
if err := json.Unmarshal([]byte(stdout), &m); err != nil {
75+
t.Fatalf("the manifest did not parse: %v", err)
76+
}
77+
if !strings.HasSuffix(m.GeneratedAt, "Z") {
78+
t.Errorf("generated_at is %q - it has to be UTC, or two machines describe one run differently", m.GeneratedAt)
79+
}
80+
if _, err := time.Parse(time.RFC3339, m.GeneratedAt); err != nil {
81+
t.Errorf("generated_at is not a time anything can read: %v", err)
82+
}
83+
}
84+
85+
// A recipe is UTF-8 and anything else is refused rather than guessed at.
86+
//
87+
// Measured on 2026-08-04: a recipe saved as cp1250 with Polish letters in a
88+
// name was accepted with exit 0, and the file arrived called "za" followed by
89+
// four replacement characters. The manifest agreed with the disk, so nothing
90+
// downstream noticed - the tool had simply produced a different name from the
91+
// one that was asked for, silently. Notepad on Windows still offers ANSI, and
92+
// this tool is aimed at testers on Windows.
93+
func TestARecipeThatIsNotUTF8IsRefused(t *testing.T) {
94+
dir := t.TempDir()
95+
path := filepath.Join(dir, "r.yaml")
96+
97+
// "zażółć.txt" in cp1250 - valid text in that encoding, not valid UTF-8.
98+
body := []byte("version: 1\ntargets:\n - id: t\n format: txt\n size: 1kb\n name: za")
99+
body = append(body, 0xbf, 0xf3, 0xb3, 0xe6) // ż ó ł ć
100+
body = append(body, []byte(".txt\n")...)
101+
if utf8.Valid(body) {
102+
t.Fatal("the fixture is valid UTF-8, so it cannot show what it was written to show")
103+
}
104+
if err := os.WriteFile(path, body, 0o644); err != nil {
105+
t.Fatalf("writing: %v", err)
106+
}
107+
108+
code, _, errOut := run(t, "validate", path)
109+
if code == cli.ExitOK {
110+
t.Fatal("a recipe that is not UTF-8 was accepted, so a name comes back with replacement characters in it")
111+
}
112+
if !strings.Contains(errOut, "UTF-8") {
113+
t.Errorf("the refusal does not say what is wrong with the file:\n%s", errOut)
114+
}
115+
if !strings.Contains(strings.ToLower(errOut), "save") && !strings.Contains(errOut, "encoding") {
116+
t.Errorf("the refusal does not say what to do about it:\n%s", errOut)
117+
}
118+
}
119+
120+
// A name is judged the same way on every system.
121+
//
122+
// The rule about separators says so in as many words - a recipe that only
123+
// works on the machine it was written on is not portable - and the rule beside
124+
// it about absolute paths was asking the local machine. Measured on
125+
// 2026-08-04: "a:b.txt" was accepted on Linux and refused on Windows from one
126+
// recipe, because filepath.VolumeName answers for the system it runs on.
127+
//
128+
// The name below is the same on every run of this test, so a disagreement
129+
// between two systems shows up as one of them failing rather than as nobody
130+
// noticing.
131+
func TestANameIsJudgedTheSameWayOnEverySystem(t *testing.T) {
132+
for _, name := range []string{"a:b.txt", "C:x.txt", "z:", "AB:c.txt"} {
133+
t.Run(name, func(t *testing.T) {
134+
dir := t.TempDir()
135+
code, _, errOut := run(t,
136+
"generate", "--format", "txt", "--size", "1kb",
137+
"--name", name, "--dry-run", "--out", dir)
138+
if code == cli.ExitOK {
139+
t.Errorf("the name %q was accepted here, and a machine with drive letters refuses it - so one recipe gives two answers", name)
140+
}
141+
_ = errOut
142+
})
143+
}
144+
145+
// The other direction: a colon is not the only thing with a letter in
146+
// front of it, and an ordinary name has to survive.
147+
for _, name := range []string{"ab.txt", "a-b.txt", "a.b.txt"} {
148+
t.Run("still works "+name, func(t *testing.T) {
149+
dir := t.TempDir()
150+
if code, _, errOut := run(t,
151+
"generate", "--format", "txt", "--size", "1kb",
152+
"--name", name, "--dry-run", "--out", dir); code != cli.ExitOK {
153+
t.Errorf("an ordinary name was refused: exit %d\n%s", code, errOut)
154+
}
155+
})
156+
}
157+
}
158+
159+
// And a recipe that is UTF-8 keeps working, with the letters intact all the
160+
// way to the name on the disk.
161+
func TestARecipeInUTF8KeepsItsLetters(t *testing.T) {
162+
dir := t.TempDir()
163+
out := filepath.Join(dir, "out")
164+
path := writeRecipe(t, dir, "version: 1\ntargets:\n - id: t\n format: txt\n size: 1kb\n name: za\u017c\u00f3\u0142\u0107.txt\noutput:\n dir: "+filepath.ToSlash(out)+"\n")
165+
166+
if code, _, errOut := run(t, "generate", path); code != cli.ExitOK {
167+
t.Fatalf("exit %d:\n%s", code, errOut)
168+
}
169+
entries, err := os.ReadDir(out)
170+
if err != nil {
171+
t.Fatalf("reading the output: %v", err)
172+
}
173+
found := false
174+
for _, e := range entries {
175+
if e.Name() == "za\u017c\u00f3\u0142\u0107.txt" {
176+
found = true
177+
}
178+
if strings.ContainsRune(e.Name(), '\uFFFD') {
179+
t.Errorf("the name came back with a replacement character in it: %q", e.Name())
180+
}
181+
}
182+
if !found {
183+
var names []string
184+
for _, e := range entries {
185+
names = append(names, e.Name())
186+
}
187+
t.Errorf("the file is not there under the name that was asked for: %v", names)
188+
}
189+
}

0 commit comments

Comments
 (0)