Fix atomic object-cache publication

.github/workflows/ci.yml

@@ -4,6 +4,9 @@ on:
   pull_request:
     branches:
       - main
+  merge_group:
+    types:
+      - checks_requested
   push:
     branches:
       - main
@@ -35,8 +38,8 @@ jobs:
         env:
           EVENT_NAME: ${{ github.event_name }}
           EVENT_BEFORE: ${{ github.event.before }}
-          EVENT_BASE_SHA: ${{ github.event.pull_request.base.sha }}
-          EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+          EVENT_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}
+          EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }}
         run: bash scripts/detect-changes.sh
 
   quality:
@@ -136,6 +139,10 @@ jobs:
         run: bash scripts/build.sh
       - name: Diagnose installation
         run: "build/bin/peeper${{ matrix.exe }} doctor --json"
+      - name: Test native object-cache publication
+        env:
+          CCACHE_DISABLE: "1"
+        run: go test -count=1 -run '^TestCompileObject' -v ./cmd
       - name: Run source fixtures
         env:
           PEEPER_BIN: ${{ github.workspace }}/build/bin/peeper${{ matrix.exe }}

cmd/build.go

@@ -84,36 +84,10 @@ func buildExecutable(ctx *project.CompilerContext, entry *module.Module, outputP
 		if module == nil {
 			continue
 		}
-		ir := strings.TrimSpace(module.LLVMIR)
-		if ir == "" {
-			return fmt.Errorf("empty LLVM IR for module %s", module.ID.ImportPath)
-		}
-		cachePath := objectCachePath(ctx, profile, ir)
-		if cachePath != "" {
-			if _, err := os.Stat(cachePath); err == nil {
-				objectPaths = append(objectPaths, cachePath)
-				continue
-			} else if !os.IsNotExist(err) {
-				return fmt.Errorf("inspect cached object for %s: %w", module.ID.ImportPath, err)
-			}
-		}
-		llPath := filepath.Join(artifactDir, fmt.Sprintf("mod_%d.ll", i))
-		if err := os.WriteFile(llPath, []byte(ir), 0o644); err != nil {
-			return fmt.Errorf("write llvm ir: %w", err)
-		}
-		objectPath := filepath.Join(artifactDir, fmt.Sprintf("mod_%d.o", i))
-		if err := runCompilerTool(profile.ClangPath, profile.ObjectArgs(llPath, objectPath, ctx.Config.IsDebugBuild), "compile LLVM module "+module.ID.ImportPath); err != nil {
+		objectPath, err := compileObject(ctx, profile, module, filepath.Join(artifactDir, fmt.Sprintf("mod_%d", i)))
+		if err != nil {
 			return err
 		}
-		if cachePath != "" {
-			if err := os.MkdirAll(filepath.Dir(cachePath), 0o755); err != nil {
-				return fmt.Errorf("create object cache directory: %w", err)
-			}
-			if err := os.Rename(objectPath, cachePath); err != nil {
-				return fmt.Errorf("publish cached object: %w", err)
-			}
-			objectPath = cachePath
-		}
 		objectPaths = append(objectPaths, objectPath)
 	}
 	if len(objectPaths) == 0 {
@@ -144,6 +118,71 @@ func buildExecutable(ctx *project.CompilerContext, entry *module.Module, outputP
 	return replacePath(stagedPath, outputPath)
 }
 
+// compileObject owns one module's cache staging lifetime, ending before linking.
+func compileObject(ctx *project.CompilerContext, profile toolchain.Profile, module *module.Module, objectBase string) (string, error) {
+	ir := strings.TrimSpace(module.LLVMIR)
+	if ir == "" {
+		return "", fmt.Errorf("empty LLVM IR for module %s", module.ID.ImportPath)
+	}
+	cachePath := objectCachePath(ctx, profile, ir)
+	if cachePath != "" {
+		if exists, err := inspectCachedObject(cachePath); err != nil {
+			return "", fmt.Errorf("inspect cached object for %s: %w", module.ID.ImportPath, err)
+		} else if exists {
+			return cachePath, nil
+		}
+	}
+	llPath := objectBase + ".ll"
+	if err := os.WriteFile(llPath, []byte(ir), 0o644); err != nil {
+		return "", fmt.Errorf("write llvm ir: %w", err)
+	}
+	objectPath := objectBase + ".o"
+	if cachePath != "" {
+		cacheDir := filepath.Dir(cachePath)
+		if err := os.MkdirAll(cacheDir, 0o755); err != nil {
+			return "", fmt.Errorf("create object cache directory: %w", err)
+		}
+		// Clang must finish on the destination filesystem before publication.
+		stageDir, err := os.MkdirTemp(cacheDir, "."+filepath.Base(cachePath)+"-stage-*")
+		if err != nil {
+			return "", fmt.Errorf("stage cached object for %s: %w", module.ID.ImportPath, err)
+		}
+		defer os.RemoveAll(stageDir)
+		objectPath = filepath.Join(stageDir, filepath.Base(cachePath))
+	}
+	if err := runCompilerTool(profile.ClangPath, profile.ObjectArgs(llPath, objectPath, ctx.Config.IsDebugBuild), "compile LLVM module "+module.ID.ImportPath); err != nil {
+		return "", err
+	}
+	if cachePath != "" {
+		// A hard link installs the completed object without replacing a winner.
+		if linkErr := os.Link(objectPath, cachePath); linkErr != nil {
+			// Any failed link may race with another publisher. Reuse only a
+			// regular directory entry and preserve the publication error otherwise.
+			if exists, err := inspectCachedObject(cachePath); err != nil {
+				return "", fmt.Errorf("publish cached object: %w; inspect cache winner: %w", linkErr, err)
+			} else if !exists {
+				return "", fmt.Errorf("publish cached object: %w", linkErr)
+			}
+		}
+		return cachePath, nil
+	}
+	return objectPath, nil
+}
+
+func inspectCachedObject(path string) (bool, error) {
+	info, err := os.Lstat(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	if !info.Mode().IsRegular() {
+		return false, fmt.Errorf("%s is not a regular file", path)
+	}
+	return true, nil
+}
+
 func objectCachePath(ctx *project.CompilerContext, profile toolchain.Profile, ir string) string {
 	if ctx == nil || !profile.IsManaged || ctx.Config.RootDir == "" {
 		return ""

cmd/build_linux_test.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"encoding/binary"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"syscall"
+	"testing"
+)
+
+func TestCompileObjectNeverMovesPublishedCache(t *testing.T) {
+	ctx, profile, mod, base := objectCompileFixture(t, t.TempDir())
+	cachePath := objectCachePath(ctx, profile, strings.TrimSpace(mod.LLVMIR))
+	if err := os.MkdirAll(filepath.Dir(cachePath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	watch, err := syscall.InotifyInit1(syscall.IN_CLOEXEC | syscall.IN_NONBLOCK)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer syscall.Close(watch)
+	if _, err := syscall.InotifyAddWatch(watch, filepath.Dir(cachePath), syscall.IN_MOVED_FROM|syscall.IN_DELETE); err != nil {
+		t.Fatal(err)
+	}
+	// The fixture publishes a winner after our cache miss, before our publication.
+	// Watching directory events catches even a gap too short for polling readers.
+	t.Setenv("PEEPER_TEST_OBJECT_MODE", "winner")
+	if _, err := compileObject(ctx, profile, mod, base); err != nil {
+		t.Fatal(err)
+	}
+	var events [65536]byte
+	n, err := syscall.Read(watch, events[:])
+	if errors.Is(err, syscall.EAGAIN) {
+		return
+	}
+	if err != nil {
+		t.Fatal(err)
+	}
+	for offset := 0; offset < n; {
+		nameLen := int(binary.NativeEndian.Uint32(events[offset+12 : offset+16]))
+		name := strings.TrimRight(string(events[offset+16:offset+16+nameLen]), "\x00")
+		if name == filepath.Base(cachePath) {
+			t.Fatalf("published cache object was moved away or deleted: %s", name)
+		}
+		offset += syscall.SizeofInotifyEvent + nameLen
+	}
+}