From c63f77724d98e50d7c4efb0cf9d41f422ecf258a Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 15 Oct 2026 12:21:18 +0900 Subject: [PATCH 1/3] refactor: move the record store into the resultstore package The record store becomes resultstore.Store[D], opened with Open[D]. Its records, file names and helpers are unexported, so compilecheck can only use Reusable, RecordCompile, LoadBaseline, GeneratorOutputUnchanged, RecordBaseline, GeneratedDirectory and Forget. D is the caller's diagnostic type, which the store keeps without reading. The non-source key crosses as KeyDigest, which nonSourceIdentity converts to and from without copying. The package imports only the standard library. Its own tests use a stand-in diagnostic type with the caller's field order, so the golden bytes are unchanged. --- .../internal/compilecheck/baseline_record.go | 11 +- .../compilecheck/baseline_record_test.go | 5 +- .../internal/compilecheck/compiler.go | 6 +- .../internal/compilecheck/fast_path.go | 13 +- .../compilecheck/fast_path_fixture_test.go | 7 +- .../compilecheck/fast_path_order_test.go | 2 +- .../internal/compilecheck/fast_path_test.go | 17 +- .../internal/compilecheck/record_store.go | 206 ------------------ .../internal/compilecheck/result_cache.go | 12 +- .../compilecheck/result_cache_write_test.go | 2 +- .../baseline.go} | 99 +++++---- .../baseline_test.go} | 8 +- .../resultstore/compile_result.go | 133 +++++++++++ .../compile_result_test.go} | 40 ++-- .../compilecheck/resultstore/fixture_test.go | 52 +++++ .../{ => resultstore}/record_format_test.go | 16 +- .../compilecheck/resultstore/store.go | 134 ++++++++++++ .../compilecheck/scheduler_cache_test.go | 10 +- 18 files changed, 452 insertions(+), 321 deletions(-) delete mode 100644 cli/dispatcher/internal/compilecheck/record_store.go rename cli/dispatcher/internal/compilecheck/{record_store_baseline.go => resultstore/baseline.go} (77%) rename cli/dispatcher/internal/compilecheck/{record_store_baseline_test.go => resultstore/baseline_test.go} (84%) create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/compile_result.go rename cli/dispatcher/internal/compilecheck/{record_store_test.go => resultstore/compile_result_test.go} (87%) create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go rename cli/dispatcher/internal/compilecheck/{ => resultstore}/record_format_test.go (90%) create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/store.go diff --git a/cli/dispatcher/internal/compilecheck/baseline_record.go b/cli/dispatcher/internal/compilecheck/baseline_record.go index 360045d57..38be92048 100644 --- a/cli/dispatcher/internal/compilecheck/baseline_record.go +++ b/cli/dispatcher/internal/compilecheck/baseline_record.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck" + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // RecordBaseline records what the next changed-files check of a unit compares against, after a @@ -56,9 +57,9 @@ func (runner *fastPathRunner) RecordBaseline( return err } - return c.records(plan).recordBaseline(unit.Assembly.AssemblyName, baselineInputs{ + return c.records(plan).RecordBaseline(unit.Assembly.AssemblyName, resultstore.BaselineInputs{ InputKey: keyResult.Key, - NonSourceKey: nonSourceKey, + NonSourceKey: resultstore.KeyDigest(nonSourceKey), WorkerKey: worker.Key, Files: files, }) @@ -67,8 +68,8 @@ func (runner *fastPathRunner) RecordBaseline( // baselineFiles pairs each source's content hash, as the input key read it, with its fingerprint. func baselineFiles( projectRoot string, unit CompileUnit, keyResult inputKeyResult, fingerprints fastcheck.FingerprintResponse, -) (map[string]baselineFile, error) { - files := make(map[string]baselineFile, len(unit.Sources)) +) (map[string]resultstore.BaselineFile, error) { + files := make(map[string]resultstore.BaselineFile, len(unit.Sources)) for _, source := range unit.Sources { absolute := projectPath(projectRoot, source) fingerprint, hasFingerprint := fingerprints.Fingerprints[absolute] @@ -79,7 +80,7 @@ func baselineFiles( if contentHash != keyResult.SourceHashes[source] { return nil, fmt.Errorf("source changed during the compile: %s", source) } - files[source] = baselineFile{ContentHash: contentHash, Fingerprint: fingerprint} + files[source] = resultstore.BaselineFile{ContentHash: contentHash, Fingerprint: fingerprint} } return files, nil diff --git a/cli/dispatcher/internal/compilecheck/baseline_record_test.go b/cli/dispatcher/internal/compilecheck/baseline_record_test.go index 6667ad622..b84ce7279 100644 --- a/cli/dispatcher/internal/compilecheck/baseline_record_test.go +++ b/cli/dispatcher/internal/compilecheck/baseline_record_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck" + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // Verifies that a successful full compile records a baseline whose content hashes are the ones the @@ -125,7 +126,7 @@ func TestCompileUnitPassesGeneratedFilesOut(t *testing.T) { written, _ := compileSecondUnit(t, "", 0) plan := newCompilerPlan() - want := quoteFlag(generatedFilesOutFlagPrefix, filepath.ToSlash(openRecordStore("", plan.OutputDir).generatedDirectory(plan.Units[1].Assembly.AssemblyName))) + want := quoteFlag(generatedFilesOutFlagPrefix, filepath.ToSlash(resultstore.Open[Diagnostic]("", plan.OutputDir).GeneratedDirectory(plan.Units[1].Assembly.AssemblyName))) if !strings.Contains(written, want) { t.Errorf("expected %s in the response file, got:\n%s", want, written) } @@ -217,7 +218,7 @@ func TestCompileUnitCreatesGeneratedFilesOutDirectory(t *testing.T) { ProjectRoot: projectRoot, Timeout: time.Minute, Run: func(_ context.Context, _ *exec.Cmd) (string, string, int, error) { - info, err := os.Stat(filepath.Join(projectRoot, openRecordStore(projectRoot, plan.OutputDir).generatedDirectory(plan.Units[1].Assembly.AssemblyName))) + info, err := os.Stat(filepath.Join(projectRoot, resultstore.Open[Diagnostic](projectRoot, plan.OutputDir).GeneratedDirectory(plan.Units[1].Assembly.AssemblyName))) existed = err == nil && info.IsDir() return "", "", 0, nil }, diff --git a/cli/dispatcher/internal/compilecheck/compiler.go b/cli/dispatcher/internal/compilecheck/compiler.go index 530362d8b..aa2ffacaa 100644 --- a/cli/dispatcher/internal/compilecheck/compiler.go +++ b/cli/dispatcher/internal/compilecheck/compiler.go @@ -108,7 +108,7 @@ func (c Compiler) CompileUnit( // Why the generator output directory is created here: the csc of Roslyn 4.3 (Unity 2022.3) creates // the folder of each generator under it but not the directory itself, and fails every generated // file with CS0016 when it is missing. Newer Roslyn creates it either way. - generatedPath := filepath.Join(c.ProjectRoot, c.records(plan).generatedDirectory(unit.Assembly.AssemblyName)) + generatedPath := filepath.Join(c.ProjectRoot, c.records(plan).GeneratedDirectory(unit.Assembly.AssemblyName)) if err := os.MkdirAll(generatedPath, outputDirPermissions); err != nil { return UnitResult{}, fmt.Errorf("failed to create %s: %w", generatedPath, err) } @@ -116,7 +116,7 @@ func (c Compiler) CompileUnit( // Why the generator output is saved: a later fast check reads it instead of running the generators. responseFilePath, err := writeRewrittenResponseFile(c.ProjectRoot, plan, unit, unit.Assembly.AssemblyName+responseFileExtension, - []string{quoteFlag(generatedFilesOutFlagPrefix, filepath.ToSlash(c.records(plan).generatedDirectory(unit.Assembly.AssemblyName)))}) + []string{quoteFlag(generatedFilesOutFlagPrefix, filepath.ToSlash(c.records(plan).GeneratedDirectory(unit.Assembly.AssemblyName)))}) if err != nil { return UnitResult{}, err } @@ -168,7 +168,7 @@ func (c Compiler) removeUnitOutputs(plan BuildPlan, unit CompileUnit) error { } } - return c.records(plan).forget(unit.Assembly.AssemblyName) + return c.records(plan).Forget(unit.Assembly.AssemblyName) } // buildUnitResult turns one compiler invocation's output into the result for its assembly. diff --git a/cli/dispatcher/internal/compilecheck/fast_path.go b/cli/dispatcher/internal/compilecheck/fast_path.go index 5665398e1..5c9dfaa9a 100644 --- a/cli/dispatcher/internal/compilecheck/fast_path.go +++ b/cli/dispatcher/internal/compilecheck/fast_path.go @@ -10,6 +10,7 @@ import ( "time" "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck" + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) const ( @@ -78,7 +79,7 @@ func (runner *fastPathRunner) Try( nonSourceKey nonSourceIdentity, ) fastPathAttempt { records := c.records(plan) - baseline, reason := records.loadBaseline(unit.Assembly.AssemblyName) + baseline, reason := records.LoadBaseline(unit.Assembly.AssemblyName) if reason != "" { return fallBack(reason) } @@ -88,10 +89,10 @@ func (runner *fastPathRunner) Try( } if nonSourceKey.Key != baseline.NonSourceKey.Key { return fallBack("inputs other than source contents changed: " + - strings.Join(changedInputKinds(nonSourceKey, baseline.NonSourceKey), ", ")) + strings.Join(changedInputKinds(nonSourceKey, nonSourceIdentity(baseline.NonSourceKey)), ", ")) } - if !records.generatorOutputUnchanged(baseline) { + if !records.GeneratorOutputUnchanged(baseline) { return fallBack("generator output of the baseline is gone") } @@ -114,7 +115,7 @@ func (runner *fastPathRunner) Try( // last full compile reported for the files that did not change. func (runner *fastPathRunner) check( ctx context.Context, c Compiler, plan BuildPlan, unit CompileUnit, - baseline storedBaseline, changed []string, + baseline resultstore.Baseline[Diagnostic], changed []string, ) fastPathAttempt { startedAt := time.Now() worker, err := runner.Worker.Ensure(ctx) @@ -174,7 +175,7 @@ func projectRelativeReason(projectRoot string, reason string, changed []string) // buildCheckRequest names, in absolute paths, what the worker checks and against which baseline. func buildCheckRequest( - projectRoot string, baseline storedBaseline, responseFilePath string, changed []string, + projectRoot string, baseline resultstore.Baseline[Diagnostic], responseFilePath string, changed []string, ) fastcheck.CheckRequest { request := fastcheck.CheckRequest{ ProjectRoot: projectRoot, @@ -196,7 +197,7 @@ func buildCheckRequest( // changedSources lists the sources whose content differs from the baseline, using the hashes the // input key read rather than reading the files again. -func changedSources(unit CompileUnit, keyResult inputKeyResult, files map[string]baselineFile) []string { +func changedSources(unit CompileUnit, keyResult inputKeyResult, files map[string]resultstore.BaselineFile) []string { changed := []string{} for _, source := range unit.Sources { if keyResult.SourceHashes[source] != files[source].ContentHash { diff --git a/cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go b/cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go index cf9ff5458..312bd215b 100644 --- a/cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go +++ b/cli/dispatcher/internal/compilecheck/fast_path_fixture_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck" + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // fakeDeclarationSeparator splits a fixture source into its declaration part and its body part, so @@ -227,9 +228,9 @@ func (project *fastPathProject) hasBaseline() bool { } // baseline reads Alpha's baseline through the store and fails the test when there is none to use. -func (project *fastPathProject) baseline(t *testing.T) storedBaseline { +func (project *fastPathProject) baseline(t *testing.T) resultstore.Baseline[Diagnostic] { t.Helper() - baseline, reason := project.records().loadBaseline("Alpha") + baseline, reason := project.records().LoadBaseline("Alpha") if reason != "" { t.Fatalf("expected a usable baseline for Alpha, got %q", reason) } @@ -238,7 +239,7 @@ func (project *fastPathProject) baseline(t *testing.T) storedBaseline { } // generatedFileCount counts the files under the generator output a baseline points at. -func (project *fastPathProject) generatedFileCount(t *testing.T, baseline storedBaseline) int { +func (project *fastPathProject) generatedFileCount(t *testing.T, baseline resultstore.Baseline[Diagnostic]) int { t.Helper() count := 0 err := filepath.WalkDir(projectPath(project.root, baseline.GeneratedDirectory), diff --git a/cli/dispatcher/internal/compilecheck/fast_path_order_test.go b/cli/dispatcher/internal/compilecheck/fast_path_order_test.go index 858fc7362..e1002a4f7 100644 --- a/cli/dispatcher/internal/compilecheck/fast_path_order_test.go +++ b/cli/dispatcher/internal/compilecheck/fast_path_order_test.go @@ -13,7 +13,7 @@ import ( // matches the baseline, and the check that it is gone could run anywhere in the order unnoticed. func (project *fastPathProject) recordBaselineWithGeneratorOutput(t *testing.T) string { t.Helper() - generatedDirectoryPath := project.records().generatedDirectory("Alpha") + generatedDirectoryPath := project.records().GeneratedDirectory("Alpha") recorder := newCachedSchedulerRecorder("Alpha", "Beta") recorder.during = func(unit string) { if unit == "Alpha" { diff --git a/cli/dispatcher/internal/compilecheck/fast_path_test.go b/cli/dispatcher/internal/compilecheck/fast_path_test.go index 4f9389081..d2b7149f0 100644 --- a/cli/dispatcher/internal/compilecheck/fast_path_test.go +++ b/cli/dispatcher/internal/compilecheck/fast_path_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/fastcheck" + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // Verifies that unchanged inputs are reused before the changed-files check is tried. @@ -123,8 +124,8 @@ func TestHandCorruptedFailedManifestFallsBack(t *testing.T) { project.recordBaseline(t) project.editBody(t, 0, "edited") baseline := project.baseline(t) - if err := project.records().recordCompile("Alpha", baseline.InputKey, - recordedCompile{Succeeded: false, Diagnostics: baseline.Diagnostics}, unitOutputNames(project.plan.Units[0])); err != nil { + if err := project.records().RecordCompile("Alpha", baseline.InputKey, + resultstore.CompileResult[Diagnostic]{Succeeded: false, Diagnostics: baseline.Diagnostics}, unitOutputNames(project.plan.Units[0])); err != nil { t.Fatalf("failed to record the failed compile: %v", err) } recorder := newCachedSchedulerRecorder("Alpha", "Beta") @@ -222,11 +223,11 @@ func TestMissingOrChangedGeneratorOutputFallsBack(t *testing.T) { } { t.Run(name, func(t *testing.T) { project := newFastPathProject(t, fastPathMinimumSources) - generatedPath := filepath.Join(project.root, project.records().generatedDirectory("Alpha")) + generatedPath := filepath.Join(project.root, project.records().GeneratedDirectory("Alpha")) recorder := newCachedSchedulerRecorder("Alpha", "Beta") recorder.during = func(unit string) { if unit == "Alpha" { - project.writeFile(t, filepath.Join(project.records().generatedDirectory("Alpha"), "Gen", "G.g.cs"), "class G { }") + project.writeFile(t, filepath.Join(project.records().GeneratedDirectory("Alpha"), "Gen", "G.g.cs"), "class G { }") } } project.run(t, recorder, false) @@ -361,7 +362,7 @@ func TestFastPathCleanResultKeepsBaselineFiles(t *testing.T) { if recorder.runs["Beta"] != 0 || outcome.Skipped != 1 { t.Errorf("expected Beta to be skipped, got %d runs and %d skipped", recorder.runs["Beta"], outcome.Skipped) } - if project.worker.lastCheck.GeneratedDirectory != filepath.Join(project.root, project.records().generatedDirectory("Alpha")) { + if project.worker.lastCheck.GeneratedDirectory != filepath.Join(project.root, project.records().GeneratedDirectory("Alpha")) { t.Errorf("expected the saved generator output to be passed, got %q", project.worker.lastCheck.GeneratedDirectory) } } @@ -434,11 +435,11 @@ func TestZeroChangedFilesDropsBaselineAndFallsBack(t *testing.T) { project.recordBaseline(t) baseline := project.baseline(t) records := project.records() - if err := records.recordCompile("Alpha", "tampered", - recordedCompile{Succeeded: true, Diagnostics: baseline.Diagnostics}, unitOutputNames(project.plan.Units[0])); err != nil { + if err := records.RecordCompile("Alpha", "tampered", + resultstore.CompileResult[Diagnostic]{Succeeded: true, Diagnostics: baseline.Diagnostics}, unitOutputNames(project.plan.Units[0])); err != nil { t.Fatalf("failed to record the tampered compile: %v", err) } - if err := records.recordBaseline("Alpha", baselineInputs{ + if err := records.RecordBaseline("Alpha", resultstore.BaselineInputs{ InputKey: "tampered", NonSourceKey: baseline.NonSourceKey, WorkerKey: baseline.WorkerKey, Files: baseline.Files, }); err != nil { t.Fatalf("failed to record the tampered baseline: %v", err) diff --git a/cli/dispatcher/internal/compilecheck/record_store.go b/cli/dispatcher/internal/compilecheck/record_store.go deleted file mode 100644 index 539446450..000000000 --- a/cli/dispatcher/internal/compilecheck/record_store.go +++ /dev/null @@ -1,206 +0,0 @@ -package compilecheck - -import ( - "encoding/json" - "fmt" - "os" - "path/filepath" -) - -const ( - // resultManifestFormatVersion guards against reading a manifest an older or newer build wrote: - // what belongs in the key is part of this check's code, so a manifest from another version says - // nothing about whether this one would compile the same thing. - resultManifestFormatVersion = 1 - - resultManifestExtension = ".result.json" - resultManifestTempSuffix = ".tmp" -) - -// recordStore reads and writes what earlier runs of the check recorded for each assembly in one -// output directory: the result of its last compile, the baseline the changed-files check compares -// against, and the generator output that baseline was compiled with. -type recordStore struct { - projectRoot string - // outputDir is relative to the project root, which is how the records name the generator output. - outputDir string -} - -// recordedCompile is what one compile of an assembly reported. -type recordedCompile struct { - Succeeded bool - Diagnostics []Diagnostic -} - -// unitResultManifest is what one finished compile left behind for the next run: the inputs it ran -// on, what it reported, and the outputs it wrote. -type unitResultManifest struct { - FormatVersion int - InputKey string - Succeeded bool - Diagnostics []Diagnostic - // Outputs maps the name of each file the compile wrote in the check's output directory to its - // content hash, so a run that finds them changed underneath compiles again. - Outputs map[string]string -} - -// openRecordStore opens the records kept in outputDir, a directory relative to projectRoot. -func openRecordStore(projectRoot string, outputDir string) recordStore { - return recordStore{projectRoot: projectRoot, outputDir: outputDir} -} - -// directoryPath names the output directory the records live in. -func (s recordStore) directoryPath() string { - return filepath.Join(s.projectRoot, s.outputDir) -} - -// manifestPath names where one assembly's manifest lives. -func manifestPath(directoryPath string, assembly string) string { - return filepath.Join(directoryPath, assembly+resultManifestExtension) -} - -// reusableResult reports what a previous compile of the assembly found, when that compile ran on -// the inputs key describes and its outputs are still on disk exactly as it wrote them. -// Why anything unexpected simply means no: the caller compiles the unit, which is what it would -// have done anyway. Nothing here is worth failing a run over. -func (s recordStore) reusableResult(assembly string, key string) (recordedCompile, bool) { - content, err := os.ReadFile(manifestPath(s.directoryPath(), assembly)) - if err != nil { - return recordedCompile{}, false - } - - var manifest unitResultManifest - if err := json.Unmarshal(content, &manifest); err != nil { - return recordedCompile{}, false - } - if manifest.FormatVersion != resultManifestFormatVersion || manifest.InputKey != key || key == "" { - return recordedCompile{}, false - } - if !outputsStillMatch(s.directoryPath(), manifest.Outputs) { - return recordedCompile{}, false - } - - // Why the recorded outcome is replayed rather than assumed successful: a compile that reported - // errors reports them again until one of its inputs moves, and swallowing them here would turn a - // broken project into a clean run. - return recordedCompile{Succeeded: manifest.Succeeded, Diagnostics: manifest.Diagnostics}, true -} - -// outputsStillMatch reports whether every file a compile wrote is still there and still holds what -// it wrote. -func outputsStillMatch(directoryPath string, outputs map[string]string) bool { - for name, recordedHash := range outputs { - currentHash, err := hashFile(filepath.Join(directoryPath, name)) - if err != nil || currentHash != recordedHash { - return false - } - } - - return true -} - -// recordCompile records what a compile of the assembly ran on and what it reported, together with -// the outputs it wrote, named in the output directory. It records nothing when that compile claimed -// success without producing every one of those outputs. -// Why it is written through a temporary file: a run interrupted while writing would otherwise leave -// half a manifest behind, and the next run would read it as a description of outputs it never saw. -func (s recordStore) recordCompile(assembly string, key string, result recordedCompile, outputs []string) error { - hashes, complete, err := hashOutputs(s.directoryPath(), outputs, result.Succeeded) - if err != nil { - return err - } - if !complete { - return nil - } - - content, err := json.Marshal(unitResultManifest{ - FormatVersion: resultManifestFormatVersion, - InputKey: key, - Succeeded: result.Succeeded, - Diagnostics: result.Diagnostics, - Outputs: hashes, - }) - if err != nil { - return fmt.Errorf("failed to record the result of %s: %w", assembly, err) - } - - return writeFileAtomically(s.directoryPath(), manifestPath(s.directoryPath(), assembly), content) -} - -// writeFileAtomically writes content to path through a temporary file in directoryPath. -func writeFileAtomically(directoryPath string, path string, content []byte) error { - // Why the temporary name is unique per writer: two runs of the same project writing the same - // file at once must not rename each other's half-written file into place. - temporary, err := os.CreateTemp(directoryPath, filepath.Base(path)+".*"+resultManifestTempSuffix) - if err != nil { - return fmt.Errorf("failed to write %s: %w", path, err) - } - temporaryPath := temporary.Name() - if _, err := temporary.Write(content); err != nil { - _ = temporary.Close() - _ = os.Remove(temporaryPath) - - return fmt.Errorf("failed to write %s: %w", temporaryPath, err) - } - if err := temporary.Close(); err != nil { - _ = os.Remove(temporaryPath) - - return fmt.Errorf("failed to write %s: %w", temporaryPath, err) - } - if err := os.Rename(temporaryPath, path); err != nil { - _ = os.Remove(temporaryPath) - - return fmt.Errorf("failed to write %s: %w", path, err) - } - - return nil -} - -// hashOutputs hashes the named files a compile wrote in directoryPath. -// Why a missing output is tolerated only for a failed compile: that is the compile which writes -// neither the assembly nor its reference assembly. A successful compile that left one of them out -// is not a compile whose result describes anything - recording it would replay a success backed by -// files no run ever produced, and the outputs check would pass because it has nothing to check. -func hashOutputs(directoryPath string, names []string, succeeded bool) (map[string]string, bool, error) { - outputs := map[string]string{} - for _, name := range names { - hashed, err := hashFile(filepath.Join(directoryPath, name)) - if os.IsNotExist(err) { - if succeeded { - return nil, false, nil - } - - continue - } - if err != nil { - return nil, false, err - } - outputs[name] = hashed - } - - return outputs, true, nil -} - -// forget removes every record of the assembly: its manifest, its baseline, and its generator output. -func (s recordStore) forget(assembly string) error { - names := []string{ - // Why the manifest goes with the outputs: it describes those very files, so leaving it behind - // would let a later run replay diagnostics for outputs this one removed. - assembly + resultManifestExtension, - // The baseline and the generator output describe the same compile as the manifest. - assembly + baselineRecordExtension, - } - for _, name := range names { - path := filepath.Join(s.directoryPath(), name) - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to remove %s: %w", path, err) - } - } - - generatedPath := filepath.Join(s.directoryPath(), assembly+generatedDirectoryExtension) - if err := os.RemoveAll(generatedPath); err != nil { - return fmt.Errorf("failed to remove %s: %w", generatedPath, err) - } - - return nil -} diff --git a/cli/dispatcher/internal/compilecheck/result_cache.go b/cli/dispatcher/internal/compilecheck/result_cache.go index ea3c83057..4e8e8c97f 100644 --- a/cli/dispatcher/internal/compilecheck/result_cache.go +++ b/cli/dispatcher/internal/compilecheck/result_cache.go @@ -8,6 +8,8 @@ import ( "io" "os" "path/filepath" + + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // reuseOrCompile reports what csc would say about one unit, replaying the previous run's answer when @@ -26,7 +28,7 @@ func (c Compiler) reuseOrCompile( // Why --all only skips the lookup and still records: it is the way out of a reuse that got // something wrong, and the ordinary run after it should still be able to reuse what it compiled. if key != "" && !plan.All { - if recorded, found := records.reusableResult(unit.Assembly.AssemblyName, key); found { + if recorded, found := records.Reusable(unit.Assembly.AssemblyName, key); found { return UnitResult{ Assembly: unit.Assembly.AssemblyName, Diagnostics: recorded.Diagnostics, @@ -56,8 +58,8 @@ func (c Compiler) reuseOrCompile( if err != nil || key == "" || result.RawOutput != "" { return result, err } - if writeErr := records.recordCompile(unit.Assembly.AssemblyName, key, - recordedCompile{Succeeded: result.Succeeded, Diagnostics: result.Diagnostics}, unitOutputNames(unit)); writeErr != nil { + if writeErr := records.RecordCompile(unit.Assembly.AssemblyName, key, + resultstore.CompileResult[Diagnostic]{Succeeded: result.Succeeded, Diagnostics: result.Diagnostics}, unitOutputNames(unit)); writeErr != nil { return UnitResult{}, writeErr } @@ -75,8 +77,8 @@ func (c Compiler) reuseOrCompile( } // records opens the store of what earlier runs of the check recorded in the plan's output directory. -func (c Compiler) records(plan BuildPlan) recordStore { - return openRecordStore(c.ProjectRoot, plan.OutputDir) +func (c Compiler) records(plan BuildPlan) resultstore.Store[Diagnostic] { + return resultstore.Open[Diagnostic](c.ProjectRoot, plan.OutputDir) } // unitOutputNames lists the files a compile of the unit writes in the output directory, by name, diff --git a/cli/dispatcher/internal/compilecheck/result_cache_write_test.go b/cli/dispatcher/internal/compilecheck/result_cache_write_test.go index a9c2869cd..a7b348857 100644 --- a/cli/dispatcher/internal/compilecheck/result_cache_write_test.go +++ b/cli/dispatcher/internal/compilecheck/result_cache_write_test.go @@ -29,7 +29,7 @@ func TestUnitOutputNamesSkipsAnAbsentReferenceAssemblyPath(t *testing.T) { func TestRemoveUnitOutputsLeavesNoRecords(t *testing.T) { project := newFastPathProject(t, fastPathMinimumSources) project.recordBaseline(t) - generatedPath := filepath.Join(project.root, project.records().generatedDirectory("Alpha")) + generatedPath := filepath.Join(project.root, project.records().GeneratedDirectory("Alpha")) if _, err := os.Stat(generatedPath); err != nil { t.Fatalf("expected the compile to leave its generator output directory: %v", err) } diff --git a/cli/dispatcher/internal/compilecheck/record_store_baseline.go b/cli/dispatcher/internal/compilecheck/resultstore/baseline.go similarity index 77% rename from cli/dispatcher/internal/compilecheck/record_store_baseline.go rename to cli/dispatcher/internal/compilecheck/resultstore/baseline.go index 7ce838233..4364b3f37 100644 --- a/cli/dispatcher/internal/compilecheck/record_store_baseline.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/baseline.go @@ -1,4 +1,4 @@ -package compilecheck +package resultstore import ( "encoding/json" @@ -14,67 +14,76 @@ const ( generatedDirectoryExtension = ".generated" ) -// baselineRecord is what a successful full compile leaves for the next changed-files check. -type baselineRecord struct { - FormatVersion int - // InputKey is the key of the full compile this baseline describes; its manifest carries the same. - InputKey string - // NonSourceKey covers every input but the source contents, so any other change compiles in full. - NonSourceKey nonSourceIdentity - // WorkerKey names the worker build that computed the fingerprints. - WorkerKey string - GeneratedDirectory string // relative to the project root - // GeneratedFiles maps each generated file, relative to GeneratedDirectory, to its content hash, - // so a check never runs without the generator output its baseline compile had. - GeneratedFiles map[string]string - Files map[string]baselineFile +// KeyDigest is a key over several kinds of input together with a digest of each kind, so a change +// of the key can be told by kind. +type KeyDigest struct { + Key string + // Parts maps each kind of input to its digest. + Parts map[string]string } -// baselineFile describes one source as the full compile saw it. -type baselineFile struct { +// BaselineFile describes one source as the full compile saw it. +type BaselineFile struct { ContentHash string Fingerprint string } -// baselineInputs is what the caller knows about the full compile a baseline describes: its input -// key, its non-source key, the worker build that fingerprinted its sources, and each source. -type baselineInputs struct { +// BaselineInputs is what the caller knows about the full compile a baseline describes: its input +// key, its key over every input but the source contents, the worker build that fingerprinted its +// sources, and each source. +type BaselineInputs struct { InputKey string - NonSourceKey nonSourceIdentity + NonSourceKey KeyDigest WorkerKey string - Files map[string]baselineFile + Files map[string]BaselineFile } -// storedBaseline is a recorded baseline together with what its full compile reported. -type storedBaseline struct { +// Baseline is a recorded baseline together with what its full compile reported. +type Baseline[D any] struct { InputKey string - NonSourceKey nonSourceIdentity + NonSourceKey KeyDigest WorkerKey string - Files map[string]baselineFile + Files map[string]BaselineFile // GeneratedDirectory is relative to the project root and slash-separated, as recorded. GeneratedDirectory string // Diagnostics is what the full compile this baseline describes reported. - Diagnostics []Diagnostic + Diagnostics []D generatedFiles map[string]string } -// generatedDirectory names, relative to the project root, where a full compile saves the generator +// baselineRecord is what a successful full compile leaves for the next changed-files check. +type baselineRecord struct { + FormatVersion int + // InputKey is the key of the full compile this baseline describes; its manifest carries the same. + InputKey string + // NonSourceKey covers every input but the source contents, so any other change compiles in full. + NonSourceKey KeyDigest + // WorkerKey names the worker build that computed the fingerprints. + WorkerKey string + GeneratedDirectory string // relative to the project root + // GeneratedFiles maps each generated file, relative to GeneratedDirectory, to its content hash, + // so a check never runs without the generator output its baseline compile had. + GeneratedFiles map[string]string + Files map[string]BaselineFile +} + +// GeneratedDirectory names, relative to the project root, where a full compile saves the generator // output of an assembly. -func (s recordStore) generatedDirectory(assembly string) string { +func (s Store[D]) GeneratedDirectory(assembly string) string { return filepath.Join(s.outputDir, assembly+generatedDirectoryExtension) } -// loadBaseline reads the baseline of an assembly together with what the full compile it describes +// LoadBaseline reads the baseline of an assembly together with what the full compile it describes // reported, or says why there is none to use. -func (s recordStore) loadBaseline(assembly string) (storedBaseline, string) { +func (s Store[D]) LoadBaseline(assembly string) (Baseline[D], string) { content, err := os.ReadFile(baselineRecordPath(s.directoryPath(), assembly)) if err != nil { - return storedBaseline{}, s.missingBaselineReason(assembly) + return Baseline[D]{}, s.missingBaselineReason(assembly) } var record baselineRecord if err := json.Unmarshal(content, &record); err != nil || record.FormatVersion != baselineRecordFormatVersion { - return storedBaseline{}, "no baseline: it was recorded by another version of compile-check" + return Baseline[D]{}, "no baseline: it was recorded by another version of compile-check" } // In the normal flow the record and the manifest are written together under one key, and a full @@ -82,17 +91,17 @@ func (s recordStore) loadBaseline(assembly string) (storedBaseline, string) { const goneFromDisk = "baseline compile is no longer on disk" manifestContent, err := os.ReadFile(manifestPath(s.directoryPath(), assembly)) if err != nil { - return storedBaseline{}, goneFromDisk + return Baseline[D]{}, goneFromDisk } - var manifest unitResultManifest + var manifest unitResultManifest[D] if err := json.Unmarshal(manifestContent, &manifest); err != nil || manifest.InputKey != record.InputKey || !manifest.Succeeded || !outputsStillMatch(s.directoryPath(), manifest.Outputs) { - return storedBaseline{}, goneFromDisk + return Baseline[D]{}, goneFromDisk } - return storedBaseline{ + return Baseline[D]{ InputKey: record.InputKey, NonSourceKey: record.NonSourceKey, WorkerKey: record.WorkerKey, @@ -109,14 +118,14 @@ func (s recordStore) loadBaseline(assembly string) (storedBaseline, string) { // left, and a reason that read as "never compiled" sent users looking for a compile that had run. // Why a missing manifest says nothing about what happened: a run that skips or blocks the unit // removes the manifest and the baseline alike, so nothing on disk tells that apart from no compile. -func (s recordStore) missingBaselineReason(assembly string) string { +func (s Store[D]) missingBaselineReason(assembly string) string { const notOnRecord = "no baseline: no full compile of this assembly is on record" content, err := os.ReadFile(manifestPath(s.directoryPath(), assembly)) if err != nil { return notOnRecord } - var manifest unitResultManifest + var manifest unitResultManifest[D] if err := json.Unmarshal(content, &manifest); err != nil { return notOnRecord } @@ -127,16 +136,16 @@ func (s recordStore) missingBaselineReason(assembly string) string { return "no baseline: the last full compile did not record one" } -// generatorOutputUnchanged reports whether the generator output on disk is exactly what the -// baseline recorded. -func (s recordStore) generatorOutputUnchanged(baseline storedBaseline) bool { +// GeneratorOutputUnchanged reports whether the generator output on disk is exactly what the +// baseline recorded: no file missing, added, or changed. +func (s Store[D]) GeneratorOutputUnchanged(baseline Baseline[D]) bool { return generatedFilesMatch(projectPath(s.projectRoot, baseline.GeneratedDirectory), baseline.generatedFiles) } -// recordBaseline records what the next changed-files check of an assembly compares against, with +// RecordBaseline records what the next changed-files check of an assembly compares against, with // the generator output its full compile saved. -func (s recordStore) recordBaseline(assembly string, inputs baselineInputs) error { - generatedDirectory := s.generatedDirectory(assembly) +func (s Store[D]) RecordBaseline(assembly string, inputs BaselineInputs) error { + generatedDirectory := s.GeneratedDirectory(assembly) generatedFiles, err := hashGeneratedFiles(filepath.Join(s.projectRoot, generatedDirectory)) if err != nil { return err diff --git a/cli/dispatcher/internal/compilecheck/record_store_baseline_test.go b/cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go similarity index 84% rename from cli/dispatcher/internal/compilecheck/record_store_baseline_test.go rename to cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go index 53309e8c7..8557a2d41 100644 --- a/cli/dispatcher/internal/compilecheck/record_store_baseline_test.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/baseline_test.go @@ -1,4 +1,4 @@ -package compilecheck +package resultstore import ( "os" @@ -9,7 +9,7 @@ import ( // Verifies that only generated C# files are recorded, keyed relative to the generator output directory. func TestHashGeneratedFilesRecordsOnlyCSharpFiles(t *testing.T) { directory := t.TempDir() - if err := os.MkdirAll(filepath.Join(directory, "Gen"), outputDirPermissions); err != nil { + if err := os.MkdirAll(filepath.Join(directory, "Gen"), testDirectoryPermissions); err != nil { t.Fatalf("failed to create the generator folder: %v", err) } writeOutputFile(t, filepath.Join(directory, "Gen"), "A.g.cs", "class A { }") @@ -30,7 +30,7 @@ func TestHashGeneratedFilesRecordsOnlyCSharpFiles(t *testing.T) { func TestLoadBaselineReportsABaselineOfAnotherFormat(t *testing.T) { directoryPath := newRecordedOutputs(t) store := directoryStore(directoryPath) - if err := store.recordCompile("Alpha", "input", recordedCompile{Succeeded: true}, alphaOutputs()); err != nil { + if err := store.RecordCompile("Alpha", "input", CompileResult[testDiagnostic]{Succeeded: true}, alphaOutputs()); err != nil { t.Fatalf("failed to write the manifest: %v", err) } if err := writeBaselineRecord(directoryPath, "Alpha", baselineRecord{ @@ -40,7 +40,7 @@ func TestLoadBaselineReportsABaselineOfAnotherFormat(t *testing.T) { t.Fatalf("failed to write the baseline: %v", err) } - _, reason := store.loadBaseline("Alpha") + _, reason := store.LoadBaseline("Alpha") if reason != "no baseline: it was recorded by another version of compile-check" { t.Errorf("reason = %q", reason) diff --git a/cli/dispatcher/internal/compilecheck/resultstore/compile_result.go b/cli/dispatcher/internal/compilecheck/resultstore/compile_result.go new file mode 100644 index 000000000..6387507ef --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/compile_result.go @@ -0,0 +1,133 @@ +package resultstore + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +const ( + // resultManifestFormatVersion guards against reading a manifest an older or newer build wrote: + // what belongs in the key is part of this check's code, so a manifest from another version says + // nothing about whether this one would compile the same thing. + resultManifestFormatVersion = 1 + + resultManifestExtension = ".result.json" +) + +// CompileResult is what one compile of an assembly reported. +type CompileResult[D any] struct { + Succeeded bool + Diagnostics []D +} + +// unitResultManifest is what one finished compile left behind for the next run: the inputs it ran +// on, what it reported, and the outputs it wrote. +type unitResultManifest[D any] struct { + FormatVersion int + InputKey string + Succeeded bool + Diagnostics []D + // Outputs maps the name of each file the compile wrote in the check's output directory to its + // content hash, so a run that finds them changed underneath compiles again. + Outputs map[string]string +} + +// manifestPath names where one assembly's manifest lives. +func manifestPath(directoryPath string, assembly string) string { + return filepath.Join(directoryPath, assembly+resultManifestExtension) +} + +// Reusable reports what a previous compile of the assembly found, when that compile ran on the +// inputs key describes and its outputs are still on disk exactly as it wrote them. +// Why anything unexpected simply means no: the caller compiles the unit, which is what it would +// have done anyway. Nothing here is worth failing a run over. +func (s Store[D]) Reusable(assembly string, key string) (CompileResult[D], bool) { + content, err := os.ReadFile(manifestPath(s.directoryPath(), assembly)) + if err != nil { + return CompileResult[D]{}, false + } + + var manifest unitResultManifest[D] + if err := json.Unmarshal(content, &manifest); err != nil { + return CompileResult[D]{}, false + } + if manifest.FormatVersion != resultManifestFormatVersion || manifest.InputKey != key || key == "" { + return CompileResult[D]{}, false + } + if !outputsStillMatch(s.directoryPath(), manifest.Outputs) { + return CompileResult[D]{}, false + } + + // Why the recorded outcome is replayed rather than assumed successful: a compile that reported + // errors reports them again until one of its inputs moves, and swallowing them here would turn a + // broken project into a clean run. + return CompileResult[D]{Succeeded: manifest.Succeeded, Diagnostics: manifest.Diagnostics}, true +} + +// outputsStillMatch reports whether every file a compile wrote is still there and still holds what +// it wrote. +func outputsStillMatch(directoryPath string, outputs map[string]string) bool { + for name, recordedHash := range outputs { + currentHash, err := hashFile(filepath.Join(directoryPath, name)) + if err != nil || currentHash != recordedHash { + return false + } + } + + return true +} + +// RecordCompile records what a compile of the assembly ran on (key) and what it reported, together +// with the outputs it wrote, named in the output directory. It records nothing when that compile +// claimed success without producing every one of those outputs. +// Why it is written through a temporary file: a run interrupted while writing would otherwise leave +// half a manifest behind, and the next run would read it as a description of outputs it never saw. +func (s Store[D]) RecordCompile(assembly string, key string, result CompileResult[D], outputs []string) error { + hashes, complete, err := hashOutputs(s.directoryPath(), outputs, result.Succeeded) + if err != nil { + return err + } + if !complete { + return nil + } + + content, err := json.Marshal(unitResultManifest[D]{ + FormatVersion: resultManifestFormatVersion, + InputKey: key, + Succeeded: result.Succeeded, + Diagnostics: result.Diagnostics, + Outputs: hashes, + }) + if err != nil { + return fmt.Errorf("failed to record the result of %s: %w", assembly, err) + } + + return writeFileAtomically(s.directoryPath(), manifestPath(s.directoryPath(), assembly), content) +} + +// hashOutputs hashes the named files a compile wrote in directoryPath. +// Why a missing output is tolerated only for a failed compile: that is the compile which writes +// neither the assembly nor its reference assembly. A successful compile that left one of them out +// is not a compile whose result describes anything - recording it would replay a success backed by +// files no run ever produced, and the outputs check would pass because it has nothing to check. +func hashOutputs(directoryPath string, names []string, succeeded bool) (map[string]string, bool, error) { + outputs := map[string]string{} + for _, name := range names { + hashed, err := hashFile(filepath.Join(directoryPath, name)) + if os.IsNotExist(err) { + if succeeded { + return nil, false, nil + } + + continue + } + if err != nil { + return nil, false, err + } + outputs[name] = hashed + } + + return outputs, true, nil +} diff --git a/cli/dispatcher/internal/compilecheck/record_store_test.go b/cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go similarity index 87% rename from cli/dispatcher/internal/compilecheck/record_store_test.go rename to cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go index 5601ad07b..659f959b4 100644 --- a/cli/dispatcher/internal/compilecheck/record_store_test.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/compile_result_test.go @@ -1,4 +1,4 @@ -package compilecheck +package resultstore import ( "encoding/json" @@ -24,16 +24,16 @@ func newRecordedOutputs(t *testing.T) string { } // directoryStore opens the records kept directly in directoryPath. -func directoryStore(directoryPath string) recordStore { - return openRecordStore(directoryPath, "") +func directoryStore(directoryPath string) Store[testDiagnostic] { + return Open[testDiagnostic](directoryPath, "") } // storedResult is the result the tests record and expect to read back unchanged. -func storedResult() recordedCompile { - return recordedCompile{ +func storedResult() CompileResult[testDiagnostic] { + return CompileResult[testDiagnostic]{ Succeeded: false, - Diagnostics: []Diagnostic{{ - Severity: severityError, + Diagnostics: []testDiagnostic{{ + Severity: "error", Code: "CS0103", Message: "The name 'Missing' does not exist in the current context", File: filepath.Join("Assets", "Alpha", "Alpha.cs"), @@ -46,7 +46,7 @@ func storedResult() recordedCompile { // recordResult stores one compile of Alpha's outcome under a key. func recordResult(t *testing.T, directoryPath string, key string) { t.Helper() - if err := directoryStore(directoryPath).recordCompile("Alpha", key, storedResult(), alphaOutputs()); err != nil { + if err := directoryStore(directoryPath).RecordCompile("Alpha", key, storedResult(), alphaOutputs()); err != nil { t.Fatalf("failed to write the manifest: %v", err) } } @@ -64,7 +64,7 @@ func TestReusableResultReplaysTheRecordedDiagnostics(t *testing.T) { directoryPath := newRecordedOutputs(t) recordResult(t, directoryPath, "key") - result, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key") + result, reusable := directoryStore(directoryPath).Reusable("Alpha", "key") if !reusable { t.Fatal("expected the recorded result to be reusable") @@ -82,7 +82,7 @@ func TestReusableResultRejectsAnotherKey(t *testing.T) { directoryPath := newRecordedOutputs(t) recordResult(t, directoryPath, "key") - if _, reusable := directoryStore(directoryPath).reusableResult("Alpha", "other key"); reusable { + if _, reusable := directoryStore(directoryPath).Reusable("Alpha", "other key"); reusable { t.Error("a result recorded for other inputs should not be reused") } } @@ -95,7 +95,7 @@ func TestReusableResultRejectsAChangedOutput(t *testing.T) { writeOutputFile(t, directoryPath, "Alpha"+referenceAssemblyExtension, "another surface") - if _, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key"); reusable { + if _, reusable := directoryStore(directoryPath).Reusable("Alpha", "key"); reusable { t.Error("a changed output should stop the reuse") } } @@ -109,7 +109,7 @@ func TestReusableResultRejectsAMissingOutput(t *testing.T) { t.Fatalf("failed to remove the output: %v", err) } - if _, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key"); reusable { + if _, reusable := directoryStore(directoryPath).Reusable("Alpha", "key"); reusable { t.Error("a missing output should stop the reuse") } } @@ -120,7 +120,7 @@ func TestReusableResultRejectsAnUnreadableManifest(t *testing.T) { directoryPath := newRecordedOutputs(t) writeOutputFile(t, directoryPath, "Alpha.result.json", "{not json") - if _, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key"); reusable { + if _, reusable := directoryStore(directoryPath).Reusable("Alpha", "key"); reusable { t.Error("an unreadable manifest should not be reused") } } @@ -134,7 +134,7 @@ func TestReusableResultRejectsAnotherFormatVersion(t *testing.T) { if err != nil { t.Fatalf("failed to read the manifest: %v", err) } - var manifest unitResultManifest + var manifest unitResultManifest[testDiagnostic] if err := json.Unmarshal(content, &manifest); err != nil { t.Fatalf("failed to parse the manifest: %v", err) } @@ -147,7 +147,7 @@ func TestReusableResultRejectsAnotherFormatVersion(t *testing.T) { t.Fatalf("failed to write the manifest back: %v", err) } - if _, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key"); reusable { + if _, reusable := directoryStore(directoryPath).Reusable("Alpha", "key"); reusable { t.Error("a manifest of another format version should not be reused") } } @@ -163,7 +163,7 @@ func TestRecordCompileRecordsACompileThatWroteNothing(t *testing.T) { } recordResult(t, directoryPath, "key") - result, reusable := directoryStore(directoryPath).reusableResult("Alpha", "key") + result, reusable := directoryStore(directoryPath).Reusable("Alpha", "key") if !reusable { t.Fatal("expected a compile that wrote nothing to be reusable") @@ -179,7 +179,7 @@ func TestRecordCompileReportsAFailedRename(t *testing.T) { directoryPath := newRecordedOutputs(t) blockManifestPath(t, manifestPath(directoryPath, "Alpha")) - err := directoryStore(directoryPath).recordCompile("Alpha", "key", storedResult(), alphaOutputs()) + err := directoryStore(directoryPath).RecordCompile("Alpha", "key", storedResult(), alphaOutputs()) if err == nil || !strings.HasPrefix(err.Error(), "failed to write "+manifestPath(directoryPath, "Alpha")+":") { t.Fatalf("err = %v", err) @@ -195,7 +195,7 @@ func TestRecordCompileReportsAFailedRename(t *testing.T) { func TestRecordCompileReportsAMissingOutputDirectory(t *testing.T) { directoryPath := filepath.Join(t.TempDir(), "missing") - err := directoryStore(directoryPath).recordCompile("Alpha", "key", storedResult(), alphaOutputs()) + err := directoryStore(directoryPath).RecordCompile("Alpha", "key", storedResult(), alphaOutputs()) if err == nil || !strings.HasPrefix(err.Error(), "failed to write "+manifestPath(directoryPath, "Alpha")+":") { t.Fatalf("err = %v", err) @@ -236,12 +236,12 @@ func TestForgetRemovesTheManifestTheBaselineAndTheGeneratorOutput(t *testing.T) recordResult(t, directoryPath, "key") writeOutputFile(t, directoryPath, "Alpha"+baselineRecordExtension, "{}") generatedPath := filepath.Join(directoryPath, "Alpha"+generatedDirectoryExtension, "Nested") - if err := os.MkdirAll(generatedPath, outputDirPermissions); err != nil { + if err := os.MkdirAll(generatedPath, testDirectoryPermissions); err != nil { t.Fatalf("failed to create the generated directory: %v", err) } writeOutputFile(t, generatedPath, "G.cs", "class G {}") - if err := directoryStore(directoryPath).forget("Alpha"); err != nil { + if err := directoryStore(directoryPath).Forget("Alpha"); err != nil { t.Fatalf("expected the records to be removed, got error: %v", err) } diff --git a/cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go b/cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go new file mode 100644 index 000000000..8c3c842c8 --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/fixture_test.go @@ -0,0 +1,52 @@ +package resultstore + +import ( + "os" + "path/filepath" + "testing" +) + +const ( + assemblyExtension = ".dll" + referenceAssemblyExtension = ".ref.dll" + testDirectoryPermissions = 0o755 +) + +// testDiagnostic stands in for the caller's diagnostic type. Its fields are in the same order as the +// caller's, so a recorded compile result keeps the bytes the caller's type gives it. +type testDiagnostic struct { + Severity string + Code string + Message string + File string + Line int + Column int +} + +// writeOutputFile writes one file into the check's own output directory. +func writeOutputFile(t *testing.T, outputDirectoryPath string, name string, content string) { + t.Helper() + if err := os.WriteFile( + filepath.Join(outputDirectoryPath, name), []byte(content), 0o600); err != nil { + t.Fatalf("failed to write %s: %v", name, err) + } +} + +// writeFileAt creates a file and every parent directory it needs. +func writeFileAt(t *testing.T, path string, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), testDirectoryPermissions); err != nil { + t.Fatalf("failed to create directory for %s: %v", path, err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write %s: %v", path, err) + } +} + +// makeDirAt creates a directory and every parent directory it needs. +func makeDirAt(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(path, testDirectoryPermissions); err != nil { + t.Fatalf("failed to create directory %s: %v", path, err) + } +} diff --git a/cli/dispatcher/internal/compilecheck/record_format_test.go b/cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go similarity index 90% rename from cli/dispatcher/internal/compilecheck/record_format_test.go rename to cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go index cb09bda6d..5bd69cb30 100644 --- a/cli/dispatcher/internal/compilecheck/record_format_test.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/record_format_test.go @@ -1,4 +1,4 @@ -package compilecheck +package resultstore import ( "os" @@ -16,7 +16,7 @@ func TestLoadBaselineReportsAnotherFormatBeforeAMissingManifest(t *testing.T) { t.Fatalf("failed to write the baseline: %v", err) } - _, reason := directoryStore(directoryPath).loadBaseline("Alpha") + _, reason := directoryStore(directoryPath).LoadBaseline("Alpha") if reason != "no baseline: it was recorded by another version of compile-check" { t.Errorf("reason = %q", reason) @@ -26,10 +26,10 @@ func TestLoadBaselineReportsAnotherFormatBeforeAMissingManifest(t *testing.T) { // Verifies the bytes a compile result is recorded as, so moving the record keeps its format. func TestWriteUnitResultManifestWritesTheRecordedFormat(t *testing.T) { directoryPath := newRecordedOutputs(t) - result := recordedCompile{ + result := CompileResult[testDiagnostic]{ Succeeded: false, - Diagnostics: []Diagnostic{{ - Severity: severityError, + Diagnostics: []testDiagnostic{{ + Severity: "error", Code: "CS0103", Message: "The name 'Missing' does not exist in the current context", File: "Assets/Alpha/Alpha.cs", @@ -38,7 +38,7 @@ func TestWriteUnitResultManifestWritesTheRecordedFormat(t *testing.T) { }}, } - if err := directoryStore(directoryPath).recordCompile("Alpha", "key", result, alphaOutputs()); err != nil { + if err := directoryStore(directoryPath).RecordCompile("Alpha", "key", result, alphaOutputs()); err != nil { t.Fatalf("failed to write the manifest: %v", err) } @@ -55,14 +55,14 @@ func TestWriteBaselineRecordWritesTheRecordedFormat(t *testing.T) { record := baselineRecord{ FormatVersion: baselineRecordFormatVersion, InputKey: "input", - NonSourceKey: nonSourceIdentity{ + NonSourceKey: KeyDigest{ Key: "non-source", Parts: map[string]string{"compiler": "c", "source list": "s"}, }, WorkerKey: "worker", GeneratedDirectory: "Library/out/Alpha.generated", GeneratedFiles: map[string]string{"Gen/G.g.cs": "g"}, - Files: map[string]baselineFile{"Assets/A.cs": {ContentHash: "a", Fingerprint: "f"}}, + Files: map[string]BaselineFile{"Assets/A.cs": {ContentHash: "a", Fingerprint: "f"}}, } if err := writeBaselineRecord(directoryPath, "Alpha", record); err != nil { diff --git a/cli/dispatcher/internal/compilecheck/resultstore/store.go b/cli/dispatcher/internal/compilecheck/resultstore/store.go new file mode 100644 index 000000000..929d4dc22 --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/store.go @@ -0,0 +1,134 @@ +// Package resultstore keeps what earlier runs of compile-check recorded for each assembly in one +// output directory, and is the only code that reads or writes those records. +// +// A store keeps three records per assembly: +// - the result of its last compile: the inputs it ran on (as a key), what it reported, and a hash +// of every output it wrote; +// - the baseline of the changed-files check: what the last successful full compile saw of each +// source, its non-source inputs and the worker build that fingerprinted it; +// - the generator output that full compile saved. +// +// Contract: +// - Records are written through a temporary file and renamed into place, so a run stopped while +// writing never leaves half a record behind. +// - A compile result is reusable only under the key it was recorded with, and only while every +// output it hashed still holds those bytes. +// - A baseline is handed out only when the compile result recorded with it is still on disk: the +// same key, a successful compile, and outputs that still match. Otherwise LoadBaseline says why +// there is none, in one of these reasons: +// "no baseline: no full compile of this assembly is on record", +// "no baseline: the last full compile had errors", +// "no baseline: the last full compile did not record one", +// "no baseline: it was recorded by another version of compile-check", +// "baseline compile is no longer on disk". +// - Forget removes every kind of record of an assembly. It does not touch the assembly's outputs, +// and the store never creates the generator output directory; it only names where it is. +package resultstore + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "path/filepath" +) + +const resultManifestTempSuffix = ".tmp" + +// Store reads and writes the records kept in one output directory. D is the type of a diagnostic a +// compile reported; the store keeps diagnostics as they were given and never reads them. +type Store[D any] struct { + projectRoot string + // outputDir is relative to the project root, which is how the records name the generator output. + outputDir string +} + +// Open opens the records kept in outputDir, a directory relative to projectRoot. +func Open[D any](projectRoot string, outputDir string) Store[D] { + return Store[D]{projectRoot: projectRoot, outputDir: outputDir} +} + +// directoryPath names the output directory the records live in. +func (s Store[D]) directoryPath() string { + return filepath.Join(s.projectRoot, s.outputDir) +} + +// Forget removes every record of the assembly: its compile result, its baseline, and its saved +// generator output. +func (s Store[D]) Forget(assembly string) error { + names := []string{ + // Why the compile result goes with the outputs: it describes those very files, so leaving it + // behind would let a later run replay diagnostics for outputs this one removed. + assembly + resultManifestExtension, + // The baseline and the generator output describe the same compile as the compile result. + assembly + baselineRecordExtension, + } + for _, name := range names { + path := filepath.Join(s.directoryPath(), name) + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove %s: %w", path, err) + } + } + + generatedPath := filepath.Join(s.directoryPath(), assembly+generatedDirectoryExtension) + if err := os.RemoveAll(generatedPath); err != nil { + return fmt.Errorf("failed to remove %s: %w", generatedPath, err) + } + + return nil +} + +// writeFileAtomically writes content to path through a temporary file in directoryPath. +func writeFileAtomically(directoryPath string, path string, content []byte) error { + // Why the temporary name is unique per writer: two runs of the same project writing the same + // file at once must not rename each other's half-written file into place. + temporary, err := os.CreateTemp(directoryPath, filepath.Base(path)+".*"+resultManifestTempSuffix) + if err != nil { + return fmt.Errorf("failed to write %s: %w", path, err) + } + temporaryPath := temporary.Name() + if _, err := temporary.Write(content); err != nil { + _ = temporary.Close() + _ = os.Remove(temporaryPath) + + return fmt.Errorf("failed to write %s: %w", temporaryPath, err) + } + if err := temporary.Close(); err != nil { + _ = os.Remove(temporaryPath) + + return fmt.Errorf("failed to write %s: %w", temporaryPath, err) + } + if err := os.Rename(temporaryPath, path); err != nil { + _ = os.Remove(temporaryPath) + + return fmt.Errorf("failed to write %s: %w", path, err) + } + + return nil +} + +// hashFile reports what a file holds, as a hash. +func hashFile(path string) (string, error) { + file, err := os.Open(path) + if err != nil { + return "", err + } + defer func() { _ = file.Close() }() + + digest := sha256.New() + if _, err := io.Copy(digest, file); err != nil { + return "", fmt.Errorf("failed to read %s: %w", path, err) + } + + return hex.EncodeToString(digest.Sum(nil)), nil +} + +// projectPath resolves a path relative to the project root, leaving an absolute one as it is. +func projectPath(projectRoot string, path string) string { + if filepath.IsAbs(path) { + return path + } + + return filepath.Join(projectRoot, path) +} diff --git a/cli/dispatcher/internal/compilecheck/scheduler_cache_test.go b/cli/dispatcher/internal/compilecheck/scheduler_cache_test.go index 8eb8c39f3..27661be28 100644 --- a/cli/dispatcher/internal/compilecheck/scheduler_cache_test.go +++ b/cli/dispatcher/internal/compilecheck/scheduler_cache_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" "time" + + "github.com/hatayama/unity-cli-loop/dispatcher/internal/compilecheck/resultstore" ) // cachedSchedulerProject is a project laid out on disk in enough detail for the input key to read @@ -118,8 +120,8 @@ func writeOutputFile(t *testing.T, outputDirectoryPath string, name string, cont } // records opens the store the runs of this test project write to. -func (project *cachedSchedulerProject) records() recordStore { - return openRecordStore(project.root, project.plan.OutputDir) +func (project *cachedSchedulerProject) records() resultstore.Store[Diagnostic] { + return resultstore.Open[Diagnostic](project.root, project.plan.OutputDir) } // hasBaselineOf reports whether a baseline of the current format is on disk for the assembly. @@ -127,7 +129,7 @@ func (project *cachedSchedulerProject) records() recordStore { // was written by another version; a baseline file that disagrees with its compile result is // reported as "baseline compile is no longer on disk" instead. func (project *cachedSchedulerProject) hasBaselineOf(name string) bool { - _, reason := project.records().loadBaseline(name) + _, reason := project.records().LoadBaseline(name) return !strings.HasPrefix(reason, "no baseline:") } @@ -137,7 +139,7 @@ func (project *cachedSchedulerProject) hasBaselineOf(name string) bool { // needs, so they never get a baseline, and the store explains the missing baseline from the // compile result alone - this reason is the one it gives when there is no compile result. func (project *cachedSchedulerProject) hasRecordedCompile(name string) bool { - _, reason := project.records().loadBaseline(name) + _, reason := project.records().LoadBaseline(name) return reason != "no baseline: no full compile of this assembly is on record" } From 61a2450d998a9d15fafa386da02e3382c7430ee4 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 15 Oct 2026 12:25:31 +0900 Subject: [PATCH 2/3] test: pin the record store's baseline contract at its boundary Coverage is measured per package, so the store's baseline paths that the compilecheck tests used to exercise counted for nothing once the store moved. These tests cover them through the public methods: a recorded baseline comes back as recorded, a changed, removed or added generated file stops it matching, a compile result that is gone stops it being handed out, the reasons a baseline is missing, and a failed baseline write. --- .../resultstore/baseline_contract_test.go | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go diff --git a/cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go b/cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go new file mode 100644 index 000000000..ea841f0cd --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/baseline_contract_test.go @@ -0,0 +1,213 @@ +package resultstore + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// projectStore is a store over an output directory inside a project root, laid out the way the +// check lays it out, with Alpha's outputs already written. +type projectStore struct { + root string + store Store[testDiagnostic] +} + +// newProjectStore lays out a project root with an output directory holding Alpha's outputs. +func newProjectStore(t *testing.T) projectStore { + t.Helper() + root := t.TempDir() + outputDir := filepath.Join("Library", "out") + makeDirAt(t, filepath.Join(root, outputDir)) + project := projectStore{root: root, store: Open[testDiagnostic](root, outputDir)} + writeOutputFile(t, project.outputPath(), "Alpha"+assemblyExtension, "assembly") + writeOutputFile(t, project.outputPath(), "Alpha"+referenceAssemblyExtension, "surface") + + return project +} + +// outputPath names the output directory on disk. +func (project projectStore) outputPath() string { + return filepath.Join(project.root, "Library", "out") +} + +// generatedPath names Alpha's generator output directory on disk. +func (project projectStore) generatedPath() string { + return filepath.Join(project.root, project.store.GeneratedDirectory("Alpha")) +} + +// recordCompile records a compile of Alpha under key, failing the test when it cannot. +func (project projectStore) recordCompile(t *testing.T, key string, succeeded bool) { + t.Helper() + result := CompileResult[testDiagnostic]{ + Succeeded: succeeded, + Diagnostics: []testDiagnostic{{Severity: "warning", Code: "CS0168", File: "Assets/A.cs", Line: 1, Column: 1}}, + } + if err := project.store.RecordCompile("Alpha", key, result, alphaOutputs()); err != nil { + t.Fatalf("failed to record the compile: %v", err) + } +} + +// recordBaseline records a baseline of Alpha under key, with one generated file on disk. +func (project projectStore) recordBaseline(t *testing.T, key string) BaselineInputs { + t.Helper() + writeFileAt(t, filepath.Join(project.generatedPath(), "Gen", "G.g.cs"), "class G { }") + inputs := BaselineInputs{ + InputKey: key, + NonSourceKey: KeyDigest{Key: "non-source", Parts: map[string]string{"compiler": "c"}}, + WorkerKey: "worker", + Files: map[string]BaselineFile{"Assets/A.cs": {ContentHash: "a", Fingerprint: "f"}}, + } + if err := project.store.RecordBaseline("Alpha", inputs); err != nil { + t.Fatalf("failed to record the baseline: %v", err) + } + + return inputs +} + +// Verifies a recorded baseline comes back with what it was recorded with, the diagnostics of the +// compile it describes, and its generator output named relative to the project root. +func TestLoadBaselineHandsBackWhatWasRecorded(t *testing.T) { + project := newProjectStore(t) + project.recordCompile(t, "input", true) + inputs := project.recordBaseline(t, "input") + + baseline, reason := project.store.LoadBaseline("Alpha") + + if reason != "" { + t.Fatalf("reason = %q, want a usable baseline", reason) + } + if baseline.InputKey != inputs.InputKey || baseline.WorkerKey != inputs.WorkerKey || + !reflect.DeepEqual(baseline.NonSourceKey, inputs.NonSourceKey) || !reflect.DeepEqual(baseline.Files, inputs.Files) { + t.Errorf("baseline = %+v, want what was recorded: %+v", baseline, inputs) + } + if baseline.GeneratedDirectory != "Library/out/Alpha.generated" { + t.Errorf("generated directory = %q", baseline.GeneratedDirectory) + } + if len(baseline.Diagnostics) != 1 || baseline.Diagnostics[0].Code != "CS0168" { + t.Errorf("diagnostics = %+v, want the recorded compile's", baseline.Diagnostics) + } + if !project.store.GeneratorOutputUnchanged(baseline) { + t.Error("expected the generator output to match right after recording") + } +} + +// Verifies the generator output stops matching its baseline when a generated file is changed, +// removed, or added. +func TestGeneratorOutputUnchangedNoticesEveryChange(t *testing.T) { + for name, damage := range map[string]func(t *testing.T, generatedPath string){ + "changed": func(t *testing.T, generatedPath string) { + writeFileAt(t, filepath.Join(generatedPath, "Gen", "G.g.cs"), "class G { int x; }") + }, + "removed": func(t *testing.T, generatedPath string) { + if err := os.RemoveAll(generatedPath); err != nil { + t.Fatalf("failed to remove the generator output: %v", err) + } + }, + "added": func(t *testing.T, generatedPath string) { + writeFileAt(t, filepath.Join(generatedPath, "Gen", "H.g.cs"), "class H { }") + }, + } { + t.Run(name, func(t *testing.T) { + project := newProjectStore(t) + project.recordCompile(t, "input", true) + project.recordBaseline(t, "input") + baseline, reason := project.store.LoadBaseline("Alpha") + if reason != "" { + t.Fatalf("reason = %q, want a usable baseline", reason) + } + + damage(t, project.generatedPath()) + + if project.store.GeneratorOutputUnchanged(baseline) { + t.Error("expected the damaged generator output not to match") + } + }) + } +} + +// Verifies a baseline is not handed out once the compile it describes is no longer what is on disk. +func TestLoadBaselineRefusesABaselineWhoseCompileIsGone(t *testing.T) { + for name, damage := range map[string]func(t *testing.T, project projectStore){ + "compile result removed": func(t *testing.T, project projectStore) { + if err := os.Remove(manifestPath(project.outputPath(), "Alpha")); err != nil { + t.Fatalf("failed to remove the compile result: %v", err) + } + }, + "compile result unreadable": func(t *testing.T, project projectStore) { + writeOutputFile(t, project.outputPath(), "Alpha"+resultManifestExtension, "{not json") + }, + "compile of another key": func(t *testing.T, project projectStore) { + project.recordCompile(t, "other", true) + }, + "failed compile": func(t *testing.T, project projectStore) { + project.recordCompile(t, "input", false) + }, + "output changed": func(t *testing.T, project projectStore) { + writeOutputFile(t, project.outputPath(), "Alpha"+assemblyExtension, "changed by hand") + }, + } { + t.Run(name, func(t *testing.T) { + project := newProjectStore(t) + project.recordCompile(t, "input", true) + project.recordBaseline(t, "input") + + damage(t, project) + + if _, reason := project.store.LoadBaseline("Alpha"); reason != "baseline compile is no longer on disk" { + t.Errorf("reason = %q", reason) + } + }) + } +} + +// Verifies a missing baseline is explained from the compile result recorded without it. +func TestLoadBaselineExplainsAMissingBaseline(t *testing.T) { + for name, testCase := range map[string]struct { + record func(t *testing.T, project projectStore) + want string + }{ + "no compile result": { + record: func(*testing.T, projectStore) {}, + want: "no baseline: no full compile of this assembly is on record", + }, + "unreadable compile result": { + record: func(t *testing.T, project projectStore) { + writeOutputFile(t, project.outputPath(), "Alpha"+resultManifestExtension, "{not json") + }, + want: "no baseline: no full compile of this assembly is on record", + }, + "failed compile": { + record: func(t *testing.T, project projectStore) { project.recordCompile(t, "input", false) }, + want: "no baseline: the last full compile had errors", + }, + "successful compile": { + record: func(t *testing.T, project projectStore) { project.recordCompile(t, "input", true) }, + want: "no baseline: the last full compile did not record one", + }, + } { + t.Run(name, func(t *testing.T) { + project := newProjectStore(t) + testCase.record(t, project) + + if _, reason := project.store.LoadBaseline("Alpha"); reason != testCase.want { + t.Errorf("reason = %q, want %q", reason, testCase.want) + } + }) + } +} + +// Verifies a baseline that cannot be written is reported with the baseline's path. +func TestRecordBaselineReportsAMissingOutputDirectory(t *testing.T) { + root := t.TempDir() + store := Open[testDiagnostic](root, "missing") + + err := store.RecordBaseline("Alpha", BaselineInputs{InputKey: "input"}) + + want := "failed to write " + baselineRecordPath(filepath.Join(root, "missing"), "Alpha") + ":" + if err == nil || !strings.HasPrefix(err.Error(), want) { + t.Fatalf("err = %v, want it to start with %q", err, want) + } +} From cd104de22eb8ecbefccc2ac9bc06a4b1877fac9d Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 15 Oct 2026 12:38:08 +0900 Subject: [PATCH 3/3] test: pin the JSON form of a diagnostic The store's golden record test now uses a stand-in diagnostic type, so a change to the real diagnostic's field order, names or tags went unnoticed. The store records diagnostics as it is given them, so this form is part of what earlier runs left on disk. --- .../compilecheck/diagnostic_format_test.go | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 cli/dispatcher/internal/compilecheck/diagnostic_format_test.go diff --git a/cli/dispatcher/internal/compilecheck/diagnostic_format_test.go b/cli/dispatcher/internal/compilecheck/diagnostic_format_test.go new file mode 100644 index 000000000..b66a5872e --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/diagnostic_format_test.go @@ -0,0 +1,31 @@ +package compilecheck + +import ( + "encoding/json" + "testing" +) + +// Verifies the JSON form of a diagnostic, field names and order included. The result store records +// diagnostics exactly as it is handed them, so changing this form would change how the records an +// earlier run left are read, without anything marking those records as written by another version. +func TestDiagnosticKeepsTheJSONFormTheResultStoreRecords(t *testing.T) { + diagnostic := Diagnostic{ + Severity: severityError, + Code: "CS0103", + Message: "The name 'Missing' does not exist in the current context", + File: "Assets/Alpha/Alpha.cs", + Line: 3, + Column: 9, + } + + content, err := json.Marshal(diagnostic) + if err != nil { + t.Fatalf("failed to encode the diagnostic: %v", err) + } + + want := `{"Severity":"error","Code":"CS0103","Message":"The name 'Missing' does not exist in the current context",` + + `"File":"Assets/Alpha/Alpha.cs","Line":3,"Column":9}` + if string(content) != want { + t.Errorf("JSON form =\n%s\nwant\n%s", content, want) + } +}