CI-related improvements - #740
Merged
Merged
Conversation
M-r-A
added a commit
to M-r-A/Multiplayer
that referenced
this pull request
Jul 27, 2026
Researched actions/setup-dotnet issue rwmt#740 first: v5.3.0 had a bug where global.json + rollForward silently skipped installing a pinned prerelease SDK. Confirmed our pinned v6.0.0 SHA is 8 commits ahead of the fix (PR rwmt#742), so it doesn't affect us. Confirmed A.B.x version format and global.json's default allowPrerelease already include preview builds without extra config. Mark the '11' leg continue-on-error at the job level: previews can have real upstream bugs unrelated to this repo (e.g. a confirmed dotnet-tool crash in .NET 11 preview 2), and this leg exists to catch upcoming breaks early, not to gate merges on preview-SDK issues.
romangr
pushed a commit
to RimWorld-mods-patches/Multiplayer
that referenced
this pull request
Aug 4, 2026
(#8) * Downgrade Microsoft.CodeAnalysis packages from 5.3.0 to 4.13.0 (rwmt#970) The 5.3.0 analyzer packages require Roslyn 5.3.0+, which is only bundled in .NET SDK 10.0.300+. However, build-beta.yml specifies dotnet 9.x, which bundles Roslyn 4.14.0. This causes CS9057 analyzer loading failures and cascading CS8795 partial method implementation errors. Version 4.13.0 is compatible with Roslyn 4.14+ (bundled in 9.x SDKs) and also compatible with 10.x and 11.x SDKs, allowing builds to succeed on all. * Add global.json pinning .NET SDK to 9.0.x for local development Specify SDK 9.0.0 with rollForward: latestMinor to ensure all developers build against 9.0.x locally, matching the CI workflow's baseline version. This prevents accidental SDK mismatches between local and CI environments. * Add SDK version observability to all CI workflows Add "Display SDK version" step to all pipeline workflows for better observability and troubleshooting: - build-beta.yml: Shows SDK in each matrix job (9.x, 10.x) - pr-check.yml: Shows SDK version when validating PRs - build-workshop.yml: Shows SDK when building releases This ensures CI logs explicitly show which .NET SDK version was used, making it easier to diagnose SDK-related issues and verify that intended versions are being tested. * Add SDK version display to build-beta workflow Display the .NET SDK version used in the build for better observability and troubleshooting in CI logs. * Add NuGet dependency caching to workflows Cache ~/.nuget/packages using packages.lock.json hash. Expected 70-90% cache hit rate, reducing workflow time by 2-3 minutes per run. * Add NuGet dependency caching to PR validation Cache ~/.nuget/packages using packages.lock.json hash. Expected 70-90% cache hit rate, reducing PR validation time by 2-3 minutes per run. * Add cross-platform matrix testing to PR checks Test builds on ubuntu-latest, windows-latest, and macos-latest in parallel. Catches platform-specific issues (CRLF, path separators, line endings). Job names now display the OS being tested via matrix label. Uses fail-fast: false to report all platform failures, not just the first. * Fix test result logging in PR checks Add explicit TRX logger to dotnet test step so TestResults directory is created on all platforms. This allows the artifact upload step to successfully capture test results for failed runs, enabling post-mortem debugging across Ubuntu, Windows, and macOS. * Pin pr-check.yml actions to latest versions with commit SHAs Upgrade all 4 GitHub Actions to latest stable releases: - actions/checkout: v4 → v7.0.1 (3d3c42e) - actions/setup-dotnet: v4 → v6.0.0 (a98b568) - actions/cache: v4 → v6.1.0 (55cc834) - actions/upload-artifact: v4 → v7.0.1 (043fb46) Pin by full commit SHA instead of floating tags to prevent supply-chain risks and ensure reproducible builds. Add version tags in comments for readability. All major version bumps (up to 3 versions per action) are backward-compatible for this workflow: runtime Node 20→24 (GitHub runners already support), checkout fork-blocking doesn't apply (uses plain pull_request trigger), and setup-dotnet/cache inputs unchanged. Fix artifact collision bug: upload-artifact v4+ requires unique artifact names when used in a matrix. Change fixed name "test-results" to include OS via "test-results-${{ matrix.os }}" so 3 matrix legs produce 3 distinct artifacts instead of colliding. * Add timeout-minutes to PR check job Neither the job nor any step had a timeout, so a hang anywhere in restore/build/test would run until GitHub's 6-hour default before being killed. We already saw Tests.ServerTest.Test() hang on macOS and only stop because NUnit's own 3s test-level timeout caught it, which is not a CI-level safety net. Cap the job at 15 minutes. * Cancel stale PR check runs on new pushes No concurrency group existed, so every push to a PR branch started a fresh 3-way OS matrix without cancelling the previous run. A few quick force-pushes could pile up 9+ simultaneous jobs. Group runs by workflow and ref, cancelling in-progress runs so only the latest push keeps running. * Fail loudly when test result artifacts are missing actions/upload-artifact defaults to if-no-files-found: warn. This is exactly why the earlier "No files were found with the provided path: **/TestResults/**" bug sat unnoticed in run logs instead of failing CI - we only caught it by reading warnings manually. Set to error so a missing-artifact regression fails the build instead of hiding in warnings. * Add explicit least-privilege permissions to PR checks No permissions block was set, so the workflow ran with whatever the repo/org default GITHUB_TOKEN scope is, which can be broad (e.g. contents: write). This workflow only checks out code and runs tests - it never needs write access. Confirmed against dotnet/aspnetcore: all 12 of their hand-authored workflows declare this explicitly, scoped to exactly what each one does, with zero exceptions. * Fix NuGet cache key by switching to setup-dotnet's built-in cache The repo has no packages.lock.json anywhere and RestorePackagesWithLockFile is not set, so hashFiles('**/packages.lock.json') in the manual actions/cache step always evaluated to an empty string. Confirmed directly in run 30247575118's logs: the cache key resolved to a bare trailing dash ("key: Linux-nuget-", etc.) on all 3 legs, and Ubuntu's post-job cleanup logged "Cache hit occurred on the primary key Linux-nuget-, not saving cache." The cache was permanently frozen on its first-ever save and would never update again. Remove the manual actions/cache step and use actions/setup-dotnet's built-in cache instead, pointed at cache-dependency-path: '**/*.csproj' so the key changes whenever a package or version is added or bumped. Same pattern dotnet/aspnetcore uses for npm caching via setup-node's cache: 'npm' input. * Parallelize Debug/Release builds and test both configurations Split configuration into a second matrix axis alongside os, so Debug and Release build and test in parallel instead of sequentially in one leg (3 OS x 2 config = 6 legs). Testing only Debug was a real gap: this repo ships Release builds (build-beta.yml), and Release-mode JIT optimizations can surface bugs that never reproduce in Debug. The test suite takes ~8s, so testing both configs is negligible added cost. * Read .NET SDK from global.json and add a forward-compat SDK matrix dotnet-version: 9.x was hardcoded in the workflow while global.json already pins the SDK to 9.0.0 with rollForward: latestMinor for local dev, so the two could silently drift apart. Add a sdk matrix axis: one leg reads the version from global.json (single source of truth, tracks whatever's pinned there automatically), the other explicitly targets 10.0.x to catch forward-compatibility issues against the next SDK early. rollForward: latestMinor keeps the global.json leg within 9.x, so the two legs stay meaningfully distinct. This is a third matrix axis alongside os and configuration (3 x 2 x 2 = 12 legs). Artifact names include the sdk leg to avoid collisions. * Fix invalid SDK version in global.json "9.0.0" isn't a valid full SDK version: Microsoft's global.json docs require major.minor.featureband+patch (e.g. 10.0.100) and explicitly list "10.0" as an invalid example producing this same error class. actions/setup-dotnet's global-json-file parser enforces this strictly and failed once F6 started actually reading this file in CI. Use 9.0.316. rollForward: latestMinor is documented as major-version- scoped ("matches the requested major"), so this still only ever resolves within 9.x - it can't roll forward to 10.x. * Fix SDK override leg being silently ignored by committed global.json The committed global.json always wins SDK resolution, so the "10" matrix leg either crashed (Windows) or silently tested the pinned version instead of 10.x (Ubuntu/macOS) - confirmed via run 30252784775 logs, where dotnet --version printed the wrong version on every "10" leg. Fix per https://github.com/actions/setup-dotnet#matrix-testing: rewrite global.json before Set up .NET runs, anchored to this leg's own major version. * Add non-blocking dotnet format check to PR checks Verified locally that dotnet format --verify-no-changes exits non-zero against the current codebase (236/449 files, mostly WHITESPACE/CHARSET violations) despite an existing .editorconfig, so gating on it would fail every PR regardless of what it touches. Add it as a separate single-run job (formatting doesn't depend on os/configuration/sdk, so running it 12x in the build matrix would be wasteful) with continue-on-error: true, so violations show up in the job's log without blocking merges until the existing debt is cleaned up separately. * Apply a consistent naming convention to pr-check.yml Every step name is now imperative verb + object, sentence case, with short parallel qualifiers only where needed to disambiguate ((pinned)/ (override)) or flag job-level behavior ((non-blocking)). Renames the builds job to build-test / "Build, Test" since the old name undersold what it does, and names both checkout steps explicitly. * Fix DOTNET_CLI_TELEMETRY_OPTOUT being silently ineffective Both env vars were scoped only to the "Set up .NET" steps, not Restore/Build/Test. Per Microsoft's telemetry docs, DOTNET_NOLOGO only suppresses the disclosure text and "has no effect on telemetry opt out" - the opt-out itself is re-checked on every CLI invocation with no persisted/sentinel behavior. So telemetry was actually still being sent on every restore/build/test/format call across all legs, despite the apparent intent to disable it. Move both to the top-level workflow env block so they apply to every step in every job, matching actions/setup-dotnet's own documented usage pattern. * Add .NET 11 preview to the forward-compat SDK matrix Researched actions/setup-dotnet issue rwmt#740 first: v5.3.0 had a bug where global.json + rollForward silently skipped installing a pinned prerelease SDK. Confirmed our pinned v6.0.0 SHA is 8 commits ahead of the fix (PR rwmt#742), so it doesn't affect us. Confirmed A.B.x version format and global.json's default allowPrerelease already include preview builds without extra config. Mark the '11' leg continue-on-error at the job level: previews can have real upstream bugs unrelated to this repo (e.g. a confirmed dotnet-tool crash in .NET 11 preview 2), and this leg exists to catch upcoming breaks early, not to gate merges on preview-SDK issues. * Fix .NET 11 override leg and add SDK-major verification A generic version anchor + rollForward doesn't reliably match a preview-only install (dotnet/sdk#12335, reproduced for the "11" leg). Pin global.json to the exact resolved version instead, captured via setup-dotnet's own output. Also assert the resolved SDK's major version matches what each leg expects, so a repeat of the earlier silent wrong-SDK bug fails loudly instead of requiring a manual log read. * Add persist-credentials: false to checkout steps Neither job pushes anything, so there's no reason for the checked-out git config to retain a usable credential. Found in dotnet/aspnetcore's runtime-sync.yml during earlier research; reduces blast radius if a build step or dependency ever tries to misuse the token. * Publish mod and server artifacts from PR checks Add a package-artifacts job that builds the mod (Release) and publishes the server for win-x64/linux-x64, uploading both as plain workflow artifacts - no write permission needed, works for fork PRs. Runs once on ubuntu-latest rather than as a conditional matrix step. * Give published artifacts a proper root folder path: output/Multiplayer/ strips "Multiplayer" itself and flattens its contents to the zip root - verified against actions/toolkit's zip spec logic. Stage each output under its own parent directory instead and point path: at that parent, so Multiplayer/ and Server/ survive as the zip's top-level folder. * Reorder build-test job name to (os, sdk, configuration) * Embed PR number and short SHA into artifact names Keeps zips traceable after download, once GitHub's run-scoped context is gone. E.g. Multiplayer-mod-pr123-a1b2c3d. * Use 9.0.100 as the global.json SDK anchor * Remove NuGet caching from build-beta.yml Keep this workflow's only change to Display SDK version, as originally scoped - the caching addition didn't belong here. Also restores the "Setup Dotnet" step name, which an earlier commit renamed to "Setup .NET" as an unrelated side effect of matrix work that was since reverted. * Allow re-runs to overwrite test-result artifacts test-results-* is keyed by matrix values, not run attempt, so re-running a flaky test leg without a new push hit a 409 conflict against the first attempt's artifact instead of replacing it. Scoped to test-results only - a package-artifacts failure is rarer and a fresh push is the more normal recovery path there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.