diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 0ca224750d..f37b0d9ea8 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -83,9 +83,10 @@ jobs: # Unique per build: the date alone is not. Multiple dispatches a day are normal here - over # 2026-08-24..09-12, 13 of 20 days published two or more distinct source trees under one # version string, peaking at five - and nothing in the artifact then distinguishes them. - # `FileVersion` / `ProductVersion` / `InformationalVersion` and the asset names all carry - # this string, and the apphost exe is byte-identical across builds, so with a colliding - # stamp there is no read that answers "which nightly is this". + # `InformationalVersion` — and the Win32 ProductVersion built from it — and the asset names + # all carry this string (#3455 is what actually threads it into the binaries; FileVersion + # stays the numeric prefix), and the apphost exe is byte-identical across builds, so with a + # colliding stamp there is no read that answers "which nightly is this". # # run_number is monotonic and numeric, so SemVer 2.0 compares these prereleases in build # order; the date stays leftmost and human-readable. A short sha would identify the tree @@ -142,6 +143,27 @@ jobs: cache: true cache-dependency-path: '**/packages.lock.json' + # The stamp must reach the BINARIES, not just the names around them. This step's output feeds + # every artifact name and the vpk pack below, but a name is not a stamp: each assembly records + # whatever the msbuild version property resolves to at build time, and that is the clean + # release string Directory.Build.props declares unless the build overrides it. Until #3455 + # nothing did, so a zip named 3.7.0-nightly.20260915.438 shipped a service whose informational + # version read bare 3.7.0 — and the informational version is the one surface a RUNNING service + # answers build questions from: DarlingCliCommands.ProductVersion() reads that attribute for + # --version, and #3453's get_collection_health service block deliberately serves the same + # method's result. The night #3453 shipped, its first live reading came back bare on a .438 + # nightly — the question "which nightly is this" answered with silence at the read built to + # answer it. + # + # So every dotnet publish below passes -p:Version with this value. The SDK derives the + # informational version from it verbatim, while AssemblyVersion and FileVersion derive as the + # numeric prefix (3.7.0.0) — identical to a release build, so nothing assembly-identity-shaped + # differs between the two. No +sha metadata rides along: Directory.Build.props suppresses the + # source-revision append tree-wide (#3222), the stamp's run_number already names the build + # uniquely, and ProductVersion() strips any +suffix from display regardless. Release builds + # keep their clean string by CONSTRUCTION rather than by restraint — build.yml passes no + # version property — and NightlyVersionInjectionTests pins both halves, so the next copied-in + # publish step cannot quietly ship a bare-versioned nightly again. - name: Set nightly version id: version shell: pwsh @@ -190,20 +212,20 @@ jobs: # Lite/PerformanceMonitorLite.csproj declares RuntimeIdentifiers so the committed lock # file covers this RID; see the comment there before removing either half. - name: Publish Lite - run: dotnet publish Lite/PerformanceMonitorLite.csproj -c Release -r win-x64 --self-contained -o publish/Lite + run: dotnet publish Lite/PerformanceMonitorLite.csproj -c Release -r win-x64 --self-contained -o publish/Lite -p:Version=${{ steps.version.outputs.VERSION }} - name: Publish Darling Service - run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -o publish/DarlingService + run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -o publish/DarlingService -p:Version=${{ steps.version.outputs.VERSION }} - name: Publish Darling Viewer - run: dotnet publish Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj -c Release -o publish/DarlingViewer + run: dotnet publish Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj -c Release -o publish/DarlingViewer -p:Version=${{ steps.version.outputs.VERSION }} # Self-contained viewer publish that feeds the remote-seat Velopack Setup.exe (#1555), the same # publish shape build.yml uses for the Dashboard/Lite Velopack packs. This is IN ADDITION to the # framework-dependent "Publish Darling Viewer" above, which still feeds the co-located viewer\ # folder inside PerformanceMonitorDarling-*.zip — that zip is unchanged. - name: Publish Darling Viewer (self-contained for Velopack) - run: dotnet publish Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj -c Release -r win-x64 --self-contained -o publish/DarlingViewer-velopack + run: dotnet publish Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj -c Release -r win-x64 --self-contained -o publish/DarlingViewer-velopack -p:Version=${{ steps.version.outputs.VERSION }} # Same cache key as build.yml's release path: the fetch script's content hash (the SHA256 # pins live inside it). The ~340MB EDB/TimescaleDB fetch runs at most once per pin-set per @@ -518,8 +540,11 @@ jobs: echo "VERSION=${base}-nightly.${NIGHTLY_STAMP}" >> "$GITHUB_OUTPUT" echo "Nightly version: ${base}-nightly.${NIGHTLY_STAMP}" + # -p:Version for the reason the windows build job's version step documents (#3455): the name + # on the tarball is not the stamp in the service, and the informational version is what + # --version prints and what the get_collection_health service block serves. - name: Publish service (linux-x64) - run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -r linux-x64 --self-contained false -o publish/DarlingService-linux + run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -r linux-x64 --self-contained false -o publish/DarlingService-linux -p:Version=${{ steps.version.outputs.VERSION }} - name: Package linux artifact + checksum shell: bash @@ -544,7 +569,12 @@ jobs: version="${{ steps.version.outputs.VERSION }}" image="ghcr.io/${{ github.repository_owner }}/performancemonitor-darling" echo "$GH_TOKEN" | docker login ghcr.io -u "${{ github.actor }}" --password-stdin - docker build -f Darling/Dockerfile -t "${image}:nightly" -t "${image}:${version}" . + # VERSION reaches the publish that runs INSIDE the image build (#3455): an image tagged + # with the nightly string whose service answers the clean release string is the zip + # defect again, one artifact over. The Dockerfile defaults the arg to empty, so + # build.yml's release image and any hand-run docker build keep publishing against the + # clean declared version. + docker build -f Darling/Dockerfile --build-arg VERSION="${version}" -t "${image}:nightly" -t "${image}:${version}" . docker push "${image}:nightly" docker push "${image}:${version}" diff --git a/Darling/Darling.Tests/NightlyVersionInjectionTests.cs b/Darling/Darling.Tests/NightlyVersionInjectionTests.cs new file mode 100644 index 0000000000..125125e34d --- /dev/null +++ b/Darling/Darling.Tests/NightlyVersionInjectionTests.cs @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Linq; +using Xunit; +using static Darling.Tests.RepoFile; + +namespace Darling.Tests; + +/// +/// Every build the nightly workflow publishes receives the derived nightly version as an msbuild +/// property, and no release build does (#3455). +/// +/// The defect this closes was a name without a stamp. nightly.yml derives its +/// prerelease string once per run (#3390) and named every artifact with it, but no publish ever passed +/// it to msbuild, so the assemblies inside those artifacts stamped the clean release string +/// Directory.Build.props declares. The first live reading of #3453's service block — +/// built precisely to answer "is the running service the build that carries fix X" — came back bare +/// 3.7.0 on a .438 nightly, on every store that had installed it. The informational +/// version is the one surface that read and the CLI's --version share, and it derives from the +/// version property at build time, so a publish that does not pass the property cannot stamp the +/// nightly, however correct the artifact's name is. +/// +/// Why a pin and not the workflow comment alone. The stamp's own #3390 rationale already +/// CLAIMED the binaries carried it while they did not — prose describing intent outlives the mechanics +/// it describes, which is the drift these source-parsing tests exist to catch. The routes enumerated: +/// a publish step arrives in nightly.yml without the flag, the ordinary fate of hand-copied +/// steps (#2384's checksum fix landing in one workflow and not the other is the local precedent); the +/// container's inner publish loses the arg threading on either side of the docker build +/// boundary, where each half alone reads as complete at its own file; and the inverse, a version +/// property leaking into build.yml, which would take the release binaries' stamp away from the +/// one declaration the release process bumps and check-version-bump.yml gates (#3222). +/// +public sealed class NightlyVersionInjectionTests +{ + private const string NightlyWorkflow = ".github/workflows/nightly.yml"; + private const string ReleaseWorkflow = ".github/workflows/build.yml"; + private const string ContainerFile = "Darling/Dockerfile"; + + /// The exact spelling every nightly publish must carry: the version step's own output, + /// not a re-derivation beside it — the same one-authority rule the version-set steps already obey + /// for the artifact names. + private const string VersionFlag = "-p:Version=${{ steps.version.outputs.VERSION }}"; + + [Fact] + public void EveryNightlyPublish_PassesTheDerivedVersionToMsBuild() + { + var offending = PublishLines(NightlyWorkflow) + .Where(line => !line.Contains(VersionFlag, StringComparison.Ordinal)) + .ToList(); + + Assert.True( + offending.Count == 0, + "A nightly publish that does not pass the derived version to msbuild builds assemblies " + + "stamping the clean release string inside an artifact named nightly, and two same-day " + + "nightlies become indistinguishable at the read built to distinguish them (#3455):\n " + + string.Join("\n ", offending)); + } + + [Fact] + public void NoReleasePublish_PassesAVersionProperty() + { + var offending = PublishLines(ReleaseWorkflow) + .Where(line => line.Contains("-p:Version", StringComparison.Ordinal)) + .ToList(); + + Assert.True( + offending.Count == 0, + "A release publish passing a version property takes the release binaries' stamp away from " + + "the one declaration the release process bumps (#3222). Release builds stay clean by " + + "CONSTRUCTION — the property flows only where a prerelease stamp exists to flow:\n " + + string.Join("\n ", offending)); + } + + [Fact] + public void TheNightlyContainerBuild_ThreadsTheVersionThroughTheImageBoundary() + { + /* Both halves or neither. The workflow passing an arg the Dockerfile does not declare stamps + nothing; the Dockerfile declaring an arg the workflow does not pass stamps the clean string. + Either failure leaves a :nightly image whose service answers with the release string, and + each half alone reads as complete at its own file. */ + var dockerBuilds = ReadRepoFileLf(NightlyWorkflow) + .Split('\n') + .Select(line => line.Trim()) + .Where(line => line.StartsWith("docker build", StringComparison.Ordinal)) + .ToList(); + + var build = Assert.Single(dockerBuilds); + Assert.Contains("--build-arg VERSION=\"${version}\"", build, StringComparison.Ordinal); + + /* Continuations joined before the line scan: the publish RUN is split across lines, and a + per-line scan would read the flag's line as not containing a publish and vice versa. */ + var containerFile = ReadRepoFileLf(ContainerFile).Replace("\\\n", " ", StringComparison.Ordinal); + + Assert.Contains("ARG VERSION=", containerFile, StringComparison.Ordinal); + + var publishes = containerFile + .Split('\n') + .Where(line => line.Contains("dotnet publish", StringComparison.Ordinal)) + .ToList(); + + var inner = Assert.Single(publishes); + + /* The conditional expansion, not a bare flag: an empty arg must add NOTHING, so a release or + hand-run docker build keeps publishing against the declared version rather than against an + empty property that would override it with nothing. */ + Assert.Contains("${VERSION:+-p:Version=$VERSION}", inner, StringComparison.Ordinal); + } + + /// The dotnet publish commands in one workflow, comment lines excluded so a rationale + /// quoting a command is not judged as one. + private static List PublishLines(string workflow) + { + var lines = ReadRepoFileLf(workflow) + .Split('\n') + .Select(line => line.Trim()) + .Where(line => !line.StartsWith("#", StringComparison.Ordinal)) + .Where(line => line.Contains("dotnet publish ", StringComparison.Ordinal)) + .ToList(); + + /* Non-vacuity: nightly.yml publishes four windows trees and one linux tree, build.yml six. + A scan reading nothing must fail here rather than satisfy every assertion above by having + nothing to judge. */ + Assert.True( + lines.Count >= 5, + $"only {lines.Count} publish lines found in {workflow}; the scan is not reading the file"); + + return lines; + } +} diff --git a/Darling/Darling.Tests/RepoFileAdoptionTests.cs b/Darling/Darling.Tests/RepoFileAdoptionTests.cs index 232067b60b..3256cc6886 100644 --- a/Darling/Darling.Tests/RepoFileAdoptionTests.cs +++ b/Darling/Darling.Tests/RepoFileAdoptionTests.cs @@ -116,6 +116,7 @@ public sealed class RepoFileAdoptionTests "FleetCardCollectionStaleNamesItsPopulationTests.cs", "FleetPageAttentionFilterTests.cs", "LockedModeRestoreCoverageTests.cs", + "NightlyVersionInjectionTests.cs", "PgCpuCapacityHeadroomTests.cs", "PgIndexBloatGridReachTests.cs", "PgLoggingCollectorOffTests.cs", diff --git a/Darling/Dockerfile b/Darling/Dockerfile index af9cb9d961..a547875b78 100644 --- a/Darling/Dockerfile +++ b/Darling/Dockerfile @@ -23,10 +23,21 @@ # container had an independent opinion about which SDK to use. Pinning removes that second opinion: bump # this line and global.json together, deliberately, instead of being bumped by an upstream tag move. FROM mcr.microsoft.com/dotnet/sdk:10.0.302 AS build + +# The msbuild version property for the publish below, threaded in by nightly.yml (#3455) so the +# service compiled into a :nightly image stamps the same 3.7.0-nightly.. string its image +# tag carries. The informational version is what --version prints and what get_collection_health's +# service block serves, so a container answering with the clean release string while running a +# nightly is the defect that issue documents on the zip, one artifact over. Empty by default ON +# PURPOSE: the shell expansion below then adds no flag at all, the publish inherits the version +# Directory.Build.props declares, and every other consumer of this Dockerfile — build.yml's release +# image, a hand-run docker build — produces byte-for-byte the command it produced before the ARG +# existed. +ARG VERSION= WORKDIR /src COPY . . RUN dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj \ - -c Release -o /app/publish + -c Release -o /app/publish ${VERSION:+-p:Version=$VERSION} FROM mcr.microsoft.com/dotnet/aspnet:10.0 # Microsoft.Data.SqlClient probes the Kerberos GSSAPI library at connect time on Linux — even for SQL