From e7519e668c801f1f2b53d335bd72a6fca04eca19 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Wed, 8 Jul 2026 14:06:51 +1000 Subject: [PATCH] default to no bundling in PullRequests --- .../IntegrationTests/AuthorPackTests.cs | 49 +++++++++++++++++++ .../IntegrationTests/ThePackageBuilder.cs | 32 ++++++++++-- docs/BundlerDiagnosticCodes.md | 1 + readme.md | 15 ++++++ src/Directory.Build.props | 2 +- src/SponsorCheck/build/SponsorCheck.targets | 38 +++++++++++++- 6 files changed, 132 insertions(+), 5 deletions(-) diff --git a/IntegrationTests/IntegrationTests/AuthorPackTests.cs b/IntegrationTests/IntegrationTests/AuthorPackTests.cs index 5c333cc..982c622 100644 --- a/IntegrationTests/IntegrationTests/AuthorPackTests.cs +++ b/IntegrationTests/IntegrationTests/AuthorPackTests.cs @@ -297,6 +297,55 @@ public async Task MissingCredential_FailsPackWithSC102() await Assert.That(result.Combined).Contains("GitHub Sponsors: API token required"); } + [Test] + public async Task PullRequestBuild_SkipsBundling() + { + // On PR CI the platform credential is normally unavailable, so rather than failing SC102 the + // bundler is skipped and the package packs cleanly without the verifier. Simulate a PR via + // the AppVeyor signal — MSBuild reads the env-var-named property directly. + // forceBundleInPullRequest:false so the suite's hermeticity guard doesn't re-enable bundling. + var (result, feed) = await ThePackageBuilder.TryPackToFeed( + "ThePackage", + extraProperties: new Dictionary + { + ["APPVEYOR_PULL_REQUEST_NUMBER"] = "7" + }, + forceBundleInPullRequest: false); + + await Assert.That(result.ExitCode).IsEqualTo(0).Because(result.Combined); + await Assert.That(result.Combined).Contains("skipping sponsor-list bundling"); + + var nupkg = Directory.GetFiles(feed, "ThePackage.*.nupkg").Single(); + using var zip = ZipFile.OpenRead(nupkg); + var entries = zip.Entries.Select(_ => _.FullName).ToList(); + // No verifier, no bundled sponsor data, no tasks/ DLLs — all live inside the skipped target. + await Assert.That(entries.Any(_ => _ == "build/ThePackage.targets")).IsFalse(); + await Assert.That(entries.Any(_ => _.StartsWith("build/SponsorCheck."))).IsFalse(); + await Assert.That(entries.Any(_ => _.StartsWith("tasks/"))).IsFalse(); + } + + [Test] + public async Task PullRequestBuild_OverrideForcesBundling() + { + // true opts back in: the bundler runs even on a PR, so + // the verifier and bundled sponsor data are present exactly as on a normal build. + var (result, feed) = await ThePackageBuilder.TryPackToFeed( + "ThePackage", + extraProperties: new Dictionary + { + ["APPVEYOR_PULL_REQUEST_NUMBER"] = "7" + }, + forceBundleInPullRequest: true); + + await Assert.That(result.ExitCode).IsEqualTo(0).Because(result.Combined); + + var nupkg = Directory.GetFiles(feed, "ThePackage.*.nupkg").Single(); + using var zip = ZipFile.OpenRead(nupkg); + var entries = zip.Entries.Select(_ => _.FullName).ToList(); + await Assert.That(entries).Contains("build/ThePackage.targets"); + await Assert.That(entries).Contains("build/SponsorCheck.SponsorHashes.txt"); + } + [Test] public async Task SeverityOverrides_InvalidValue_FailsPackWithSC104() { diff --git a/IntegrationTests/IntegrationTests/ThePackageBuilder.cs b/IntegrationTests/IntegrationTests/ThePackageBuilder.cs index bdd94ff..6f3a0fe 100644 --- a/IntegrationTests/IntegrationTests/ThePackageBuilder.cs +++ b/IntegrationTests/IntegrationTests/ThePackageBuilder.cs @@ -61,7 +61,11 @@ public static async Task EnsureBuilt(string fixtureName) ["SponsorCheckVersion"] = sponsorCheckVersion, // Backdate pack so Consumer.RecentSponsor can have a SponsorshipStart that is // both AFTER the pack date and BEFORE today (i.e. not in the future). - ["SponsorCheck_PackDateOverride"] = "2024-01-01" + ["SponsorCheck_PackDateOverride"] = "2024-01-01", + // Force bundling so the suite is hermetic: if it runs on a PR CI build, the + // ambient PR env var would otherwise trip the bundler's pull-request skip and + // these fixtures would pack without the verifier, breaking pack assertions. + ["SponsorCheckBundleInPullRequest"] = "true" }, workDir, packagesDir).ConfigureAwait(false); @@ -130,7 +134,9 @@ public static async Task EnsureBuiltCombined(params string[] fixtureName ["SponsorListOverride"] = TestEnvironment.OverrideListPath, ["PackageOutputPath"] = feed, ["SponsorCheckVersion"] = sponsorCheckVersion, - ["SponsorCheck_PackDateOverride"] = "2024-01-01" + ["SponsorCheck_PackDateOverride"] = "2024-01-01", + // Force bundling for hermeticity — see the note in EnsureBuilt. + ["SponsorCheckBundleInPullRequest"] = "true" }, workDir, packagesDir).ConfigureAwait(false); @@ -162,6 +168,20 @@ public static async Task TryPack( string fixtureName, bool useOverrideList = true, IReadOnlyDictionary? extraProperties = null) + { + var (result, _) = await TryPackToFeed(fixtureName, useOverrideList, extraProperties).ConfigureAwait(false); + return result; + } + + /// Like but also returns the fresh feed dir so the caller can open the + /// produced nupkg. defaults to true for hermeticity + /// (so an ambient PR env var on CI doesn't disable the bundler); the PR-skip tests pass false to + /// exercise the actual pull-request skip. + public static async Task<(CliResult Result, string Feed)> TryPackToFeed( + string fixtureName, + bool useOverrideList = true, + IReadOnlyDictionary? extraProperties = null, + bool forceBundleInPullRequest = true) { var feed = Path.Combine(Path.GetTempPath(), "sponsorcheck-it-feed", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(feed); @@ -183,6 +203,11 @@ public static async Task TryPack( ["SponsorCheckVersion"] = sponsorCheckVersion, ["SponsorCheck_PackDateOverride"] = "2024-01-01" }; + if (forceBundleInPullRequest) + { + properties["SponsorCheckBundleInPullRequest"] = "true"; + } + if (useOverrideList) { properties["SponsorListOverride"] = TestEnvironment.OverrideListPath; @@ -196,13 +221,14 @@ public static async Task TryPack( } } - return await DotnetCliRunner.Run( + var result = await DotnetCliRunner.Run( "pack", Path.Combine(workDir, $"{fixtureName}.csproj"), "Release", properties, workDir, packagesDir).ConfigureAwait(false); + return (result, feed); } static string ExtractVersion(string nupkgPath) diff --git a/docs/BundlerDiagnosticCodes.md b/docs/BundlerDiagnosticCodes.md index 24c0686..a07a2cb 100644 --- a/docs/BundlerDiagnosticCodes.md +++ b/docs/BundlerDiagnosticCodes.md @@ -30,6 +30,7 @@ Every emitted message is prefixed with the code's short **Name** (e.g. `Platform - **Meaning:** A platform that requires a credential is missing one (GitHub Sponsors, Polar). Setup advice flips between user-secrets-first (local) and env-var-only (CI) based on `BuildServerDetector`. - **Syntax:** `{platformLabel}: API token required. {advice}` where `advice` is `Run \`dotnet user-secrets set SponsorCheck:{Platform}Token \` (recommended for local dev), or set the <{Platform}Token> MSBuild property, or set the '{Platform}Token' env var.` locally, or `Set the '{Platform}Token' env var (CI providers should expose their encrypted secret under this name; MSBuild auto-imports it as the <{Platform}Token> property).` on CI. - **Example:** `GitHub Sponsors: API token required. Run \`dotnet user-secrets set SponsorCheck:GitHubToken \` (recommended for local dev), or set the MSBuild property, or set the 'GitHubToken' env var.` +- **Pull-request builds:** on a detected pull-request CI build the bundler is *skipped* rather than failing with SC102 — the credential is normally unavailable on PRs and the PR package is throwaway (packs without the verifier). Force bundling on PRs with `true`. See readme → "Pull request builds". ### SC103 diff --git a/readme.md b/readme.md index 24e7598..026366e 100644 --- a/readme.md +++ b/readme.md @@ -449,6 +449,21 @@ dotnet user-secrets set "SponsorCheck:PolarToken" "polar_yyy" Recommended for CI builds, where there's no per-developer profile to hold a user-secrets file. Encrypt the token in the CI provider's secret store (AppVeyor "secure variable", GitHub Actions secret, Azure DevOps secret variable, etc.) and surface it as an env var named `GitHubToken`, `OpenCollectiveToken`, or `PolarToken`. MSBuild auto-imports env vars as properties, so no extra wiring is needed — the bundler picks them up via the same `` / `` / `` resolution path. The env var name must match the MSBuild property name modulo case (`GitHubToken`, `githubtoken`, and `GITHUBTOKEN` all resolve via case-insensitive property lookup), but punctuation matters — conventional CI names like `GITHUB_TOKEN` won't auto-flow. +#### Pull request builds + +Most CI providers withhold encrypted secrets from pull-request builds (especially PRs from forks), so the credential above isn't available — a pack there would otherwise fail with [SC102](docs/BundlerDiagnosticCodes.md#sc102). A PR build also never publishes the package it produces, so the sponsorship verifier that bundling would embed is throwaway anyway. + +So on a detected pull-request build the bundler is **skipped**: the package still packs — without the verifier — and no credential is required. A high-importance build message records that it happened. Detection covers the common providers via their PR-only signals — AppVeyor (`APPVEYOR_PULL_REQUEST_NUMBER`), GitHub Actions (`GITHUB_EVENT_NAME=pull_request`), Azure DevOps (`SYSTEM_PULLREQUEST_PULLREQUESTID`), GitLab, Bitbucket, Jenkins, Travis, CircleCI, Buildkite — and is deliberately conservative, so a real release build is never mistaken for a PR. + +To validate the verifier on PR builds that *do* have the credential, opt back in: + +```xml + + true + +``` + + ### Multiple packable projects in one repo For repos that produce multiple NuGet packages, configure once and let MSBuild's normal cascading mechanisms apply: diff --git a/src/Directory.Build.props b/src/Directory.Build.props index e992671..60d793f 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU1608;NU1109;NU1901;PolyfillTargetsForNuget - 0.13.0 + 0.14.0 preview + + <_SponsorCheck_IsPullRequest>false + <_SponsorCheck_IsPullRequest Condition="'$(APPVEYOR_PULL_REQUEST_NUMBER)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(SYSTEM_PULLREQUEST_PULLREQUESTID)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(GITHUB_EVENT_NAME)' == 'pull_request' or '$(GITHUB_EVENT_NAME)' == 'pull_request_target'">true + <_SponsorCheck_IsPullRequest Condition="'$(CHANGE_ID)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(CI_MERGE_REQUEST_IID)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(BITBUCKET_PR_ID)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(CIRCLE_PULL_REQUEST)' != ''">true + <_SponsorCheck_IsPullRequest Condition="'$(TRAVIS_PULL_REQUEST)' != '' and '$(TRAVIS_PULL_REQUEST)' != 'false'">true + <_SponsorCheck_IsPullRequest Condition="'$(BUILDKITE_PULL_REQUEST)' != '' and '$(BUILDKITE_PULL_REQUEST)' != 'false'">true + + <_SponsorCheck_SkipForPullRequest>false + <_SponsorCheck_SkipForPullRequest Condition="'$(_SponsorCheck_IsPullRequest)' == 'true' and '$(SponsorCheckBundleInPullRequest)' != 'true'">true + + + + + + + + Condition="'$(Configuration)' == 'Release' and '$(IsPackable)' == 'true' and '$(_SponsorCheck_SkipForPullRequest)' != 'true'"> <_SponsorCheck_HashListPath>$(IntermediateOutputPath)SponsorCheck\SponsorHashes.txt <_SponsorCheck_GeneratedTargetsPath>$(IntermediateOutputPath)SponsorCheck\$(PackageId).targets