From dbd1f6e63719b3f7d174bcc3805b5d4bfcee3fae Mon Sep 17 00:00:00 2001 From: Matt Thalman Date: Tue, 24 Mar 2026 15:11:38 -0500 Subject: [PATCH] Set deterministic FileVersion for Newtonsoft.Json to match MSFT-shipped package The upstream Newtonsoft.Json build computes a date-based FileVersion revision via PowerShell. Since source-build uses dotnet pack without this override, the 4th component defaulted to 0. This adds FileVersionRevision and composes the FileVersion to match the official package. Also updates the FileVersionRevision and ReleaseVersion validation tests to discover package IDs from build output instead of requiring explicit properties, and adds a new test that verifies release versions match built package output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eng/Versions.props | 1 + ...y-identitymodel-extensions-for-dotnet.proj | 2 +- .../projects/newtonsoft-json.proj | 6 + tests/SbrpTests/ExternalPackageTests.cs | 127 +++++++++++++++--- 4 files changed, 116 insertions(+), 20 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index d261d3d3d9..947a6569b4 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -19,6 +19,7 @@ 20230414.1 2.14.1 1.10.2 + 13.0.3 1.0.52 0.54.0 diff --git a/src/externalPackages/projects/azure-activedirectory-identitymodel-extensions-for-dotnet.proj b/src/externalPackages/projects/azure-activedirectory-identitymodel-extensions-for-dotnet.proj index 97d6471fa2..26c7ec5db2 100644 --- a/src/externalPackages/projects/azure-activedirectory-identitymodel-extensions-for-dotnet.proj +++ b/src/externalPackages/projects/azure-activedirectory-identitymodel-extensions-for-dotnet.proj @@ -7,7 +7,7 @@ 50722 - + Microsoft.IdentityModel.Tokens diff --git a/src/externalPackages/projects/newtonsoft-json.proj b/src/externalPackages/projects/newtonsoft-json.proj index 1702bbf0bd..b3c2fb334d 100644 --- a/src/externalPackages/projects/newtonsoft-json.proj +++ b/src/externalPackages/projects/newtonsoft-json.proj @@ -14,6 +14,12 @@ $(DotNetToolArgs) /p:PublicSign=true $(DotNetToolArgs) /p:TreatWarningsAsErrors=false $(DotNetToolArgs) /p:AdditionalConstants=SIGNED + + 27908 + + Newtonsoft.Json + $(DotNetToolArgs) /p:FileVersion=$(NewtonsoftJsonReleaseVersion).$(FileVersionRevision) $(DotNetTool) pack $(NewtonsoftJsonProjectPath) /bl:$(ArtifactsLogRepoDir)build.binlog $(DotNetToolArgs) diff --git a/tests/SbrpTests/ExternalPackageTests.cs b/tests/SbrpTests/ExternalPackageTests.cs index ea24fb02fa..a491d205a8 100644 --- a/tests/SbrpTests/ExternalPackageTests.cs +++ b/tests/SbrpTests/ExternalPackageTests.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading.Tasks; using System.Xml.Linq; +using NuGet.Packaging; using SbrpUtilities; using Xunit; using Xunit.Abstractions; @@ -87,17 +88,18 @@ public void SourceRevisionIdMatchesSubmoduleCommit() /// /// Validates that the FileVersionRevision property in external package .proj files /// matches the actual FileVersion revision from the Microsoft-shipped NuGet package. - /// The test downloads the package specified by FileVersionValidationPackage using the - /// NuGet protocol API (honoring NuGet.config sources), extracts a DLL, reads its - /// FileVersion, and compares the revision component. + /// The test discovers the package ID from the component's build output directory + /// (artifacts/obj/{component}/), downloads the same package from NuGet, extracts a DLL, + /// reads its FileVersion, and compares the revision component. /// See https://github.com/dotnet/source-build/issues/5509 /// - [Fact] + [SkippableFact] public async Task FileVersionRevisionMatchesPublishedPackage() { string repoRoot = PathUtilities.GetRepoRoot().TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); string projectsDir = Path.Combine(repoRoot, "src", "externalPackages", "projects"); string versionsPropsPath = Path.Combine(repoRoot, "eng", "Versions.props"); + string artifactsObjDir = Path.Combine(repoRoot, "artifacts", "obj"); string[] projFiles = Directory.GetFiles(projectsDir, "*.proj"); Assert.True(projFiles.Length > 0, $"No .proj files found in {projectsDir}"); @@ -107,7 +109,7 @@ public async Task FileVersionRevisionMatchesPublishedPackage() foreach (string projFile in projFiles) { - string packageName = Path.GetFileNameWithoutExtension(projFile); + string componentName = Path.GetFileNameWithoutExtension(projFile); XDocument doc = XDocument.Load(projFile); string? fileVersionRevision = doc.Descendants("FileVersionRevision").FirstOrDefault()?.Value; @@ -116,47 +118,134 @@ public async Task FileVersionRevisionMatchesPublishedPackage() continue; } - string? validationPackage = doc.Descendants("FileVersionValidationPackage").FirstOrDefault()?.Value; - if (string.IsNullOrEmpty(validationPackage)) + // Find matching release version in eng/Versions.props + string? releaseVersion = CommonUtilities.FindReleaseVersion(versionsPropsPath, componentName); + + if (string.IsNullOrEmpty(releaseVersion)) { - errors.Add($"{packageName}.proj: Has FileVersionRevision but no FileVersionValidationPackage property."); + errors.Add($"{componentName}.proj: No matching release version property found in eng/Versions.props."); checkedCount++; continue; } - // Find matching release version in eng/Versions.props - string? releaseVersion = CommonUtilities.FindReleaseVersion(versionsPropsPath, packageName); + // Find a .nupkg produced by this specific component + string componentObjDir = Path.Combine(artifactsObjDir, componentName); + if (!Directory.Exists(componentObjDir)) + { + Output.WriteLine($"Skipping {componentName}: build output directory not found ({componentObjDir})."); + continue; + } - if (string.IsNullOrEmpty(releaseVersion)) + string[] componentNupkgs = Directory.GetFiles(componentObjDir, "*.nupkg", SearchOption.AllDirectories); + + // Find the first package that contains a DLL so we can read its FileVersion + string? packageId = null; + foreach (string nupkg in componentNupkgs) { - errors.Add($"{packageName}.proj: No matching release version property found in eng/Versions.props."); - checkedCount++; + using PackageArchiveReader reader = new(nupkg); + bool hasDll = reader.GetLibItems() + .SelectMany(group => group.Items) + .Any(item => item.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)); + if (hasDll) + { + packageId = reader.NuspecReader.GetId(); + break; + } + } + + if (packageId is null) + { + Output.WriteLine($"Skipping {componentName}: no .nupkg with a DLL found in build output."); continue; } - // Download the package and read the FileVersion revision + // Download the published package and read the FileVersion revision var (revision, fileVersion) = await CommonUtilities.GetFileVersionRevisionAsync( - repoRoot, validationPackage, releaseVersion); + repoRoot, packageId, releaseVersion); if (revision is null) { - errors.Add($"{packageName}.proj: Unable to download {validationPackage} {releaseVersion} to validate FileVersionRevision."); + errors.Add($"{componentName}.proj: Unable to download {packageId} {releaseVersion} to validate FileVersionRevision."); checkedCount++; continue; } if (!int.TryParse(fileVersionRevision, out int expectedRevision) || expectedRevision != revision.Value) { - errors.Add($"{packageName}.proj: FileVersionRevision '{fileVersionRevision}' does not match " + - $"actual revision '{revision}' from {validationPackage} {releaseVersion} " + + errors.Add($"{componentName}.proj: FileVersionRevision '{fileVersionRevision}' does not match " + + $"actual revision '{revision}' from {packageId} {releaseVersion} " + $"(FileVersion: {fileVersion})."); } checkedCount++; } - Assert.True(checkedCount > 0, "No external packages with FileVersionRevision were found to validate."); + Skip.If(checkedCount == 0, "No components with FileVersionRevision had build output to validate."); Assert.True(errors.Count == 0, $"FileVersionRevision validation failed:{Environment.NewLine}{string.Join(Environment.NewLine, errors)}"); } + + /// + /// Validates that the release version configured in eng/Versions.props for each external + /// component matches the version of at least one NuGet package produced by that component. + /// The test scans each component's build output directory (artifacts/obj/{component}/) for + /// .nupkg files and verifies at least one has the expected version. + /// This catches cases where a submodule is updated but the release version is not. + /// + [SkippableFact] + public void ReleaseVersionMatchesPackageOutput() + { + string repoRoot = PathUtilities.GetRepoRoot().TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + string projectsDir = Path.Combine(repoRoot, "src", "externalPackages", "projects"); + string versionsPropsPath = Path.Combine(repoRoot, "eng", "Versions.props"); + string artifactsObjDir = Path.Combine(repoRoot, "artifacts", "obj"); + + string[] projFiles = Directory.GetFiles(projectsDir, "*.proj"); + Assert.True(projFiles.Length > 0, $"No .proj files found in {projectsDir}"); + + List errors = new(); + int checkedCount = 0; + + foreach (string projFile in projFiles) + { + string componentName = Path.GetFileNameWithoutExtension(projFile); + string? releaseVersion = CommonUtilities.FindReleaseVersion(versionsPropsPath, componentName); + + if (string.IsNullOrEmpty(releaseVersion)) + { + continue; + } + + // Scan the component's own build output directory for .nupkg files + string componentObjDir = Path.Combine(artifactsObjDir, componentName); + if (!Directory.Exists(componentObjDir)) + { + Output.WriteLine($"Skipping {componentName}: build output directory not found ({componentObjDir})."); + continue; + } + + string[] componentNupkgs = Directory.GetFiles(componentObjDir, "*.nupkg", SearchOption.AllDirectories); + if (componentNupkgs.Length == 0) + { + Output.WriteLine($"Skipping {componentName}: no .nupkg files found in build output."); + continue; + } + + string versionSuffix = $".{releaseVersion}.nupkg"; + bool foundMatch = componentNupkgs.Any(f => + Path.GetFileName(f).EndsWith(versionSuffix, StringComparison.OrdinalIgnoreCase)); + + if (!foundMatch) + { + string foundPackages = string.Join(", ", componentNupkgs.Select(Path.GetFileName)); + errors.Add($"{componentName}: Expected package version {releaseVersion} but found: {foundPackages}"); + } + + checkedCount++; + } + + Skip.If(checkedCount == 0, "No components with release versions had build output to validate."); + Assert.True(errors.Count == 0, + $"Release version validation failed:{Environment.NewLine}{string.Join(Environment.NewLine, errors)}"); + } }