Skip to content

Commit 0f47fd3

Browse files
bouwkastandrewlock
andauthored
Cleanup and fix the GeneratePackageVersions (#8478)
## Summary of changes This goes back through the recent changes (#8371 and #8340) to `GeneratePackageVersions` and attempts to do some removals, refactorings, and fixes that have come up. Notably this removes the `nuget_cache`, fixes `IncludePackages` (again 😅), and rewrites the cooldown logic to not use `supported_versions.json` and `nuget_cache` and instead reads from the `.g.cs` files in addition to refactoring the verbage used throughout. ## Reason for change While working on (#8128) I started running into several issues with `GeneratePackageVersions` namely that I couldn't get `IncludePackages` to work and then I started fighting issues with the `nuget_cache` as well when attempting to add a new integration. This sets out to do a few things: - Simplify the logic for NuGet cooldown (automated dependency updates must adhere to our current cooldown policy for potential supply chain attacks) - Remove the `nuget_cache` - this was added so that we wouldn't go and query NuGet info for packages not listed in the `IncludePackages` - Change the "verbage" of the cooldown/baseline to be easier to read / follow - Do NOT read / rely upon `supported_versions` (this is what I previously used to determine what the highest version was currently allowed) - this file is planned to be changed/modified/removed by another team in the near future hence why we shouldn't use it. - I didn't account for different timezones in the publish state of the `nuget_cache` before so if someone ran it depending on their locale it would change. ## Implementation details - New `CooldownMode` `enum`: `Normal`, `BypassCooldown`, `Freeze` - Normal drops recent versions that are above the previous max - BypassCooldown accepts all in-range versions (no cooldown) (when `IncludePackages` is used is an example) - Freeze re-emits the previous run's output verbatim - `XUnitFileGenerator.LoadExistingVersions` reads the previous `.g.cs` and returns `Dictionary<IntegrationName, List<(Framework, Versions)>>`. `PackageGroup` loads this once and exposes `TryGetFrozenVersions` - I think we should probably still try and do something different here but this seems a bit more reasonable - Deleted `nuget_version_cache.json` and `NuGetVersionCache.cs` - No longer used/needed - Some readability improvements - `IsWithinCooldown` → `WasPublishedTooRecently` - `baseline` → `previousMaxVersions` - `ApplyCooldown` now uses two named local booleans (`publishedTooRecently`, `atOrBelowPreviousMax`) so the drop condition reads positively: "drop the version if it was published too recently **and** we haven't already shipped against it." ## Test coverage I ran it locally for a handful of cases I think it is better. ## Other details <!-- Fixes #{issue} --> I have ran through several instances with this and it _seems_ to be good, basically default runs, runs with configurable cooldowns, runs with `IncludePackages` and runs with `ExcludePackages`. All appeared as expected to me. <!-- ⚠️ Note: Where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. MergeQueue is NOT enabled in this repository. If you have write access to the repo, the PR has 1-2 approvals (see above), and all of the required checks have passed, you can use the Squash and Merge button to merge the PR. If you don't have write access, or you need help, reach out in the #apm-dotnet channel in Slack. --> --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
1 parent 3ee5618 commit 0f47fd3

7 files changed

Lines changed: 300 additions & 57357 deletions

File tree

tracer/build/_build/Build.Utilities.cs

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ partial class Build
5959
[Parameter("Only update package versions for packages with the following names")]
6060
readonly string[] IncludePackages;
6161

62-
[Parameter("Minimum age in days a NuGet package version must have been published before auto-including. Defaults to 2 days, or 0 when --IncludePackages is set")]
62+
[Parameter("Minimum age in days a NuGet package version must have been published before auto-including. Defaults to 2. Ignored for packages named in --IncludePackages, which always bypass the cooldown.")]
6363
readonly int? PackageVersionCooldownDays;
6464

6565
[LazyLocalExecutable(@"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\gacutil.exe")]
@@ -235,53 +235,56 @@ partial class Build
235235
var definitionsFile = BuildDirectory / FileNames.DefinitionsJson;
236236
var supportedVersionsPath = BuildDirectory / "supported_versions.json";
237237

238-
// Build the shouldQueryNuGet predicate from include/exclude filters
239-
Func<string, bool> shouldUpdatePackage = (IncludePackages, ExcludePackages) switch
238+
// Decides the cooldown treatment for each package by name. See CooldownMode for what each value does.
239+
var getCooldownMode = BuildCooldownModeSelector(IncludePackages, ExcludePackages);
240+
241+
// Dependabot re-uses the previous entry verbatim for Freeze; Skip and Normal both refresh.
242+
Func<string, bool> shouldUpdatePackage = name => getCooldownMode(name) is not CooldownMode.Freeze;
243+
244+
static Func<string, CooldownMode> BuildCooldownModeSelector(string[] includePackages, string[] excludePackages)
240245
{
241-
({ } include, _) => name => include.Contains(name, StringComparer.OrdinalIgnoreCase),
242-
(_, { } exclude) => name => !exclude.Contains(name, StringComparer.OrdinalIgnoreCase),
243-
_ => _ => true
244-
};
245-
246-
// Load caches for both pipelines
247-
var cacheFilePath = BuildDirectory / "nuget_version_cache.json";
248-
var previousVersionCache = await NuGetVersionCache.Load(cacheFilePath);
249-
Logger.Information("Loaded NuGet version cache with {Count} entries", previousVersionCache.Count);
246+
// No filter: every package goes through the normal cooldown filter.
247+
if (includePackages is null && excludePackages is null)
248+
{
249+
return _ => CooldownMode.Normal;
250+
}
251+
252+
// --IncludePackages Foo Bar: update only the listed packages (bypassing cooldown);
253+
// freeze every other package so it re-emits its previous output unchanged.
254+
if (includePackages is not null)
255+
{
256+
var targeted = new HashSet<string>(includePackages, StringComparer.OrdinalIgnoreCase);
257+
return name => targeted.Contains(name) ? CooldownMode.BypassCooldown : CooldownMode.Freeze;
258+
}
259+
260+
// --ExcludePackages Foo Bar: freeze the listed packages; everything else updates normally.
261+
var blocked = new HashSet<string>(excludePackages, StringComparer.OrdinalIgnoreCase);
262+
return name => blocked.Contains(name) ? CooldownMode.Freeze : CooldownMode.Normal;
263+
}
264+
265+
// supported_versions.json is still loaded for Pipeline B (dependabot + supported_versions.json
266+
// regeneration). The cooldown filter in Pipeline A sources its previous-max data from the
267+
// generated .g.cs files directly, because those retain per-major version history that
268+
// supported_versions.json's single-max-per-package shape cannot represent.
250269
var previousSupportedVersions = await GenerateSupportMatrix.LoadPreviousVersions(supportedVersionsPath);
251270
Logger.Information("Loaded previous supported versions with {Count} entries", previousSupportedVersions.Count);
252271

