Author SHA1 Message Date
sneak 76791d6e6c Rewrite script/test to the canonical race-enabled pattern (closes #67)
check / check (push) Failing after 0s
script/test now runs `go test -timeout 30s -race -cover ./...`, quiet on
success and rerunning verbose with `exit 1` on failure, per policy.

-race surfaced a real data race on the process-global apex/log logger:
Init reconfigures it (SetHandler/SetLevel) each CLI run while other
goroutines read it to log. internal/log now mutates the global under the
write lock and reads it under the read lock (new emit helper, DebugReal),
and drops WithError, whose returned Entry logged outside that lock.

That Entry was also the only thing printing a failed command's error to
stderr, so run() now reports it via log.Errorf (shown under -q too).

The corruption fuzz test shrinks (20000->1500 files, 500->100 iters) to
keep the suite under 20s with -race.

Model: opus-4-8
2026-09-21 08:00:35 +00:00
clawbot 6de3f1d714 Add .editorconfig, broaden .gitignore, drop dead Drone refs (closes #72)
check / check (push) Failing after 1s
Add the canonical .editorconfig from sneak/prompts verbatim. Broaden .gitignore to cover secrets (.env, .env.*, *.key, *.pem), OS files, editor files, and Go artifacts while keeping every repo-specific entry; no tracked file becomes ignored, and bin/gitrev.sh stays tracked despite the /bin/ rule. Remove the stale .drone.yml ignore entry and the DRONE_COMMIT_SHA branch from bin/gitrev.sh, left from the Gitea Actions migration; the GITREV and git-describe paths still work.

A reader may trip over .editorconfig saying four spaces for every file while Go files are tab-indented: gofmt governs Go, editorconfig does not, and the file is taken verbatim by policy.

Disclosure: the worker's cold docker build hit the 10s test timeout in the gpg signature test; this change has no Go code, and that timeout is tracked by issues 67 and 62.

Model: opus-4-8 (implementation); fable-5-1 (merge)
2026-09-21 09:56:15 +02:00
8 changed files with 93 additions and 151 deletions
+12
View File
@@ -0,0 +1,12 @@
root = true
[*]
indent_style = space
indent_size = 4
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
[Makefile]
indent_style = tab
+21 -2
View File
@@ -10,5 +10,24 @@ modcache.tzst
# Generated manifest files # Generated manifest files
.index.mf .index.mf
# Stale files # Secrets
.drone.yml .env
.env.*
*.key
*.pem
# OS files
.DS_Store
Thumbs.db
# Editor files
*.swp
*.swo
*~
.idea/
.vscode/
# Go build artifacts
*.log
*.out
*.test
+6 -3
View File
@@ -24,8 +24,12 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
# Completed Steps # Completed Steps
- 2026-09-21: fixed the `-v` collision between `--verbose` and `--version`; - 2026-09-21: rewrote `script/test` to the canonical pattern (30s timeout,
verbose owns `-v`, and version answers to `--version` and `-V` (#64) `-race -cover`, quiet-first with verbose-on-failure rerun) and fixed the
process-global logger data race it surfaced (#67)
- 2026-09-21: added the canonical `.editorconfig`, made `.gitignore` cover
secrets, OS, editor, and Go artifacts, and removed the dead Drone CI
references from `.gitignore` and `bin/gitrev.sh` (#72)
- 2026-08-09: added `.prettierrc`/`.prettierignore`, gave `script/fmt` and - 2026-08-09: added `.prettierrc`/`.prettierignore`, gave `script/fmt` and
`script/fmt-check` one shared prettier file set via `script/prettier`, dropped `script/fmt-check` one shared prettier file set via `script/prettier`, dropped
the `|| true` that hid prettier failures, and added a node-based Dockerfile the `|| true` that hid prettier failures, and added a node-based Dockerfile
@@ -64,7 +68,6 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
- Move FORMAT.md from repo root to docs/ and update the AGENTS.md reference - Move FORMAT.md from repo root to docs/ and update the AGENTS.md reference
- Pin Makefile-installed Go tools (`protoc-gen-go@v1.28.1`, - Pin Makefile-installed Go tools (`protoc-gen-go@v1.28.1`,
`golangci-lint@v2.12.2`) by module hash, not mutable tag `golangci-lint@v2.12.2`) by module hash, not mutable tag
- Set `make test` timeout to 30s (currently 10s)
- Add explicit README "Rationale" heading (content exists under other - Add explicit README "Rationale" heading (content exists under other
names); name the author in the README Description first line names); name the author in the README Description first line
- Reconcile root-level AGENTS.md with directory-hygiene policy (keep or - Reconcile root-level AGENTS.md with directory-hygiene policy (keep or
-5
View File
@@ -1,10 +1,5 @@
#!/bin/bash #!/bin/bash
# #
if [[ ! -z "$DRONE_COMMIT_SHA" ]]; then
echo "${DRONE_COMMIT_SHA:0:7}"
exit 0
fi
if [[ ! -z "$GITREV" ]]; then if [[ ! -z "$GITREV" ]]; then
echo $GITREV echo $GITREV
else else
+11 -73
View File
@@ -27,7 +27,6 @@ const (
testManifest = "/manifest.mf" testManifest = "/manifest.mf"
testFlagBase = "--base" testFlagBase = "--base"
testFlagNoExtra = "--no-extra-files" testFlagNoExtra = "--no-extra-files"
testFlagVersion = "--version"
) )
var errSimulatedWrite = errors.New("simulated write failure") var errSimulatedWrite = errors.New("simulated write failure")
@@ -103,7 +102,7 @@ func TestVersionCommand(t *testing.T) {
t.Parallel() t.Parallel()
fs := afero.NewMemMapFs() fs := afero.NewMemMapFs()
opts := testOpts([]string{testApp, cmdVersion}, fs) opts := testOpts([]string{testApp, "version"}, fs)
exitCode := runCLI(opts) exitCode := runCLI(opts)
@@ -114,69 +113,6 @@ func TestVersionCommand(t *testing.T) {
assert.Contains(t, stdout, "abc123") assert.Contains(t, stdout, "abc123")
} }
// TestVFlagCollision covers the -v/--verbose vs --version flag interaction
// (issue #64). Verbose owns -v; version answers to --version and -V. None of
// these invocations may produce a parser error, and the two ways of asking
// for the version must print the same thing.
func TestVFlagCollision(t *testing.T) {
t.Parallel()
// Invocations that must print the version and exit 0.
versionCases := map[string][]string{
"long version flag": {testApp, testFlagVersion},
"short version flag": {testApp, "-V"},
"verbose then version": {testApp, "-v", testFlagVersion},
"long verbose and version": {testApp, "--verbose", testFlagVersion},
}
for name, args := range versionCases {
t.Run(name, func(t *testing.T) {
t.Parallel()
opts := testOpts(args, afero.NewMemMapFs())
exitCode := runCLI(opts)
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStdout(t, opts), mfer.Version)
assert.NotContains(t, testStderr(t, opts), "two forms of the same flag")
})
}
// Invocations that must enable verbose and exit 0 without a parser error.
verboseCases := map[string][]string{
"short verbose flag": {testApp, "-v"},
"long verbose flag": {testApp, "--verbose"},
}
for name, args := range verboseCases {
t.Run(name, func(t *testing.T) {
t.Parallel()
opts := testOpts(args, afero.NewMemMapFs())
exitCode := runCLI(opts)
assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStdout(t, opts), cmdGenerate,
"root should show help listing subcommands")
assert.Empty(t, testStderr(t, opts))
})
}
}
// TestVersionFlagAndCommandMatch asserts that "mfer --version" and
// "mfer version" produce identical output (issue #64).
func TestVersionFlagAndCommandMatch(t *testing.T) {
t.Parallel()
flagOpts := testOpts([]string{testApp, testFlagVersion}, afero.NewMemMapFs())
require.Equal(t, 0, runCLI(flagOpts))
cmdOpts := testOpts([]string{testApp, cmdVersion}, afero.NewMemMapFs())
require.Equal(t, 0, runCLI(cmdOpts))
assert.Equal(t, testStdout(t, flagOpts), testStdout(t, cmdOpts))
}
func TestHelpCommand(t *testing.T) { func TestHelpCommand(t *testing.T) {
t.Parallel() t.Parallel()
@@ -743,11 +679,13 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
fs := afero.NewMemMapFs() fs := afero.NewMemMapFs()
rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data
// Create many small files with random names to generate a ~1MB manifest // Create many small files with random names so the manifest has many
// Each manifest entry is roughly 50-60 bytes, so we need ~20000 files // entries and random single-byte flips land at varied offsets. Each
// manifest entry is roughly 50-60 bytes. Kept modest so the suite stays
// within its wall-clock budget under -race.
require.NoError(t, fs.MkdirAll(testDir, 0o755)) require.NoError(t, fs.MkdirAll(testDir, 0o755))
numFiles := 20000 numFiles := 1500
for range numFiles { for range numFiles {
// Generate random filename // Generate random filename
filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat", filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat",
@@ -763,11 +701,11 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode := runCLI(opts) exitCode := runCLI(opts)
require.Equal(t, 0, exitCode, "generate should succeed") require.Equal(t, 0, exitCode, "generate should succeed")
// Read the valid manifest and verify it's approximately 1MB // Read the valid manifest and verify it has real size.
validManifest, err := afero.ReadFile(fs, testManifest) validManifest, err := afero.ReadFile(fs, testManifest)
require.NoError(t, err) require.NoError(t, err)
require.GreaterOrEqual(t, len(validManifest), 1024*1024, require.GreaterOrEqual(t, len(validManifest), 64*1024,
"manifest should be at least 1MB, got %d bytes", len(validManifest)) "manifest should be at least 64KB, got %d bytes", len(validManifest))
t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles) t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles)
// First corruption: truncate the manifest // First corruption: truncate the manifest
@@ -790,8 +728,8 @@ func TestCheckDetectsManifestCorruption(t *testing.T) {
exitCode = runCLI(opts) exitCode = runCLI(opts)
require.Equal(t, 0, exitCode, "check should pass with valid manifest") require.Equal(t, 0, exitCode, "check should pass with valid manifest")
// Now do 500 random corruption iterations // Now do 100 random corruption iterations
for i := range 500 { for i := range 100 {
// Corrupt: write a random byte at a random offset // Corrupt: write a random byte at a random offset
corrupted := make([]byte, len(validManifest)) corrupted := make([]byte, len(validManifest))
copy(corrupted, validManifest) copy(corrupted, validManifest)
+3 -26
View File
@@ -18,7 +18,6 @@ const (
cmdGenerate = "generate" cmdGenerate = "generate"
cmdCheck = "check" cmdCheck = "check"
cmdExport = "export" cmdExport = "export"
cmdVersion = "version"
flagProgress = "progress" flagProgress = "progress"
@@ -68,12 +67,6 @@ func (mfa *CLIApp) VersionString() string {
return mfer.Version return mfer.Version
} }
// printVersion writes the version line shared by the --version flag and the
// version subcommand, so both produce identical output.
func (mfa *CLIApp) printVersion() {
_, _ = fmt.Fprintf(mfa.Stdout, "%s version %s\n", mfa.appname, mfa.VersionString())
}
func (mfa *CLIApp) printBanner() { func (mfa *CLIApp) printBanner() {
if log.GetLevel() <= log.InfoLevel { if log.GetLevel() <= log.InfoLevel {
_, _ = fmt.Fprintln(mfa.Stdout, banner) _, _ = fmt.Fprintln(mfa.Stdout, banner)
@@ -272,10 +265,10 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
func (mfa *CLIApp) versionCommand() *cli.Command { func (mfa *CLIApp) versionCommand() *cli.Command {
return &cli.Command{ return &cli.Command{
Name: cmdVersion, Name: "version",
Usage: "Show version", Usage: "Show version",
Action: func(_ *cli.Context) error { Action: func(_ *cli.Context) error {
mfa.printVersion() _, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString())
return nil return nil
}, },
@@ -331,20 +324,6 @@ func (mfa *CLIApp) run(args []string) {
log.SetOutput(mfa.Stdout, mfa.Stderr) log.SetOutput(mfa.Stdout, mfa.Stderr)
log.Init() log.Init()
// -v means verbose, not version. urfave/cli's built-in version flag
// claims -v by default, colliding with the -v verbose alias used here and
// on every subcommand; that collision makes "mfer -v --version" fail to
// parse. Verbose is the more common meaning of -v in tools that offer
// both, so verbose keeps -v and the version flag takes the capital -V.
// VersionFlag and VersionPrinter are urfave/cli package globals; run() is
// serialized in tests, so assigning them here is safe.
cli.VersionFlag = &cli.BoolFlag{
Name: cmdVersion,
Aliases: []string{"V"},
Usage: "print the version",
}
cli.VersionPrinter = func(_ *cli.Context) { mfa.printVersion() }
mfa.app = &cli.App{ mfa.app = &cli.App{
Name: mfa.appname, Name: mfa.appname,
Usage: "Manifest generator", Usage: "Manifest generator",
@@ -352,13 +331,11 @@ func (mfa *CLIApp) run(args []string) {
EnableBashCompletion: true, EnableBashCompletion: true,
Writer: mfa.Stdout, Writer: mfa.Stdout,
ErrWriter: mfa.Stderr, ErrWriter: mfa.Stderr,
Flags: commonFlags(),
Action: func(c *cli.Context) error { Action: func(c *cli.Context) error {
if c.Args().Len() > 0 { if c.Args().Len() > 0 {
return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First()) return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First())
} }
mfa.setVerbosity(c)
mfa.printBanner() mfa.printBanner()
return cli.ShowAppHelp(c) return cli.ShowAppHelp(c)
@@ -380,6 +357,6 @@ func (mfa *CLIApp) run(args []string) {
if err != nil { if err != nil {
mfa.exitCode = 1 mfa.exitCode = 1
log.WithError(err).Debugf("exiting") log.Errorf("%s", err)
} }
} }
+34 -41
View File
@@ -112,13 +112,16 @@ func DisableStyling() {
} }
// Init initializes the logger with the CLI handler and default log level. // Init initializes the logger with the CLI handler and default log level.
//
// It reconfigures the process-global apex/log logger under the write lock so
// the global is never mutated while another goroutine holds the read lock to
// read it in emit. Without this, parallel callers (e.g. the test suite) race
// Init's SetLevel/SetHandler against concurrent log calls.
func Init() { func Init() {
mu.RLock() mu.Lock()
defer mu.Unlock()
w := stderr log.SetHandler(acli.New(stderr))
mu.RUnlock()
log.SetHandler(acli.New(w))
log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves
} }
@@ -130,74 +133,66 @@ func isEnabled(l Level) bool {
return l >= currentLevel return l >= currentLevel
} }
// emit calls fn while holding the read lock if messages at level l are
// enabled. Holding the read lock across the apex/log call keeps the global
// logger from being read while Init reconfigures it under the write lock.
func emit(l Level, fn func()) {
mu.RLock()
defer mu.RUnlock()
if l >= currentLevel {
fn()
}
}
// Fatalf logs a formatted message at fatal level. // Fatalf logs a formatted message at fatal level.
func Fatalf(format string, args ...any) { func Fatalf(format string, args ...any) {
if isEnabled(FatalLevel) { emit(FatalLevel, func() { log.Fatalf(format, args...) })
log.Fatalf(format, args...)
}
} }
// Fatal logs a message at fatal level. // Fatal logs a message at fatal level.
func Fatal(arg string) { func Fatal(arg string) {
if isEnabled(FatalLevel) { emit(FatalLevel, func() { log.Fatal(arg) })
log.Fatal(arg)
}
} }
// Errorf logs a formatted message at error level. // Errorf logs a formatted message at error level.
func Errorf(format string, args ...any) { func Errorf(format string, args ...any) {
if isEnabled(ErrorLevel) { emit(ErrorLevel, func() { log.Errorf(format, args...) })
log.Errorf(format, args...)
}
} }
// Error logs a message at error level. // Error logs a message at error level.
func Error(arg string) { func Error(arg string) {
if isEnabled(ErrorLevel) { emit(ErrorLevel, func() { log.Error(arg) })
log.Error(arg)
}
} }
// Warnf logs a formatted message at warn level. // Warnf logs a formatted message at warn level.
func Warnf(format string, args ...any) { func Warnf(format string, args ...any) {
if isEnabled(WarnLevel) { emit(WarnLevel, func() { log.Warnf(format, args...) })
log.Warnf(format, args...)
}
} }
// Warn logs a message at warn level. // Warn logs a message at warn level.
func Warn(arg string) { func Warn(arg string) {
if isEnabled(WarnLevel) { emit(WarnLevel, func() { log.Warn(arg) })
log.Warn(arg)
}
} }
// Infof logs a formatted message at info level. // Infof logs a formatted message at info level.
func Infof(format string, args ...any) { func Infof(format string, args ...any) {
if isEnabled(InfoLevel) { emit(InfoLevel, func() { log.Infof(format, args...) })
log.Infof(format, args...)
}
} }
// Info logs a message at info level. // Info logs a message at info level.
func Info(arg string) { func Info(arg string) {
if isEnabled(InfoLevel) { emit(InfoLevel, func() { log.Info(arg) })
log.Info(arg)
}
} }
// Verbosef logs a formatted message at verbose level. // Verbosef logs a formatted message at verbose level.
func Verbosef(format string, args ...any) { func Verbosef(format string, args ...any) {
if isEnabled(VerboseLevel) { emit(VerboseLevel, func() { log.Infof(format, args...) })
log.Infof(format, args...)
}
} }
// Verbose logs a message at verbose level. // Verbose logs a message at verbose level.
func Verbose(arg string) { func Verbose(arg string) {
if isEnabled(VerboseLevel) { emit(VerboseLevel, func() { log.Info(arg) })
log.Info(arg)
}
} }
// Debugf logs a formatted message at debug level with caller location. // Debugf logs a formatted message at debug level with caller location.
@@ -216,7 +211,10 @@ func Debug(arg string) {
// DebugReal logs at debug level with caller info from the specified stack depth. // DebugReal logs at debug level with caller info from the specified stack depth.
func DebugReal(arg string, cs int) { func DebugReal(arg string, cs int) {
if !isEnabled(DebugLevel) { mu.RLock()
defer mu.RUnlock()
if DebugLevel < currentLevel {
return return
} }
@@ -275,11 +273,6 @@ func GetLevel() Level {
return currentLevel return currentLevel
} }
// WithError returns a log entry with the error attached.
func WithError(e error) *log.Entry {
return log.Log.WithError(e)
}
// Progressf prints a progress message that overwrites the current line. // Progressf prints a progress message that overwrites the current line.
// Use ProgressDone() when progress is complete to move to the next line. // Use ProgressDone() when progress is complete to move to the next line.
func Progressf(format string, args ...any) { func Progressf(format string, args ...any) {
+6 -1
View File
@@ -17,7 +17,12 @@ ensure_pb() {
main() { main() {
cd "$ROOT" cd "$ROOT"
ensure_pb ensure_pb
go test -v --timeout 10s ./... go test -timeout 30s -race -cover ./... ||
{
echo "--- Rerunning with -v for details ---"
go test -timeout 30s -race -v ./...
exit 1
}
} }
main "$@" main "$@"