fix: compile-check no longer recompiles dependents of an assembly whose body alone changed when Unity's source order is not ordinal

Merged
HA
18h ago
+366-29

Summary

  • compile-check now hands csc an assembly's sources in the order Unity's response file lists them. An assembly whose body alone changed gets a reference assembly byte-identical to Unity's again, so its dependents are skipped instead of compiled.

User Impact

Before: the rebuilt source list was sorted ordinally. Unity 2022.3 writes many response files in another order (36 of the 100 multi-source response files in the development project), and csc emits types in the order of its sources. Editing only a method body in such an assembly gave a reference assembly whose types sat in another order than Unity's; the same-API comparison read the reordering as a change, and every dependent compiled again.

After: in the development project, editing one method body in an assembly with six dependents compiles that assembly alone. Its reference assembly has the same sha256 as Unity's, so no comparison is needed and the dependents are skipped. With the previous build the same edit recompiled six assemblies, and 13 assemblies needed a same-API comparison that answered changed; with this change none needed one.

The first run after updating compiles again every assembly it would otherwise have reused from a recorded result. Those results were compiled from ordinally sorted sources.

Changes

  • RebuildSources returns the recorded sources in response file order (a source listed twice counts once), followed by the sources added since the last Unity build in ordinal order. Both branches do this: the re-globbed one and the package-cache / no-.asmdef one. Which sources are compiled and how each is spelled are unchanged. A source the response file records but the glob no longer produces is still checked before any ordering happens, and the errors are unchanged.
  • Added sources are appended rather than slotted in where Unity would put them. Unity versions order the response file differently (Unity 6000.3 writes them in ordinal order), and appending keeps the relative order of every recorded source.
  • The compile result format goes from version 1 to 2, so a result recorded under the same key with outputs from sorted sources is not reused.
  • LoadBaseline refuses a baseline whose compile result has another format version, with the existing reason no baseline: it was recorded by another version of compile-check. Without that check, a changed-files check would keep handing dependents the full compile's old outputs, because it leaves them in place. The baseline and changed-files-check record formats are unchanged; a baseline tied to an old result is no longer used.
  • The input key does not change: the order follows from the response file and the source set, which the key already covers.
  • docs/compile-check.md step 3 says why the order is kept, and rewraps a long line left by the previous pull request.

Verification

  • New tests:

    • response file order that is neither ordinal nor case-insensitive, with a deleted source in the middle;
    • added sources in ordinal order after the recorded ones (N/M.cs is walked before N.cs, and C.cs sorts before b.cs only when case counts);
    • a source of an .asmref folder at the head of the response file;
    • a source only the rescue keeps (under a stale duplicate .asmdef), whose rescue is checked in the same test by rebuilding without it in the response file;
    • the package-cache and no-.asmdef branches;
    • a source the response file lists twice is compiled once, in both branches;
    • a plan-to-written-response-file test, so nothing on the way sorts the sources again;
    • a format version 1 result is not reused;
    • LoadBaseline refuses a baseline whose result is version 1, and a body edit then compiles in full with that reason.

    Each new test failed before the fix.

  • Existing tests whose expectations changed (order only): TestRebuildSourcesKeepsSourcesOutsideTheAssemblyDirectory, TestRebuildSourcesGlobsANewSourceInsideAnOutsideAsmrefFolder, and the written format version in TestWriteUnitResultManifestWritesTheRecordedFormat.

  • Mutations, each restored afterwards:

    MutationFails
    Sort the result at the end againresponse file order, added sources, .asmref folder, rescued source, written response file (plus the two adjusted tests)
    Append added sources in walk orderadded sources
    Sort added sources case-insensitivelyadded sources
    Order recorded sources by the walk (own folder, then attached folders)response file order, added sources, .asmref folder, rescued source, written response file
    Move rescued sources to the endrescued source
    Keep sorting without an .asmdef / in the package cachethe no-.asmdef test / the package-cache test
    Compile result format back to 1the version 1 result test, both baseline tests, the written format test
    Drop the format version check in LoadBaselineboth baseline tests
    Sort the sources when writing the response filewritten response file
    Keep a source the response file lists twiceduplicate source
  • In cli/dispatcher:

    • golangci-lint fmt --diff is empty;
    • go vet, golangci-lint run (0 issues) and the complexity config (0 issues) pass;
    • go test ./... passes with coverage 94.33 (baseline 94.2);
    • go test -race passes on internal/compilecheck/... and internal/dispatcher.
  • scripts/check-file-length.sh and scripts/sync-tool-docs.sh --check pass.

  • Checked in the Unity 2022.3 development project. The build before this change ran first and this one second, so the older build never read the newer records.

    • The edit: a temporary change to one string literal inside a method body, in an assembly whose response file is not in ordinal order and which has six dependents. It has since been reverted.
    • Before: the rebuilt reference assembly's sha256 differed from Unity's, the comparison answered changed, and six dependents compiled.
    • After: the sha256 matched Unity's, no comparison ran, and the dependents were skipped.
  • Repository CI does not run on pull requests into this branch; the checks above were run locally.