253-
// Derive baseline from supported_versions.json: the max tested version per package
254-
// acts as a floor to prevent cooldown filtering from downgrading previously accepted versions.
255-
// We collect all max tested versions per package (not just the global max) so that
256-
// split-range packages (e.g., GraphQL 4.x-6.x and 7.x-9.x) get a per-range baseline.
257-
var baseline = previousSupportedVersions
258-
.Where(kvp => kvp.Value.MaxVersionTestedInclusive is not null)
259-
.GroupBy(kvp => kvp.Key.PackageName)
260-
.ToDictionary(
261-
g => g.Key,
262-
g => g.Select(kvp => new Version(kvp.Value.MaxVersionTestedInclusive!)).ToList());
263-
Logger.Information("Derived version baseline with {Count} entries from supported_versions.json", baseline.Count);
264-
265-
// Resolve effective cooldown:
266-
// - Explicit --PackageVersionCooldownDays wins
267-
// - --IncludePackages without explicit cooldown defaults to 0
268-
var effectiveCooldownDays = PackageVersionCooldownDays ?? (IncludePackages is not null ? 0 : 2);
272+
var effectiveCooldownDays = PackageVersionCooldownDays ?? 2;
269273

270274
// Pipeline A: generate .g.props/.g.cs files
271275
Logger.Information("Using package version cooldown of {Days} days", effectiveCooldownDays);
272-
var versionGenerator = new PackageVersionGenerator(TracerDirectory, testDir, shouldUpdatePackage, previousVersionCache, effectiveCooldownDays, baseline);
276+
var versionGenerator = new PackageVersionGenerator(TracerDirectory, testDir, getCooldownMode, effectiveCooldownDays);
273277
var testedVersions = await versionGenerator.GenerateVersions(Solution);
274-
await NuGetVersionCache.Save(cacheFilePath, versionGenerator.VersionCache);
275278

