Add SDK version utilities and GlobalJson parsing #2550
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Licensed to the .NET Foundation under one or more agreements. | ||
| * The .NET Foundation licenses this file to you under the MIT license. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { RollForwardPolicy } from './VersionUtilities'; | ||
|
|
||
| /** | ||
| * Represents the structure of a global.json file. | ||
| * Only includes officially supported properties per: | ||
| * https://learn.microsoft.com/dotnet/core/tools/global-json | ||
| */ | ||
| export interface GlobalJson | ||
| { | ||
| sdk?: GlobalJsonSdk; | ||
| 'msbuild-sdks'?: Record<string, string>; | ||
| } | ||
|
|
||
| /** | ||
| * The SDK section of a global.json file. | ||
| */ | ||
| export interface GlobalJsonSdk | ||
| { | ||
| /** | ||
| * The SDK version to use. | ||
| * Can be a specific version (e.g., "8.0.308") or a feature band pattern (e.g., "8.0.3xx"). | ||
| */ | ||
| version?: string; | ||
|
|
||
| /** | ||
| * Whether to allow prerelease SDK versions. | ||
| * Defaults to false when version is specified, true otherwise. | ||
| */ | ||
| allowPrerelease?: boolean; | ||
|
|
||
| /** | ||
| * The roll-forward policy for SDK version selection. | ||
| * Defaults to 'latestPatch' when version is specified, 'latestMajor' otherwise. | ||
| */ | ||
| rollForward?: RollForwardPolicy; | ||
|
|
||
| /** | ||
| * Additional paths to search for SDKs (.NET 10+). | ||
| */ | ||
| paths?: string[]; | ||
|
|
||
| /** | ||
| * Custom error message when SDK requirements cannot be satisfied (.NET 10+). | ||
| */ | ||
| errorMessage?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Requirements extracted from a global.json file, with the file path included. | ||
| */ | ||
| export interface GlobalJsonRequirements | ||
| { | ||
| /** Path to the global.json file */ | ||
| filePath: string; | ||
| /** SDK version required */ | ||
| sdkVersion?: string; | ||
| /** Whether prerelease SDKs are allowed */ | ||
| allowPrerelease?: boolean; | ||
| /** Roll-forward policy */ | ||
| rollForward?: RollForwardPolicy; | ||
| } | ||
|
|
||
| /** | ||
| * Parses a global.json content string and returns its contents. | ||
| * @param content The JSON content of the global.json file. | ||
| * @returns Parsed global.json contents, or undefined if the content is invalid. | ||
| */ | ||
| export function parseGlobalJsonContent(content: string): GlobalJson | undefined | ||
| { | ||
| try | ||
| { | ||
| return JSON.parse(content) as GlobalJson; | ||
| } | ||
| catch | ||
| { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Extracts SDK requirements from a parsed global.json object. | ||
| * @param globalJson The parsed global.json object. | ||
| * @param filePath The path to the global.json file (for reference in the result). | ||
| * @returns Extracted requirements. | ||
| */ | ||
| export function getRequirementsFromGlobalJson(globalJson: GlobalJson, filePath: string): GlobalJsonRequirements | ||
| { | ||
| return { | ||
| filePath, | ||
| sdkVersion: globalJson.sdk?.version, | ||
| allowPrerelease: globalJson.sdk?.allowPrerelease, | ||
| rollForward: globalJson.sdk?.rollForward, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Parses global.json content and extracts SDK requirements in one step. | ||
| * @param content The JSON content of the global.json file. | ||
| * @param filePath The path to the global.json file (for reference in the result). | ||
| * @returns Extracted requirements, or undefined if the content is invalid. | ||
| */ | ||
| export function parseGlobalJsonRequirements(content: string, filePath: string): GlobalJsonRequirements | undefined | ||
| { | ||
| const globalJson = parseGlobalJsonContent(content); | ||
| if (!globalJson) | ||
| { | ||
| return undefined; | ||
| } | ||
| return getRequirementsFromGlobalJson(globalJson, filePath); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the effective roll-forward policy from global.json requirements. | ||
| * If not specified, returns the default policy based on whether a version is specified. | ||
| * @param requirements The global.json requirements. | ||
| * @returns The effective roll-forward policy. | ||
| */ | ||
| export function getEffectiveRollForward(requirements: GlobalJsonRequirements): RollForwardPolicy | ||
| { | ||
| if (requirements.rollForward) | ||
| { | ||
| return requirements.rollForward; | ||
| } | ||
| // Default is 'latestPatch' when version is specified, 'latestMajor' otherwise | ||
| return requirements.sdkVersion ? 'latestPatch' : 'latestMajor'; | ||
|
Member
There was a problem hiding this comment. Should we support more than latestPatch and latestMajor?
Member
There was a problem hiding this comment. I'm wondering if we should store these as static constant strings instead of having them coded here. |
||
| } | ||
|
|
||
| /** | ||
| * Gets the effective allowPrerelease setting from global.json requirements. | ||
| * If not specified, returns the default based on whether a version is specified. | ||
| * @param requirements The global.json requirements. | ||
| * @returns The effective allowPrerelease setting. | ||
| */ | ||
| export function getEffectiveAllowPrerelease(requirements: GlobalJsonRequirements): boolean | ||
| { | ||
| if (requirements.allowPrerelease !== undefined) | ||
| { | ||
| return requirements.allowPrerelease; | ||
| } | ||
| // Default is false when version is specified, true otherwise | ||
| return !requirements.sdkVersion; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The most important feedback I have here:
I think it's wise for this logic to live here, and we shouldn't reimplement what's done in
recommendedVersionandlistVersions. Although I think the overhead of the vscode command calls is quite high. The fact that the codebase parses versions as strings instead of a type is definitely a regret and hard to change from the initial design, which is why we haven't replaced it with something like semver. For new code, this makes me think the version parsing/compare could live in CDK instead if there was a smarter type around it, for the sake of not repeating a bad pattern.If we continue with this, really the best approach would be for some of the version utilities ( and our types) to be put on npm as a library dependency, considering there's no shared caching/persistent data. I have avoided doing that as there's never been a big drive to do so - (although it hurts seeing others have to copy our type files instead of using @types/.)
I understand doing so would:
A - create a new work item
B - create a new maintenance point (which we could hopefully automate the release of alongside new extension releases)
C - delay the progress of your feature
I'm curious on your opinion w.r.t this - it may be best to have it live in CDK for now.