From e24fc238d754e061cd07e867bf664b694e3288d5 Mon Sep 17 00:00:00 2001 From: fcying Date: Thu, 3 Sep 2026 17:39:05 +0800 Subject: [PATCH] fix: write compilation database atomically --- README.md | 6 +- internal/init.go | 121 ++++++++++++++- internal/init_test.go | 46 ++++++ internal/init_unix_test.go | 269 ++++++++++++++++++++++++++++++++++ internal/init_windows_test.go | 54 +++++++ 5 files changed, 485 insertions(+), 11 deletions(-) create mode 100644 internal/init_windows_test.go diff --git a/README.md b/README.md index 36f8f8f..a172969 100644 --- a/README.md +++ b/README.md @@ -166,9 +166,9 @@ compiledb make -C build all compiledb make -f core/main.mk CC=clang ``` -Use `make --cmd/-c` to select another GNU Make-compatible executable. This `-c` -belongs to the `make` subcommand and is separate from the top-level -`--command-style/-c` option: +Use `make --cmd/-c` to select another GNU Make-compatible executable for both +the real build and discovery. This `-c` belongs to the `make` subcommand and is +separate from the top-level `--command-style/-c` option: ```sh compiledb make --cmd gmake -C build diff --git a/internal/init.go b/internal/init.go index 3b05f60..6252cc1 100644 --- a/internal/init.go +++ b/internal/init.go @@ -3,7 +3,9 @@ package internal import ( "bytes" "context" + "crypto/rand" "encoding/json" + "fmt" "os" "os/exec" "path/filepath" @@ -223,6 +225,10 @@ func resolveLegacyCompilationDatabasePath(entry compilationDatabaseEntry, buildD } func loadCompilationDatabase(filename string) []json.RawMessage { + info, err := os.Stat(filename) + if err != nil || !info.Mode().IsRegular() { + return nil + } data, err := os.ReadFile(filename) if err != nil { return nil @@ -288,6 +294,112 @@ func mergeCompilationDatabase(entries []json.RawMessage, strict bool, buildDir s return filtered } +func resolveOutputFilename(filename string) (string, error) { + resolved, err := filepath.EvalSymlinks(filename) + if err == nil { + return resolved, nil + } + if !os.IsNotExist(err) { + return "", err + } + + info, lstatErr := os.Lstat(filename) + if lstatErr == nil { + if info.Mode()&os.ModeSymlink != 0 { + return "", err + } + return filename, nil + } + if !os.IsNotExist(lstatErr) { + return "", lstatErr + } + directory, base := filepath.Split(filename) + if directory == "" { + directory = "." + } + directory, err = filepath.EvalSymlinks(directory) + if err != nil { + return "", err + } + return filepath.Join(directory, base), nil +} + +func createAtomicTempFile(directory string, mode os.FileMode) (*os.File, error) { + prefix := filepath.Join(directory, ".compiledb-tmp-") + for range 100 { + outfile, err := os.OpenFile(prefix+rand.Text(), os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) + if os.IsExist(err) { + continue + } + return outfile, err + } + return nil, &os.PathError{Op: "open", Path: prefix + "*", Err: os.ErrExist} +} + +func writeFileInPlace(filename string, data []byte, mode os.FileMode) error { + outfile, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + return err + } + if _, err := outfile.Write(data); err != nil { + _ = outfile.Close() + return err + } + return outfile.Close() +} + +func replaceFileAtomically(filename string, data []byte, mode os.FileMode, targetExists bool) error { + outfile, err := createAtomicTempFile(filepath.Dir(filename), mode) + if err != nil { + return err + } + temporaryFilename := outfile.Name() + defer os.Remove(temporaryFilename) + + if targetExists { + if err := outfile.Chmod(mode); err != nil { + _ = outfile.Close() + return err + } + } + if _, err := outfile.Write(data); err != nil { + _ = outfile.Close() + return err + } + if err := outfile.Close(); err != nil { + return err + } + return os.Rename(temporaryFilename, filename) +} + +func writeFileAtomically(filename string, data []byte) error { + filename, err := resolveOutputFilename(filename) + if err != nil { + return err + } + + mode := os.FileMode(0o666) + targetExists := false + if info, statErr := os.Stat(filename); statErr == nil { + if !info.Mode().IsRegular() { + return fmt.Errorf("output %q is not a regular file", filename) + } + mode = info.Mode().Perm() + targetExists = true + } else if !os.IsNotExist(statErr) { + return statErr + } + + atomicErr := replaceFileAtomically(filename, data, mode, targetExists) + if atomicErr == nil { + return nil + } + if fallbackErr := writeFileInPlace(filename, data, mode); fallbackErr != nil { + return fmt.Errorf("atomic replace failed (%v), then in-place write failed: %w", atomicErr, fallbackErr) + } + return nil +} + func (t *Tool) WriteJSON(filename string, _ int, data *[]Command) { payload := []Command{} if data != nil && *data != nil { @@ -338,14 +450,7 @@ func (t *Tool) WriteJSON(filename string, _ int, data *[]Command) { t.Logger.Fatalf("create directory %v failed! err:%v", filepath.Dir(filename), err) } - outfile, err := os.Create(filename) - if err != nil { - t.Logger.Fatalf("create %v failed! err:%v", filename, err) - } - defer outfile.Close() - - _, err = outfile.Write(jsonData) - if err != nil { + if err := writeFileAtomically(filename, jsonData); err != nil { t.Logger.Fatalf("write %v failed! err:%v", filename, err) } t.Logger.Infof("write %d entries to %s", len(entries), filename) diff --git a/internal/init_test.go b/internal/init_test.go index 8d4d457..8c6d704 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -763,6 +763,52 @@ func TestWriteJSONStdoutUsesOnlyCurrentEntries(t *testing.T) { assertTestArgument(t, entries[0], 1, "-DTWO") } +func TestWriteJSONReplacesFileAtomically(t *testing.T) { + tmpDir := t.TempDir() + outputFile := filepath.Join(tmpDir, "compile_commands.json") + writeTestJSON(t, outputFile, []map[string]any{{ + "directory": "/project", + "command": "cc -DOLD -c main.c", + "file": "main.c", + }}) + original, err := os.Open(outputFile) + if err != nil { + t.Fatalf("open original output failed: %v", err) + } + before, statErr := original.Stat() + closeErr := original.Close() + if statErr != nil { + t.Fatalf("stat original output failed: %v", statErr) + } + if closeErr != nil { + t.Fatalf("close original output failed: %v", closeErr) + } + + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-DNEW", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + + after, err := os.Stat(outputFile) + if err != nil { + t.Fatalf("stat replaced output failed: %v", err) + } + if os.SameFile(before, after) { + t.Fatal("compilation database was rewritten in place instead of atomically replaced") + } + entries := readTestDatabase(t, outputFile) + if len(entries) != 1 || entries[0]["file"] != "main.c" { + t.Fatalf("unexpected replacement contents: %#v", entries) + } + assertTestArgument(t, entries[0], 1, "-DNEW") + directoryEntries, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("read output directory failed: %v", err) + } + if len(directoryEntries) != 1 || directoryEntries[0].Name() != filepath.Base(outputFile) { + t.Fatalf("atomic replacement left temporary files: %#v", directoryEntries) + } +} + func TestWriteJSONFileEndsWithOneNewline(t *testing.T) { outputFile := filepath.Join(t.TempDir(), "compile_commands.json") tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) diff --git a/internal/init_unix_test.go b/internal/init_unix_test.go index af12ccc..6f9e671 100644 --- a/internal/init_unix_test.go +++ b/internal/init_unix_test.go @@ -4,10 +4,13 @@ package internal import ( "context" + "encoding/json" + "errors" "os" "os/exec" "path/filepath" "slices" + "strconv" "strings" "syscall" "testing" @@ -46,6 +49,272 @@ func TestStrictSourceFileAcceptsOnlyRegularFiles(t *testing.T) { } } +func TestWriteFileAtomicallyRejectsNonRegularTarget(t *testing.T) { + tmpDir := t.TempDir() + outputFile := filepath.Join(tmpDir, "compile_commands.json") + if err := syscall.Mkfifo(outputFile, 0o600); err != nil { + t.Fatalf("create output FIFO failed: %v", err) + } + + err := writeFileAtomically(outputFile, []byte("[]\n")) + if err == nil || !strings.Contains(err.Error(), "not a regular file") { + t.Fatalf("non-regular output was not rejected: %v", err) + } + info, err := os.Lstat(outputFile) + if err != nil { + t.Fatalf("stat output FIFO failed: %v", err) + } + if info.Mode()&os.ModeNamedPipe == 0 { + t.Fatalf("output FIFO was replaced with mode %v", info.Mode()) + } + entries, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("read output directory failed: %v", err) + } + if len(entries) != 1 || entries[0].Name() != filepath.Base(outputFile) { + t.Fatalf("rejecting output FIFO left unexpected files: %#v", entries) + } +} + +func TestWriteJSONRejectsFIFOWithoutBlocking(t *testing.T) { + const ( + helperEnvironment = "COMPILEDB_TEST_WRITE_JSON_FIFO_HELPER" + outputEnvironment = "COMPILEDB_TEST_WRITE_JSON_FIFO_PATH" + ) + if parentPID, err := strconv.Atoi(os.Getenv(helperEnvironment)); err == nil && parentPID == os.Getppid() { + outputFile := os.Getenv(outputEnvironment) + info, statErr := os.Lstat(outputFile) + if statErr != nil || info.Mode()&os.ModeNamedPipe == 0 { + t.Fatalf("invalid FIFO helper output %q: %v", outputFile, statErr) + } + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + return + } + + tmpDir := t.TempDir() + outputFile := filepath.Join(tmpDir, "compile_commands.json") + if err := syscall.Mkfifo(outputFile, 0o600); err != nil { + t.Fatalf("create output FIFO failed: %v", err) + } + unrelatedFile := filepath.Join(tmpDir, "unrelated.json") + if err := os.WriteFile(unrelatedFile, []byte("unchanged\n"), 0o600); err != nil { + t.Fatalf("create unrelated output failed: %v", err) + } + t.Setenv(helperEnvironment, "ambient") + t.Setenv(outputEnvironment, unrelatedFile) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestWriteJSONRejectsFIFOWithoutBlocking$") + environment := os.Environ() + cmd.Env = make([]string, 0, len(environment)+2) + for _, value := range environment { + name, _, _ := strings.Cut(value, "=") + if name == helperEnvironment || name == outputEnvironment { + continue + } + cmd.Env = append(cmd.Env, value) + } + cmd.Env = append(cmd.Env, + helperEnvironment+"="+strconv.Itoa(os.Getpid()), + outputEnvironment+"="+outputFile, + ) + err := cmd.Run() + if ctx.Err() != nil { + t.Fatal("WriteJSON blocked while loading the output FIFO") + } + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 1 { + t.Fatalf("FIFO helper did not fail through WriteJSON: %v", err) + } + info, err := os.Lstat(outputFile) + if err != nil { + t.Fatalf("stat output FIFO failed: %v", err) + } + if info.Mode()&os.ModeNamedPipe == 0 { + t.Fatalf("WriteJSON replaced output FIFO with mode %v", info.Mode()) + } + unrelatedData, err := os.ReadFile(unrelatedFile) + if err != nil { + t.Fatalf("read unrelated output failed: %v", err) + } + if string(unrelatedData) != "unchanged\n" { + t.Fatalf("FIFO helper changed unrelated output: %q", unrelatedData) + } +} + +func TestWriteJSONPreservesOutputSymlink(t *testing.T) { + tmpDir := t.TempDir() + realDir := filepath.Join(tmpDir, "real") + physicalLinkDir := filepath.Join(realDir, "subdir") + if err := os.MkdirAll(physicalLinkDir, 0o755); err != nil { + t.Fatalf("create symlink test directories failed: %v", err) + } + target := filepath.Join(realDir, "database.json") + writeTestJSON(t, target, []Command{{Directory: "/project", Command: "cc -DOLD -c main.c", File: "main.c"}}) + aliasDir := filepath.Join(tmpDir, "alias") + if err := os.Symlink(filepath.Join("real", "subdir"), aliasDir); err != nil { + t.Fatalf("create parent directory symlink failed: %v", err) + } + outputFile := filepath.Join(aliasDir, "compile_commands.json") + if err := os.Symlink(filepath.Join("..", filepath.Base(target)), outputFile); err != nil { + t.Fatalf("create output symlink failed: %v", err) + } + + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-DNEW", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + + aliasInfo, err := os.Lstat(aliasDir) + if err != nil { + t.Fatalf("stat parent directory symlink failed: %v", err) + } + if aliasInfo.Mode()&os.ModeSymlink == 0 { + t.Fatal("atomic replacement replaced the parent directory symlink") + } + outputInfo, err := os.Lstat(outputFile) + if err != nil { + t.Fatalf("stat output symlink failed: %v", err) + } + if outputInfo.Mode()&os.ModeSymlink == 0 { + t.Fatal("atomic replacement replaced the output symlink") + } + entries := readTestDatabase(t, target) + if len(entries) != 1 { + t.Fatalf("unexpected target entries: %#v", entries) + } + assertTestArgument(t, entries[0], 1, "-DNEW") +} + +func TestResolveOutputFilenameRejectsDanglingSymlink(t *testing.T) { + outputFile := filepath.Join(t.TempDir(), "compile_commands.json") + if err := os.Symlink("missing.json", outputFile); err != nil { + t.Fatalf("create dangling output symlink failed: %v", err) + } + + if resolved, err := resolveOutputFilename(outputFile); err == nil { + t.Fatalf("resolved dangling output symlink as %q", resolved) + } +} + +func TestResolveOutputFilenameResolvesMissingParentBeforeDotDot(t *testing.T) { + tmpDir := t.TempDir() + realDir := filepath.Join(tmpDir, "real") + if err := os.MkdirAll(filepath.Join(realDir, "subdir"), 0o755); err != nil { + t.Fatalf("create physical output directory failed: %v", err) + } + aliasDir := filepath.Join(tmpDir, "alias") + if err := os.Symlink(filepath.Join("real", "subdir"), aliasDir); err != nil { + t.Fatalf("create parent directory symlink failed: %v", err) + } + separator := string(filepath.Separator) + outputFile := aliasDir + separator + ".." + separator + "compile_commands.json" + + resolved, err := resolveOutputFilename(outputFile) + if err != nil { + t.Fatalf("resolve missing output failed: %v", err) + } + physicalRealDir, err := filepath.EvalSymlinks(realDir) + if err != nil { + t.Fatalf("resolve physical output directory failed: %v", err) + } + want := filepath.Join(physicalRealDir, "compile_commands.json") + if resolved != want { + t.Fatalf("unexpected missing output target: want %q, got %q", want, resolved) + } +} + +func TestWriteJSONAcceptsLongOutputBasename(t *testing.T) { + outputFile := filepath.Join(t.TempDir(), strings.Repeat("x", 240)) + if err := os.WriteFile(outputFile, nil, 0o600); err != nil { + if errors.Is(err, syscall.ENAMETOOLONG) { + t.Skipf("filesystem rejects the long output basename: %v", err) + } + t.Fatalf("create long-name output failed: %v", err) + } + if err := os.Remove(outputFile); err != nil { + t.Fatalf("remove long-name output preflight file failed: %v", err) + } + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-c", "main.c"}, File: "main.c"}} + + tool.WriteJSON(outputFile, len(commands), &commands) + + entries := readTestDatabase(t, outputFile) + if len(entries) != 1 || entries[0]["file"] != "main.c" { + t.Fatalf("unexpected long-name output entries: %#v", entries) + } +} + +func TestWriteJSONPreservesOutputMode(t *testing.T) { + outputFile := filepath.Join(t.TempDir(), "compile_commands.json") + writeTestJSON(t, outputFile, []Command{{Directory: "/project", Command: "cc -DOLD -c main.c", File: "main.c"}}) + if err := os.Chmod(outputFile, 0o640); err != nil { + t.Fatalf("set original output mode failed: %v", err) + } + oldUmask := syscall.Umask(0o077) + defer syscall.Umask(oldUmask) + + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-DNEW", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + + info, err := os.Stat(outputFile) + if err != nil { + t.Fatalf("stat replaced output failed: %v", err) + } + if got := info.Mode().Perm(); got != 0o640 { + t.Fatalf("output mode changed: want 0640, got %04o", got) + } +} + +func TestWriteJSONNewOutputRespectsUmask(t *testing.T) { + outputFile := filepath.Join(t.TempDir(), "compile_commands.json") + oldUmask := syscall.Umask(0o077) + defer syscall.Umask(oldUmask) + + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + + info, err := os.Stat(outputFile) + if err != nil { + t.Fatalf("stat new output failed: %v", err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("new output ignored umask: want 0600, got %04o", got) + } +} + +func TestWriteJSONPreservesOpenReaderSnapshot(t *testing.T) { + outputFile := filepath.Join(t.TempDir(), "compile_commands.json") + writeTestJSON(t, outputFile, []Command{{Directory: "/project", Command: "cc -DOLD -c main.c", File: "main.c"}}) + reader, err := os.Open(outputFile) + if err != nil { + t.Fatalf("open original database failed: %v", err) + } + defer reader.Close() + + tool := newTestTool(t, Config{OutputFile: outputFile, NoStrict: true}) + commands := []Command{{Directory: "/project", Arguments: []string{"cc", "-DNEW", "-c", "main.c"}, File: "main.c"}} + tool.WriteJSON(outputFile, len(commands), &commands) + + var oldEntries []Command + if err := json.NewDecoder(reader).Decode(&oldEntries); err != nil { + t.Fatalf("decode original reader failed: %v", err) + } + if len(oldEntries) != 1 || oldEntries[0].File != "main.c" || !strings.Contains(oldEntries[0].Command, "-DOLD") { + t.Fatalf("open reader did not retain the old database: %#v", oldEntries) + } + newEntries := readTestDatabase(t, outputFile) + if len(newEntries) != 1 { + t.Fatalf("unexpected replacement entries: %#v", newEntries) + } + assertTestArgument(t, newEntries[0], 1, "-DNEW") +} + func TestExpandCompilerResponseFilesRejectsFIFO(t *testing.T) { workingDir := t.TempDir() if err := syscall.Mkfifo(filepath.Join(workingDir, "arguments.rsp"), 0o600); err != nil { diff --git a/internal/init_windows_test.go b/internal/init_windows_test.go new file mode 100644 index 0000000..1df338d --- /dev/null +++ b/internal/init_windows_test.go @@ -0,0 +1,54 @@ +//go:build windows + +package internal + +import ( + "bytes" + "os" + "path/filepath" + "testing" +) + +func TestWriteFileAtomicallyFallsBackWhenReaderBlocksReplacement(t *testing.T) { + tmpDir := t.TempDir() + outputFile := filepath.Join(tmpDir, "compile_commands.json") + if err := os.WriteFile(outputFile, []byte("old database\n"), 0o600); err != nil { + t.Fatalf("create original output failed: %v", err) + } + reader, err := os.Open(outputFile) + if err != nil { + t.Fatalf("open original output reader failed: %v", err) + } + defer reader.Close() + before, err := reader.Stat() + if err != nil { + t.Fatalf("stat original output reader failed: %v", err) + } + + newData := []byte("new database\n") + if err := writeFileAtomically(outputFile, newData); err != nil { + t.Fatalf("sharing-blocked replacement did not fall back: %v", err) + } + + after, err := os.Stat(outputFile) + if err != nil { + t.Fatalf("stat rewritten output failed: %v", err) + } + if !os.SameFile(before, after) { + t.Fatal("sharing-blocked replacement did not rewrite the original file in place") + } + got, err := os.ReadFile(outputFile) + if err != nil { + t.Fatalf("read rewritten output failed: %v", err) + } + if !bytes.Equal(got, newData) { + t.Fatalf("unexpected rewritten output: want %q, got %q", newData, got) + } + entries, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("read output directory failed: %v", err) + } + if len(entries) != 1 || entries[0].Name() != filepath.Base(outputFile) { + t.Fatalf("fallback left temporary files: %#v", entries) + } +}