Dotnetup: Add support for update, uninstall, and garbage collection#53290
Dotnetup: Add support for update, uninstall, and garbage collection#53290dsplaisted wants to merge 20 commits intodotnet:release/dnupfrom
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Migrate the dotnetup shared manifest from a flat List<DotnetInstall> to a structured format with schema versioning, dotnet roots, install specs, and installations with subcomponents. New types: - DotnetupManifestData: top-level with schemaVersion and dotnetRoots - DotnetRootEntry: groups install specs and installations per root - InstallSpec: tracks what user requested (component, channel, source) - Installation: tracks what's installed (component, version, subcomponents) - InstallSource enum: Explicit, GlobalJson, Previous Key changes: - DotnetupSharedManifest reads/writes new format with legacy migration - IDotnetupManifest extended with install spec and installation APIs - InstallerOrchestratorSingleton records both specs and installations - 8 new unit tests covering serialization, migration, and CRUD ops Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement the remaining pieces of the installation tracking design: Subcomponent tracking: - SubcomponentResolver maps archive entry paths to subcomponent IDs based on known folder depths (sdk=2, shared=3, host=3, packs=3, templates=2, sdk-manifests=4) - DotnetArchiveExtractor now collects subcomponent IDs during extraction - InstallerOrchestratorSingleton populates Installation.Subcomponents Garbage collection: - GarbageCollector class refreshes global.json specs, resolves latest matching installation per spec, removes unmarked installations, and deletes orphaned subcomponent folders from disk Update command: - SdkUpdateCommand resolves latest version per install spec, installs if needed, then runs GC to clean old versions Uninstall command: - SdkUninstallCommand removes matching install spec, runs GC, warns if installation still referenced by other specs Pre-existing root detection: - PreexistingRootDetector scans dotnet root for existing SDK/runtime folders and creates Previous-sourced install specs on first use Tests: 30 new tests covering SubcomponentResolver, GarbageCollector, and PreexistingRootDetector. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
By default, the uninstall command only removes install specs with InstallSource.Explicit. A new --source option accepts explicit (default), previous, globaljson, or all to target other install sources. When no explicit spec matches but specs with other sources exist, a helpful message directs the user to use the --source option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…l.json resolver, list improvements - Create shared UpdateWorkflow and UninstallWorkflow (#4A) - Refactor SdkUpdateCommand and SdkUninstallCommand to thin wrappers (#4B) - Fix uninstall error message to use actual source filter (dotnet#2) - Fix GC deletion assumption with targeted installation snapshot (dotnet#3) - Include channel/spec in update status messages (dotnet#8) - Create RuntimeUpdateCommand and RuntimeUninstallCommand (#4C) - Wire runtime update/uninstall in parsers (#4D) - Implement GlobalJsonChannelResolver and use in GC RefreshGlobalJsonSpecs (dotnet#16) - List command now shows both install specs and installed versions (dotnet#12) - Fix SourceOption naming/syntax for System.CommandLine compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changes include: - Rename 'managed installations' to 'tracked installations' (dotnet#10) - Fix '.metadata' -> 'metadata' folder name (dotnet#18) - Include component type in error/warning messages (dotnet#11) - Remove component spec from runtime update (dotnet#12) - Remove GetInstallations wrapper (dotnet#6) - ScopedMutex throws TimeoutException on failure (dotnet#22) - Make InstallResult.Install non-nullable (dotnet#17) - Remove ComponentFilesExist shortcut (dotnet#19) - Wire up InstallSource enum (dotnet#21) - ListCommand: enum types, narrowed mutex, read-only verify, spacing (dotnet#3,dotnet#4,dotnet#5,#1) - Show global.json path in list output (dotnet#15) - Add doc comments to ListData types (dotnet#7) - Move SourceOption to CommonOptions, remove alias fields (dotnet#9) - Remove ResolveChannelFromGlobalJson test hook wrapper (dotnet#14) - Root 'dotnetup update' updates all components (dotnet#16) - GlobalJsonChannelResolver: support rollForward policy (dotnet#13) - Error when installing to unmanaged dotnet root (dotnet#20) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changes include: - UpdateWorkflow.Execute returns non-zero exit code on failure (#1) - InfoCommand JSON output includes InstallSpecs (dotnet#2) - Wire --update-global-json through SdkUpdateCommand to UpdateWorkflow (dotnet#3) - Implement DotnetInstallManager.UpdateGlobalJson using Utf8JsonReader (dotnet#3) - Fix ResolveEntryDestPath to use normalized entry name (dotnet#4) - Fix test: MajorMinor test uses correct channel '9.0' (dotnet#5) - Fix test: LTS assertion checks Major instead of Minor (dotnet#6) - Fix ScopedMutex.Dispose to not leak mutex on ReleaseMutex throw (dotnet#7) - Make CommonOptions fields static readonly (dotnet#9) - Remove unused --no-progress from uninstall parsers (dotnet#10) - GarbageCollector catches Exception instead of just IOException (dotnet#11) - Capitalize 'SDK' in SdkCommandParser description (dotnet#12) - Eliminate redundant manifest reads in InstallerOrchestratorSingleton (dotnet#13) - Fix STS test error message to say 'major' not 'minor' (dotnet#14) - Make DotnetupUtilities.ExeSuffix readonly (dotnet#16) - Fix FormatBytes to use floating-point with one decimal (dotnet#17) - Remove dead ReleaseManifest variable in DotnetInstaller (dotnet#18) - Make UpdateChannel.Name immutable (dotnet#19) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changes: - Make GlobalJsonContents.Sdk and SdkSection public (fixes source-gen deserialization) - Add path traversal (zip slip) check in archive extraction - Add symlink and hard link support in tar extraction - Fix IsAdminInstallPath to require directory boundary match - Use MarkupLineInterpolated to prevent Spectre.Console markup injection - Fix DownloadProgress divide-by-zero when TotalBytes is 0 - Use case-sensitive path comparison on Linux in PathsEqual - Normalize paths in IsRootInManifest with Path.GetFullPath - Add clarifying comments for UpdateChannel.Matches broad matching - Make SpectreProgressTarget.Dispose idempotent with TrySetResult - Report actual file size for cached downloads instead of 100/100 - Scope anyUpdated/anyFailed per-root in UpdateWorkflow - Fix RegistryKey leak from OpenBaseKey in WindowsPathHelper - Cache ChannelVersionResolver in DotnetReleaseInfoProvider - Filter null versions before sorting in GarbageCollector - Normalize tar directory entries with ResolveEntryDestPath - Filter WindowsDesktop component on non-Windows in RuntimeUpdateCommand - Handle @ at position 0 in RuntimeInstallCommand.ParseComponentSpec - Remove unused allowPrerelease/rollForward params from UpdateGlobalJson - Add null guard in DotnetArchiveExtractor.Commit for _archivePath - Write manifest before deleting directories in GarbageCollector Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add install spec even when version is already installed via another channel - Show message when uninstall removes no files due to shared subcomponents - Fix runtime install incorrectly tagged as GlobalJson source - Fix broken Spectre.Console markup in uninstall source mismatch message - Explicit version/channel now takes priority over global.json in install workflow - Only set GlobalJsonPath on install request when source is actually GlobalJson Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends dotnetup to support update/uninstall flows and introduces manifest-driven garbage collection (GC) to remove unreferenced installed subcomponent folders. It refactors installation tracking from a legacy flat list into a structured manifest containing install specs (what the user requested) and installations (what’s actually installed + subcomponents).
Changes:
- Introduces a new dotnetup shared manifest schema (
DotnetupManifestData) withInstallSpec+Installationrecords and JSON source-gen updates. - Adds
update/uninstallcommand paths and shared workflows (UpdateWorkflow,UninstallWorkflow) that operate over install specs and run GC. - Adds GC + archive extraction subcomponent tracking (
GarbageCollector,SubcomponentResolver, extractor changes) and updates tests/docs accordingly.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnetup.Tests/SubcomponentResolverTests.cs | Unit tests for subcomponent path resolution. |
| test/dotnetup.Tests/SdkUninstallCommandTests.cs | Tests uninstall source filtering behavior. |
| test/dotnetup.Tests/RuntimeInstallTests.cs | Updates manifest tracking assertions for new schema. |
| test/dotnetup.Tests/ManifestTests.cs | Tests manifest schema, roundtrip, and operations. |
| test/dotnetup.Tests/ListCommandTests.cs | Updates list tests for new ListData output. |
| test/dotnetup.Tests/LibraryTests.cs | Normalizes file header/BOM change. |
| test/dotnetup.Tests/GarbageCollectorTests.cs | Tests GC behavior against manifest + filesystem. |
| test/dotnetup.Tests/DnupE2Etest.cs | Updates E2E manifest assertions to new format. |
| test/dotnetup.Tests/ChannelVersionResolverTests.cs | Fixes channel parsing expectations and assertions. |
| src/Installer/dotnetup/WindowsPathHelper.cs | Minor registry key handling refactor. |
| src/Installer/dotnetup/SpectreProgressTarget.cs | Makes progress completion idempotent. |
| src/Installer/dotnetup/Parser.cs | Registers new SDK uninstall command parser. |
| src/Installer/dotnetup/InstallerOrchestratorSingleton.cs | Records install specs/installations; adds root safety guard. |
| src/Installer/dotnetup/IDotnetupManifest.cs | Replaces legacy installed-version APIs with spec/installation APIs. |
| src/Installer/dotnetup/IDotnetInstallManager.cs | Simplifies UpdateGlobalJson signature. |
| src/Installer/dotnetup/GlobalJsonContents.cs | Makes SDK section public for JSON deserialization. |
| src/Installer/dotnetup/GlobalJsonChannelResolver.cs | Resolves channel from global.json + rollForward policy. |
| src/Installer/dotnetup/GarbageCollector.cs | GC implementation: prune manifest + delete orphan dirs. |
| src/Installer/dotnetup/DotnetupSharedManifest.cs | New manifest read/write + CRUD for specs/installations. |
| src/Installer/dotnetup/DotnetupManifestJsonContext.cs | Updates JSON source-gen types/options for new schema. |
| src/Installer/dotnetup/DotnetupManifestData.cs | Defines schema types: roots/specs/installations/sources. |
| src/Installer/dotnetup/DotnetInstallManager.cs | Implements global.json SDK version replacement logic. |
| src/Installer/dotnetup/Constants.cs | Renames global mutex identifier. |
| src/Installer/dotnetup/CommonOptions.cs | Makes options readonly; adds --source option. |
| src/Installer/dotnetup/Commands/Shared/UpdateWorkflow.cs | Shared update flow across specs + optional global.json update. |
| src/Installer/dotnetup/Commands/Shared/UninstallWorkflow.cs | Shared uninstall flow + GC + reporting. |
| src/Installer/dotnetup/Commands/Shared/InstallWorkflow.cs | Plumbs install source info into install requests. |
| src/Installer/dotnetup/Commands/Shared/InstallWalkthrough.cs | Ensures explicit channel overrides global.json channel. |
| src/Installer/dotnetup/Commands/Shared/InstallExecutor.cs | Switches to exception-based failure handling; improves path checks. |
| src/Installer/dotnetup/Commands/Sdk/Update/SdkUpdateCommandParser.cs | Adds root update variant + options wiring. |
| src/Installer/dotnetup/Commands/Sdk/Update/SdkUpdateCommand.cs | Implements sdk update using shared workflow. |
| src/Installer/dotnetup/Commands/Sdk/Uninstall/SdkUninstallCommandParser.cs | Adds sdk uninstall command parser. |
| src/Installer/dotnetup/Commands/Sdk/Uninstall/SdkUninstallCommand.cs | Implements sdk uninstall using shared workflow. |
| src/Installer/dotnetup/Commands/Sdk/SdkCommandParser.cs | Wires sdk uninstall into sdk command group. |
| src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommandParser.cs | Reuses common options instead of local aliases. |
| src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs | Uses GlobalJsonChannelResolver for channel derivation. |
| src/Installer/dotnetup/Commands/Runtime/Update/RuntimeUpdateCommandParser.cs | Adds runtime update command parser. |
| src/Installer/dotnetup/Commands/Runtime/Update/RuntimeUpdateCommand.cs | Updates runtime components via shared workflow. |
| src/Installer/dotnetup/Commands/Runtime/Uninstall/RuntimeUninstallCommandParser.cs | Adds runtime uninstall command parser. |
| src/Installer/dotnetup/Commands/Runtime/Uninstall/RuntimeUninstallCommand.cs | Implements runtime uninstall using shared workflow. |
| src/Installer/dotnetup/Commands/Runtime/RuntimeCommandParser.cs | Wires runtime update/uninstall into runtime group. |
| src/Installer/dotnetup/Commands/Runtime/Install/RuntimeInstallCommand.cs | Validates @ component spec edge case. |
| src/Installer/dotnetup/Commands/List/ListCommand.cs | Changes list output to include specs + validity metadata. |
| src/Installer/dotnetup/Commands/Info/InfoCommand.cs | Extends info JSON to include install specs too. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/UpdateChannel.cs | Adds Matches() for channel-to-version matching. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/SubcomponentResolver.cs | Adds subcomponent classification and depth mapping. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/ScopedMutex.cs | Makes mutex re-entrant and throws on timeout. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DownloadProgressReporter.cs | Improves human-readable byte formatting precision. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetupUtilities.cs | Makes exe suffix readonly; fixes Unix path equality semantics. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetReleaseInfoProvider.cs | Reuses resolver instance instead of recreating it. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetInstaller.cs | Removes unused manifest allocation. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetInstall.cs | Adds resolved version + install source/global.json path options. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetArchiveExtractor.cs | Tracks extracted subcomponents; strengthens extraction safety. |
| src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetArchiveDownloader.cs | Reports cached download progress using bytes. |
| src/Installer/Microsoft.Dotnet.Installation/DownloadProgress.cs | Fixes percent calc when total bytes is 0. |
| documentation/general/dotnetup/installation-tracking.md | Documents intended tracking/GC approach and schema. |
| .vscode/mcp.json | Renames MCP server key. |
You can also share your feedback on Copilot code review. Take the survey.
| public void ResolvesSkdPaths(string input, string expected) | ||
| { |
There was a problem hiding this comment.
The test name ResolvesSkdPaths looks like a typo ("Skd" vs "Sdk"). Renaming it to ResolvesSdkPaths would improve readability/searchability.
| ```json | ||
| { | ||
| schemaVersion: "1", | ||
| dotnetRoots: | ||
| [ | ||
| { | ||
| path: "C:\\Users\\Daniel\\AppData\\Local\\dotnet", | ||
| architecture: "x64", | ||
| installSpecs: | ||
| [ | ||
| { | ||
| "component": "sdk", | ||
| "versionOrChannel": "10", | ||
| "installSource": "explicit" | ||
| }, | ||
| { | ||
| "component": "runtime", | ||
| "versionOrChannel": "9", | ||
| "installSource": "explicit" | ||
| } | ||
| ], | ||
| installations: | ||
| [ | ||
| { | ||
| "component": "sdk", | ||
| "version": "10.0.103", | ||
| "subcomponents": | ||
| [ | ||
| "sdk/10.0.103", | ||
| "shared/Microsoft.AspNetCore.App/10.0.3", | ||
| "shared/Microsoft.NETCore.App/10.0.3", | ||
| "shared/Microsoft.WindowsDesktop.App/10.0.3", | ||
| "etc." | ||
| ] | ||
| }, | ||
| { | ||
| "component": "runtime", | ||
| "version": "9.0.12", | ||
| "subcomponents": | ||
| [ | ||
| "shared/Microsoft.NETCore.App/9.0.12", | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
The "Sample manifest" code block is labeled as json, but the sample isn't valid JSON (e.g., unquoted property names like schemaVersion, missing quotes around dotnetRoots, and a trailing comma in the subcomponents array). Either make the example valid JSON or label it as pseudo-code to avoid copy/paste confusion.
| private static List<string> DeleteOrphanedSubcomponents(string dotnetRootPath, HashSet<string> referencedSubcomponents) | ||
| { | ||
| var deleted = new List<string>(); | ||
|
|
||
| foreach (var topLevelDir in Directory.GetDirectories(dotnetRootPath)) | ||
| { |
There was a problem hiding this comment.
DeleteOrphanedSubcomponents calls Directory.GetDirectories(dotnetRootPath) without checking that dotnetRootPath exists. If the tracked install root has been deleted/moved, GC will throw and fail the update/uninstall flow. Consider an early Directory.Exists(dotnetRootPath) check and treat a missing root as "nothing to delete" (or emit a warning).
| private static readonly HashSet<string> s_ignoredFolders = | ||
| [ | ||
| with(StringComparer.OrdinalIgnoreCase), | ||
| "metadata", | ||
| "swidtag", | ||
| ]; |
There was a problem hiding this comment.
s_ignoredFolders is initialized with an invalid collection expression ([ with(StringComparer.OrdinalIgnoreCase), ... ]). This won't compile; initialize the HashSet<string> with the comparer via a normal constructor (e.g., new HashSet<string>(StringComparer.OrdinalIgnoreCase) with elements).
| private static readonly HashSet<string> s_ignoredFolders = | |
| [ | |
| with(StringComparer.OrdinalIgnoreCase), | |
| "metadata", | |
| "swidtag", | |
| ]; | |
| private static readonly HashSet<string> s_ignoredFolders = new(StringComparer.OrdinalIgnoreCase) | |
| { | |
| "metadata", | |
| "swidtag", | |
| }; |
| private string? _archivePath; | ||
| private IProgressReporter? _progressReporter; | ||
| private readonly HashSet<string> _extractedSubcomponents = [with(StringComparer.Ordinal)]; | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of subcomponent identifiers that were extracted during the last Commit() call. | ||
| /// </summary> | ||
| public IReadOnlyList<string> ExtractedSubcomponents => _extractedSubcomponents.ToList(); |
There was a problem hiding this comment.
_extractedSubcomponents is initialized with an invalid collection expression ([with(StringComparer.Ordinal)]), which will not compile. Use an explicit HashSet<string> constructor that supplies the comparer.
| string destPath = Path.GetFullPath(Path.Combine(targetDir, normalizedName)); | ||
| string fullTargetDir = Path.GetFullPath(targetDir) + Path.DirectorySeparatorChar; | ||
| if (!destPath.StartsWith(fullTargetDir, StringComparison.Ordinal) && | ||
| !string.Equals(destPath, Path.GetFullPath(targetDir), StringComparison.Ordinal)) | ||
| { |
There was a problem hiding this comment.
ResolveEntryDestPath uses StringComparison.Ordinal when checking that an extracted entry stays under targetDir. On Windows, path comparisons should be case-insensitive; otherwise a valid entry can be rejected if GetFullPath returns different casing (or the targetDir casing differs). Consider using OrdinalIgnoreCase on Windows (and Ordinal elsewhere).
| var installRequest = new DotnetInstallRequest( | ||
| installRoot, | ||
| channel, | ||
| spec.Component, | ||
| new InstallRequestOptions { ManifestPath = manifestPath }) | ||
| { | ||
| ResolvedVersion = latestVersion | ||
| }; |
There was a problem hiding this comment.
When creating the DotnetInstallRequest for an update, InstallRequestOptions doesn't propagate spec.InstallSource/spec.GlobalJsonPath. Since the orchestrator records an install spec from the request options, this can incorrectly add a duplicate Explicit spec for a channel that originally came from global.json. Pass through the spec's source (and global.json path when applicable) in InstallRequestOptions.
There was a problem hiding this comment.
We should add a test for this scenario. As well as for when we install something that exists not from global json but the same channel, and then make sure it actually does the update but also does not add a duplicate entry.
| var targetedInstallations = root.Installations | ||
| .Where(i => i.Component == componentFilter && | ||
| matchingSpecs.Any(s => new UpdateChannel(s.VersionOrChannel).Matches( | ||
| new Microsoft.Deployment.DotNet.Releases.ReleaseVersion(i.Version)))) | ||
| .Select(i => (i.Component, i.Version)) |
There was a problem hiding this comment.
targetedInstallations creates new ReleaseVersion(i.Version) for each installation. If the manifest contains an invalid version string (e.g., manual edit/corruption), this will throw and crash uninstall. Consider using ReleaseVersion.TryParse and treating invalid entries as manifest corruption or skipping them with a warning.
There was a problem hiding this comment.
Yes, we should verify the version input using some of the existing implementations and throw (user error ?) if it is invalid. I have another PR which tries to detect manual edits by putting a sha file side by side which is a hash over the manifest so we can detect those as user errors and the others as product errors if we edited it and it was wrong still (assumes user wont generate a new sha)
Add a new --untracked flag to sdk install and runtime install commands that installs .NET without recording in the dotnetup manifest. When set: - Bypasses the untracked-installation guard (no error for pre-existing .NET artifacts not tracked by the manifest) - Skips AddInstallSpec() and AddInstallation() manifest writes - Still performs the actual download, extraction, and validation This enables scenarios where users want dotnetup to install .NET to a directory without taking ownership of it in the manifest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shared frameworks installed by restore-toolset are for testing only and should not be tracked in the dotnetup manifest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nagilson
left a comment
There was a problem hiding this comment.
Very excited to see this working! Please see my feedback which for now covers up until src/Installer/dotnetup/InstallerOrchestratorSingleton.cs
|
|
||
| A subcomponent can be identified by a relative path to a folder from the dotnet root. The depth of the folder depends on the top-level subfolder under the dotnet root. For example: | ||
|
|
||
| - `sdk/10.0.102` - 2 levels |
There was a problem hiding this comment.
Note, I wonder if we've ever changed this structure? I looked through some older versions and the structure seemed to always be the same up to 2.1 at least, though. I did not comprehensively verify every folder layout.
|
|
||
| ```json | ||
| { | ||
| schemaVersion: "1", |
There was a problem hiding this comment.
nit, this sample manifest is actually invalid json as it is missing ""s
| foreach (var rootPath in allRoots) | ||
| { | ||
| writer.WriteLine($" {group.Key}"); | ||
| writer.WriteLine($" {rootPath}"); |
There was a problem hiding this comment.
nit, should we try to use 'pad' here instead of hardcoding the whitespace?
| specGrid.AddRow( | ||
| spec.Component.GetDisplayName(), | ||
| spec.VersionOrChannel, | ||
| $"[dim](source: {sourceDisplay})[/]" |
| int atIndex = spec.IndexOf('@'); | ||
| if (atIndex == 0) | ||
| { | ||
| return (default, null, $"Error: Invalid component specification '{spec}'. Component name is required before '@'."); |
There was a problem hiding this comment.
Since this isn't wrapped in a special exception class, will this get counted as a user error or a product error?
| Spectre.Console.AnsiConsole.MarkupLineInterpolated(CultureInfo.InvariantCulture, $"[green]Installed .NET SDK {installResult.Install.Version}, available via {installResult.Install.InstallRoot}[/]"); | ||
| } | ||
|
|
||
| public void UpdateGlobalJson(string globalJsonPath, string? sdkVersion = null) |
There was a problem hiding this comment.
Do we need to do this since the roll forward value already tells us what to install? Is there any benefit to doing this?
| public InstallComponent Component { get; set; } | ||
| public string VersionOrChannel { get; set; } = string.Empty; | ||
| public InstallSource InstallSource { get; set; } | ||
| public string? GlobalJsonPath { get; set; } |
There was a problem hiding this comment.
Should we include a Timestamp which represents when the global.json file was last modified, e.g. "timestamp": "2025-04-15T14:30:22.1234567Z", so we don't need to read the entire file if there are no changes? In general we don't expect the file to change that often. Then we know what the channel is already from the install spec.
|
|
||
| /// <summary> | ||
| /// Shared utility for resolving the SDK channel (feature band) from a global.json file. | ||
| /// Takes into account the rollForward and allowPrerelease properties. |
There was a problem hiding this comment.
I dont think this takes allowPrerelease into account. This should be tested.
| "latestminor" => string.Create(CultureInfo.InvariantCulture, $"{version.Major}"), | ||
| "latestmajor" => "latest", | ||
| // Default (null or "latestPatch") — feature band channel | ||
| _ => DeriveFeatureBandChannel(version), |
There was a problem hiding this comment.
should allowPrerelease imply a 'preview' channel?
| /// </summary> | ||
| internal static string DeriveFeatureBandChannel(ReleaseVersion version) | ||
| { | ||
| int featureBand = version.Patch / 100; |
There was a problem hiding this comment.
we should test the boundaries of this function, e.g. 99 vs 0, 100, 101
|
Is there a place to look where I can see why garbage collection is needed? Dnvm uses channels and installs. You have to manage those manually. Is this because of workloads? And how does it relate to the workloads fix that the ask already has? |
No description provided.