276279
// Log version changes: bumps, unchanged, and overridden
277-
var versionCache = versionGenerator.VersionCache;
280+
var queriedVersions = versionGenerator.QueriedVersions;
278281
var bumped = 0;
279282
var unchanged = 0;
280283
foreach (var tested in testedVersions)
281284
{
282285
var packageName = tested.NugetPackageSearchName;
283-
baseline.TryGetValue(packageName, out var previousMaxVersions);
284-
var previousMax = previousMaxVersions?
286+
var previousMax = versionGenerator
287+
.GetPreviouslyTestedVersions(tested.IntegrationName)
285288
.Where(v => v >= tested.MinVersion && v <= tested.MaxVersion)
286289
.OrderByDescending(v => v)
287290
.FirstOrDefault();
@@ -290,12 +293,12 @@ partial class Build
290293
{
291294
bumped++;
292295
var publishedDate = "(unknown)";
293-
if (versionCache.TryGetValue(packageName, out var cachedVersions))
296+
if (queriedVersions.TryGetValue(packageName, out var versionsForPackage))
294297
{
295-
var match = cachedVersions.FirstOrDefault(v => v.Version == tested.MaxVersion.ToString());
298+
var match = versionsForPackage.FirstOrDefault(v => v.Version == tested.MaxVersion.ToString());
296299
if (match?.Published is not null)
297300
{
298-
publishedDate = match.Published.Value.ToString("yyyy-MM-dd");
301+
publishedDate = match.Published.Value.UtcDateTime.ToString("yyyy-MM-dd");
299302
}
300303
}
301304

@@ -325,13 +328,11 @@ partial class Build
325328

326329
foreach (var entry in versionGenerator.CooldownReport.Entries)
327330
{
328-
var resolvedText = entry.ResolvedVersion is not null ? $"using: {entry.ResolvedVersion}" : "skipped";
329331
Logger.Warning(
330-
" {Package} {Version} overridden (published {Date}, {Resolved})",
332+
" {Package} {Version} overridden (published {Date})",
331333
entry.PackageName,
332334
entry.OverriddenVersion,
333-
entry.PublishedDate?.ToString("yyyy-MM-dd") ?? "unknown",
334-
resolvedText);
335+
entry.PublishedDate?.UtcDateTime.ToString("yyyy-MM-dd") ?? "unknown");
335336
}
336337

337338
var reportPath = TemporaryDirectory / "cooldown_report.md";
@@ -347,7 +348,7 @@ partial class Build
347348
var integrations = GenerateIntegrationDefinitions.GetAllIntegrations(assemblies, definitionsFile);
348349

349350
// Pipeline B: generate dependabot files + supported_versions.json
350-
// TestedVersions are cooldown-filtered but the baseline prevents downgrades,
351+
// TestedVersions are cooldown-filtered but the previous max pins prevent downgrades,
351352
// so they accurately reflect what we're testing.
352353
var distinctIntegrations = await DependabotFileManager.BuildDistinctIntegrationMaps(
353354
integrations, testedVersions, shouldUpdatePackage, previousSupportedVersions);
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// <copyright file="CooldownMode.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
namespace GeneratePackageVersions;
7+
8+
/// <summary>
9+
/// Controls how cooldown filtering is applied to a package.
10+
/// </summary>
11+
public enum CooldownMode
12+
{
13+
/// <summary>
14+
/// Normal cooldown: versions published within the cooldown window are dropped, unless they
15+
/// are at or below the highest version we already tested in a prior run for this entry's range
16+
/// (so cooldown never downgrades a version we already shipped against).
17+
/// </summary>
18+
Normal,
19+
20+
/// <summary>
21+
/// Bypass the cooldown filter: every NuGet version in this entry's range is accepted,
22+
/// including those published within the cooldown window. Used for packages explicitly
23+
/// targeted via --IncludePackages.
24+
/// </summary>
25+
BypassCooldown,
26+
27+
/// <summary>
28+
/// Re-emit the previous run's versions verbatim. The generators are not re-run against NuGet;
29+
/// the previous .g.cs file is parsed and its versions are fed back through Write(). Used for
30+
/// non-targeted packages when --IncludePackages or --ExcludePackages filters are active.
31+
/// </summary>
32+
Freeze,
33+
}

tracer/build/_build/GeneratePackageVersions/CooldownReport.cs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public class CooldownReport
1919
{
2020
private readonly int _cooldownDays;
2121
private readonly List<CooldownEntry> _entries = new();
22-
private readonly HashSet<(string PackageName, string Version)> _seen = new();
2322

2423
public CooldownReport(int cooldownDays)
2524
{
@@ -32,12 +31,7 @@ public CooldownReport(int cooldownDays)
3231

3332
public void Add(CooldownEntry entry)
3433
{
35-
// Deduplicate: the same version gets flagged once per framework and per selection group,
36-
// but we only need to report it once.
37-
if (_seen.Add((entry.PackageName, entry.OverriddenVersion)))
38-
{
39-
_entries.Add(entry);
40-
}
34+
_entries.Add(entry);
4135
}
4236

4337
public string ToMarkdown()
@@ -53,18 +47,17 @@ public string ToMarkdown()
5347
sb.AppendLine($"The following versions were published less than **{_cooldownDays} days** ago and have been overridden.");
5448
sb.AppendLine("These require manual review before inclusion.");
5549
sb.AppendLine();
56-
sb.AppendLine("| Package | Integration | Overridden Version | Published | Age (days) | Using Instead |");
57-
sb.AppendLine("|---------|-------------|--------------------|-----------|------------|---------------|");
50+
sb.AppendLine("| Package | Integration | Overridden Version | Published | Age (days) |");
51+
sb.AppendLine("|---------|-------------|--------------------|-----------|------------|");
5852

5953
foreach (var entry in _entries)
6054
{
61-
var published = entry.PublishedDate?.ToString("yyyy-MM-dd") ?? "unknown";
55+
var published = entry.PublishedDate?.UtcDateTime.ToString("yyyy-MM-dd") ?? "unknown";
6256
var age = entry.PublishedDate.HasValue
6357
? ((int)(DateTimeOffset.UtcNow - entry.PublishedDate.Value).TotalDays).ToString()
6458
: "?";
65-
var usingInstead = entry.ResolvedVersion ?? "(skipped)";
6659

67-
sb.AppendLine($"| {entry.PackageName} | {entry.IntegrationName} | {entry.OverriddenVersion} | {published} | {age} | {usingInstead} |");
60+
sb.AppendLine($"| {entry.PackageName} | {entry.IntegrationName} | {entry.OverriddenVersion} | {published} | {age} |");
6861
}
6962

7063
return sb.ToString();
@@ -84,6 +77,5 @@ public record CooldownEntry(
8477
string PackageName,
8578
string IntegrationName,
8679
string OverriddenVersion,
87-
DateTimeOffset? PublishedDate,
88-
string ResolvedVersion);
80+
DateTimeOffset? PublishedDate);
8981
}

tracer/build/_build/GeneratePackageVersions/NuGetVersionCache.cs

Lines changed: 0 additions & 60 deletions
This file was deleted.

0 commit comments

Comments
 (0)