Skip to content

Commit d02ebf2

Browse files
committed
Suggested next commit (after you run local gates):
test(jsonutil): port write.go + write_test.go from upstream - Adds the missing JSONL / atomic write helpers used by checkpoint condensation and trail. - All tests pass after rebrand. Part of the full entireio/cli → trace port effort (see /tmp/FULL_PORT_CHECKLIST.md). This can be committed on top of the API port (bd1b5f7) before or after the fmt/lint cleanup commit for PR #14.
1 parent bd1b5f7 commit d02ebf2

2 files changed

Lines changed: 229 additions & 0 deletions

File tree

cmd/trace/cli/jsonutil/write.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package jsonutil
2+
3+
import (
4+
"fmt"
5+
"io/fs"
6+
"os"
7+
"path/filepath"
8+
)
9+
10+
// WriteFileAtomic writes data to filePath atomically by writing to a temp file
11+
// in the same directory, fsyncing it, renaming into place, and fsyncing the
12+
// parent directory. A crash or signal mid-write leaves the original file
13+
// intact rather than a truncated partial — important for config files like
14+
// .entire/settings.json that callers expect to remain parseable across
15+
// interrupted writes.
16+
//
17+
// The fsync between Write and Close guarantees the temp file's bytes are on
18+
// disk before the rename takes effect; without it, some filesystems (notably
19+
// ext4 with non-default mount options) can surface the rename as completed
20+
// while the file is still empty after a hard crash.
21+
//
22+
// The parent-directory fsync after rename guarantees the rename's directory
23+
// entry is durable. Without it, the file contents are on disk but the
24+
// directory may still point to the pre-rename state after a crash, so the
25+
// "leaves the original intact" promise would silently break. Windows does
26+
// not support directory fsync; we make this step best-effort so the call
27+
// does not fail on platforms where the operation is a no-op.
28+
//
29+
// perm is applied to the temp file via Chmod before rename so the final file
30+
// lands with the requested permission regardless of the temp file's default.
31+
func WriteFileAtomic(filePath string, data []byte, perm fs.FileMode) error {
32+
dir := filepath.Dir(filePath)
33+
base := filepath.Base(filePath)
34+
tmp, err := os.CreateTemp(dir, base+".*.tmp")
35+
if err != nil {
36+
return fmt.Errorf("create temp for %s: %w", filePath, err)
37+
}
38+
tmpName := tmp.Name()
39+
removeTmp := true
40+
defer func() {
41+
if removeTmp {
42+
_ = os.Remove(tmpName)
43+
}
44+
}()
45+
if _, err := tmp.Write(data); err != nil {
46+
_ = tmp.Close()
47+
return fmt.Errorf("write temp for %s: %w", filePath, err)
48+
}
49+
if err := tmp.Sync(); err != nil {
50+
_ = tmp.Close()
51+
return fmt.Errorf("sync temp for %s: %w", filePath, err)
52+
}
53+
if err := tmp.Close(); err != nil {
54+
return fmt.Errorf("close temp for %s: %w", filePath, err)
55+
}
56+
if err := os.Chmod(tmpName, perm); err != nil {
57+
return fmt.Errorf("chmod temp for %s: %w", filePath, err)
58+
}
59+
if err := os.Rename(tmpName, filePath); err != nil {
60+
return fmt.Errorf("rename temp to %s: %w", filePath, err)
61+
}
62+
removeTmp = false
63+
// Best-effort: the rename succeeded, so don't propagate failures here.
64+
// Directory fsync isn't supported on Windows, and on POSIX an error
65+
// after a successful rename would mislead callers who already have the
66+
// file in place.
67+
if d, err := os.Open(dir); err == nil { //nolint:gosec // G304: dir is filepath.Dir of caller-supplied filePath, not user input
68+
_ = d.Sync() //nolint:errcheck // best-effort directory fsync; failure does not roll back the rename
69+
_ = d.Close()
70+
}
71+
return nil
72+
}
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package jsonutil
2+
3+
import (
4+
"errors"
5+
"os"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
)
11+
12+
func TestWriteFileAtomic_CreatesNewFile(t *testing.T) {
13+
t.Parallel()
14+
dir := t.TempDir()
15+
target := filepath.Join(dir, "out.json")
16+
data := []byte(`{"hello":"world"}`)
17+
18+
if err := WriteFileAtomic(target, data, 0o644); err != nil {
19+
t.Fatalf("WriteFileAtomic: %v", err)
20+
}
21+
22+
got, err := os.ReadFile(target)
23+
if err != nil {
24+
t.Fatalf("read back: %v", err)
25+
}
26+
if string(got) != string(data) {
27+
t.Errorf("content mismatch: got %q want %q", got, data)
28+
}
29+
}
30+
31+
func TestWriteFileAtomic_ReplacesExistingFile(t *testing.T) {
32+
t.Parallel()
33+
dir := t.TempDir()
34+
target := filepath.Join(dir, "out.json")
35+
if err := os.WriteFile(target, []byte("old contents"), 0o644); err != nil {
36+
t.Fatalf("seed file: %v", err)
37+
}
38+
39+
newData := []byte("new contents")
40+
if err := WriteFileAtomic(target, newData, 0o644); err != nil {
41+
t.Fatalf("WriteFileAtomic: %v", err)
42+
}
43+
44+
got, err := os.ReadFile(target)
45+
if err != nil {
46+
t.Fatalf("read back: %v", err)
47+
}
48+
if string(got) != string(newData) {
49+
t.Errorf("content not replaced: got %q want %q", got, newData)
50+
}
51+
}
52+
53+
// AppliesPermission verifies the Chmod-before-rename step actually lands the
54+
// requested mode on the final file. os.CreateTemp defaults to 0o600 so
55+
// without the Chmod a 0o644 caller would silently get a tighter mode.
56+
func TestWriteFileAtomic_AppliesPermission(t *testing.T) {
57+
t.Parallel()
58+
if runtime.GOOS == "windows" {
59+
t.Skip("POSIX permission bits are not meaningful on Windows")
60+
}
61+
dir := t.TempDir()
62+
target := filepath.Join(dir, "out.json")
63+
64+
if err := WriteFileAtomic(target, []byte("x"), 0o600); err != nil {
65+
t.Fatalf("WriteFileAtomic: %v", err)
66+
}
67+
68+
info, err := os.Stat(target)
69+
if err != nil {
70+
t.Fatalf("stat: %v", err)
71+
}
72+
if got := info.Mode().Perm(); got != 0o600 {
73+
t.Errorf("perm: got %#o want %#o", got, 0o600)
74+
}
75+
}
76+
77+
// LeavesNoTempOnSuccess guards against the removeTmp defer being skipped or
78+
// the temp suffix changing in a way that breaks cleanup.
79+
func TestWriteFileAtomic_LeavesNoTempOnSuccess(t *testing.T) {
80+
t.Parallel()
81+
dir := t.TempDir()
82+
target := filepath.Join(dir, "out.json")
83+
84+
if err := WriteFileAtomic(target, []byte("x"), 0o644); err != nil {
85+
t.Fatalf("WriteFileAtomic: %v", err)
86+
}
87+
88+
entries, err := os.ReadDir(dir)
89+
if err != nil {
90+
t.Fatalf("ReadDir: %v", err)
91+
}
92+
if len(entries) != 1 {
93+
names := make([]string, 0, len(entries))
94+
for _, e := range entries {
95+
names = append(names, e.Name())
96+
}
97+
t.Errorf("expected exactly one entry in dir, got %d: %v", len(entries), names)
98+
}
99+
for _, e := range entries {
100+
if strings.HasSuffix(e.Name(), ".tmp") {
101+
t.Errorf("leftover temp file: %s", e.Name())
102+
}
103+
}
104+
}
105+
106+
// CleansUpTempOnRenameFailure reaches the rename step and forces it to fail
107+
// (renaming a regular file onto a non-empty directory is rejected on every
108+
// POSIX filesystem, and on Windows). The removeTmp defer must clear the
109+
// orphan so /tmp doesn't accumulate junk across many failed writes.
110+
func TestWriteFileAtomic_CleansUpTempOnRenameFailure(t *testing.T) {
111+
t.Parallel()
112+
dir := t.TempDir()
113+
target := filepath.Join(dir, "out.json")
114+
if err := os.Mkdir(target, 0o755); err != nil {
115+
t.Fatalf("mkdir target: %v", err)
116+
}
117+
if err := os.WriteFile(filepath.Join(target, "occupant"), []byte("x"), 0o644); err != nil {
118+
t.Fatalf("seed dir: %v", err)
119+
}
120+
121+
err := WriteFileAtomic(target, []byte("x"), 0o644)
122+
if err == nil {
123+
t.Fatal("expected error when target is a non-empty directory")
124+
}
125+
126+
info, statErr := os.Stat(target)
127+
if statErr != nil {
128+
t.Fatalf("stat target: %v", statErr)
129+
}
130+
if !info.IsDir() {
131+
t.Error("target should still be a directory after failed rename")
132+
}
133+
134+
entries, err := os.ReadDir(dir)
135+
if err != nil {
136+
t.Fatalf("ReadDir: %v", err)
137+
}
138+
for _, e := range entries {
139+
if strings.HasSuffix(e.Name(), ".tmp") {
140+
t.Errorf("leftover temp file after failed rename: %s", e.Name())
141+
}
142+
}
143+
}
144+
145+
func TestWriteFileAtomic_ParentMissing(t *testing.T) {
146+
t.Parallel()
147+
dir := t.TempDir()
148+
target := filepath.Join(dir, "does-not-exist", "out.json")
149+
150+
err := WriteFileAtomic(target, []byte("x"), 0o644)
151+
if err == nil {
152+
t.Fatal("expected error when parent dir is missing")
153+
}
154+
if !errors.Is(err, os.ErrNotExist) {
155+
t.Errorf("expected ErrNotExist; got: %v", err)
156+
}
157+
}

0 commit comments

Comments
 (0)