-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support daily channels for specific previews #54579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/dnup
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,22 +111,33 @@ public static bool IsValidChannelFormat(string channel) | |
| } | ||
|
|
||
| // The only two forms that include a '-' are: | ||
| // * "<partial-version>-daily" (e.g. "10.0-daily", "10.0.1xx-daily"). | ||
| // Daily only applies to partial versions; "10.0.103-daily" is rejected | ||
| // because a specific patch is already specific. | ||
| // * "<partial-version>-daily" (e.g. "10.0-daily", "10.0.1xx-daily"), | ||
| // optionally with a prerelease-label qualifier ("11.0.1xx-preview.5-daily" | ||
| // or "11.0.1xx-preview5-daily"). Daily only applies to scopes; a | ||
| // specific patch like "10.0.103-daily" is already specific and is | ||
| // rejected. | ||
| // * a fully-qualified version with a prerelease tag (e.g. "10.0.100-preview.1.32640"). | ||
| // The prerelease tag is opaque; we only validate the numeric prefix. | ||
| var dashIndex = channel.IndexOf('-', StringComparison.Ordinal); | ||
| if (dashIndex >= 0) | ||
| if (channel.EndsWith(DailySuffix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var versionPart = channel.Substring(0, dashIndex); | ||
| var suffix = channel.Substring(dashIndex); | ||
| var basePart = channel.Substring(0, channel.Length - DailySuffix.Length); | ||
| if (string.IsNullOrEmpty(basePart)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (suffix.Equals(DailySuffix, StringComparison.OrdinalIgnoreCase)) | ||
| if (UpdateChannel.TrySplitPartialVersionAndPrereleaseLabel(basePart, out var bandPart, out _)) | ||
| { | ||
| return !string.IsNullOrEmpty(versionPart) && IsValidPartialVersion(versionPart); | ||
| return IsValidPartialVersion(bandPart); | ||
| } | ||
|
|
||
| return IsValidPartialVersion(basePart); | ||
| } | ||
|
|
||
| var dashIndex = channel.IndexOf('-', StringComparison.Ordinal); | ||
| if (dashIndex >= 0) | ||
| { | ||
| var versionPart = channel.Substring(0, dashIndex); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we validate the version component after the dash, so it might try to install a version string such as '10.0.1xx-RANDOM` - but don't think this is new. It likely gets caught elsewhere in the code? Could be worth improving or adding a test for now. |
||
| return IsValidNumericVersion(versionPart); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,10 +87,43 @@ public DailyChannelResolver(ReleaseManifest? releaseManifest = null, HttpClient? | |
| } | ||
|
|
||
| // "<M>-daily" → use "<M>.0" as the aka.ms partial version (aka.ms paths use major.minor). | ||
| string partialVersion = NormalizePartialVersion(UpdateChannel.StripDailySuffix(channel.Name)); | ||
| // For prerelease-qualified daily channels ("<band>-preview.5-daily"): | ||
| // 1. Translate the label to aka.ms's dotless form ("preview5") so the URL has | ||
| // the shape aka.ms expects: ".../<band>-preview5/daily/...". | ||
| // 2. If the band is a bare major.minor (e.g. "11.0-preview.5-daily"), inject | ||
| // the default ".1xx" feature band. aka.ms only publishes prerelease-qualified | ||
| // daily shortlinks under a feature-band path, so "11.0-preview5" has no | ||
| // target; "11.0.1xx-preview5" is the canonical form aka.ms serves and it | ||
| // returns the right artifact for any component (SDK, runtime, aspnetcore, | ||
| // windowsdesktop). | ||
| string scope = UpdateChannel.StripDailySuffix(channel.Name); | ||
| string partialVersion; | ||
| if (UpdateChannel.TrySplitPartialVersionAndPrereleaseLabel(scope, out var bandPart, out var prereleaseLabel)) | ||
| { | ||
| string akaMsLabel = prereleaseLabel.Replace(".", string.Empty, StringComparison.Ordinal); | ||
| string normalizedBand = EnsureFeatureBand(NormalizePartialVersion(bandPart)); | ||
| partialVersion = $"{normalizedBand}-{akaMsLabel}"; | ||
| } | ||
| else | ||
| { | ||
| partialVersion = NormalizePartialVersion(scope); | ||
| } | ||
|
|
||
| return TryResolvePartialVersion(partialVersion, archivePrefix, rid, extension); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Ensures a partial version has a feature-band component. <c>"11.0"</c> → | ||
| /// <c>"11.0.1xx"</c>; <c>"11.0.2xx"</c> passes through unchanged. Used when the | ||
| /// aka.ms path requires a feature band (prerelease-qualified daily channels) but | ||
| /// the user supplied only major.minor. | ||
| /// </summary> | ||
| private static string EnsureFeatureBand(string partialVersion) | ||
| { | ||
| var parts = partialVersion.Split('.'); | ||
| return parts.Length == 2 ? $"{partialVersion}.1xx" : partialVersion; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the right assumption - e.g., |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts a channel's partial version into the form expected by aka.ms paths: | ||
| /// bare-major <c>10</c> becomes <c>10.0</c>; major.minor and feature-band | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,105 @@ public static string StripDailySuffix(string channelName) | |
| ? channelName.Substring(0, channelName.Length - DailySuffix.Length) | ||
| : channelName; | ||
|
|
||
| /// <summary> | ||
| /// Tries to split a daily-scope string into a partial-version prefix and a | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does daily-scope string mean a string that is a not fully specified version, or a fully specified version, but one without the daily suffix? |
||
| /// prerelease label. Accepts both <c>preview5</c> and <c>preview.5</c> | ||
| /// spellings of the label (e.g. <c>"11.0.1xx-preview.5"</c> → | ||
| /// <c>("11.0.1xx", "preview.5")</c>; <c>"11.0.1xx-preview5"</c> → | ||
| /// <c>("11.0.1xx", "preview.5")</c>). The returned <paramref name="prereleaseLabel"/> | ||
| /// is always normalized to <c>label.N</c> form (i.e. with the dot) so it can | ||
| /// be compared directly against a <see cref="ReleaseVersion.Prerelease"/>. | ||
| /// </summary> | ||
| internal static bool TrySplitPartialVersionAndPrereleaseLabel( | ||
| string scope, | ||
| out string partialVersion, | ||
| out string prereleaseLabel) | ||
| { | ||
| partialVersion = string.Empty; | ||
| prereleaseLabel = string.Empty; | ||
|
|
||
| int dashIndex = scope.IndexOf('-', StringComparison.Ordinal); | ||
| if (dashIndex <= 0 || dashIndex >= scope.Length - 1) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| string left = scope.Substring(0, dashIndex); | ||
| string right = scope.Substring(dashIndex + 1); | ||
|
|
||
| if (!TryNormalizePrereleaseLabel(right, out string normalized)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| partialVersion = left; | ||
| prereleaseLabel = normalized; | ||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Normalizes a prerelease label to <c>name.N</c> form (e.g. | ||
| /// <c>"preview5"</c> and <c>"preview.5"</c> both produce <c>"preview.5"</c>). | ||
| /// Returns <c>false</c> if the input doesn't match a <c>{letters}{digits}</c> | ||
| /// or <c>{letters}.{digits}</c> shape. | ||
| /// </summary> | ||
| internal static bool TryNormalizePrereleaseLabel(string label, out string normalized) | ||
| { | ||
| normalized = string.Empty; | ||
| if (string.IsNullOrEmpty(label)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| string name; | ||
| string number; | ||
| int dotIndex = label.IndexOf('.', StringComparison.Ordinal); | ||
| if (dotIndex >= 0) | ||
| { | ||
| name = label.Substring(0, dotIndex); | ||
| number = label.Substring(dotIndex + 1); | ||
| } | ||
| else | ||
| { | ||
| // No dot: find the boundary between the alpha name and the digit run. | ||
| int boundary = 0; | ||
| while (boundary < label.Length && char.IsLetter(label[boundary])) | ||
| { | ||
| boundary++; | ||
| } | ||
| if (boundary == 0 || boundary == label.Length) | ||
| { | ||
| return false; | ||
| } | ||
| name = label.Substring(0, boundary); | ||
| number = label.Substring(boundary); | ||
| } | ||
|
|
||
| if (name.Length == 0 || number.Length == 0) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| foreach (char c in name) | ||
| { | ||
| if (!char.IsLetter(c)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| foreach (char c in number) | ||
| { | ||
| if (!char.IsDigit(c)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| normalized = $"{name.ToLowerInvariant()}.{number}"; | ||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks if the channel string looks like an SDK version or feature band pattern rather than a runtime version. | ||
| /// SDK versions have a third component >= 100 (e.g., "9.0.103", "9.0.304") or use "xx" patterns (e.g., "9.0.1xx"). | ||
|
|
@@ -112,19 +211,36 @@ public bool Matches(ReleaseVersion version) | |
| return version.Major == major; | ||
| } | ||
|
|
||
| // Scoped daily channels (e.g. "10.0-daily", "10.0.1xx-daily") match the | ||
| // same versions their base scope would, but restricted to prerelease | ||
| // versions. A stable release is not a daily build, even if its version | ||
| // falls inside the base scope; rejecting it here keeps the channel's | ||
| // "what satisfies me?" answer coherent for GC and reporting. | ||
| // Scoped daily channels (e.g. "10.0-daily", "10.0.1xx-daily", | ||
| // "11.0.1xx-preview.5-daily") match the same versions their base scope | ||
| // would, but restricted to prerelease versions. A stable release is not | ||
| // a daily build, even if its version falls inside the base scope; | ||
| // rejecting it here keeps the channel's "what satisfies me?" answer | ||
| // coherent for GC and reporting. | ||
| if (IsDaily) | ||
| { | ||
| if (IsStableRelease(version)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return new UpdateChannel(StripDailySuffix(Name)).Matches(version); | ||
| string scope = StripDailySuffix(Name); | ||
|
|
||
| // Prerelease-qualified daily channels ("<band>-preview.5-daily") match only | ||
| // versions whose prerelease starts with the requested label, in addition to the | ||
| // base scope matching. | ||
| if (TrySplitPartialVersionAndPrereleaseLabel(scope, out string partialVersion, out string prereleaseLabel)) | ||
| { | ||
| if (!new UpdateChannel(partialVersion).Matches(version)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return version.Prerelease.Equals(prereleaseLabel, StringComparison.OrdinalIgnoreCase) | ||
| || version.Prerelease.StartsWith(prereleaseLabel + ".", StringComparison.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| return new UpdateChannel(scope).Matches(version); | ||
| } | ||
|
|
||
| return MatchesMajorMinorOrFeatureBand(version); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this doing validation on
baseaPartor should we also be checking that? I think thatIsValidPartialVersionno longer executes on anything besides the band part, so it will pass as long as there is a - and valid version afterward.Suggestion 2:
add a unit test for input such as
invalid.invalid.1xx-dailyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also recognize this function appears to be a 'best-effort' implementation