Add --allow-prerelease and --no-sdk-version options to dotnet new globaljson template#54133
Add --allow-prerelease and --no-sdk-version options to dotnet new globaljson template#54133marcpopMSFT with Copilot wants to merge 14 commits into
Conversation
|
Why does file generated with |
|
Hmm, I missed in the original request that you didn't even want an sdk value. The current behavior includes a default value for that. We could add a --no-sdk flag but then that would have to have error checking with --sdk-version. I'm not sure we'd want --sdk-version without a value to do that as that's most likely a typo (as I expect this to be the less common scenario). We could allow --sdk-version false and somehow make that not provide a version but that's a bit odd as version expects a version. @baronfel any thoughts on a direction to go to have allow-prerelease with no version? |
|
A |
|
@copilot can we add a --no-sdk-version option that won't include the SDK version as there is a scenario where a customer might want all-prerelease of false but not specify a version. |
Added dotnet new globaljson --allow-prerelease false --no-sdk-versionOutput: {
"sdk": {
"allowPrerelease": false
}
}The template conditional logic handles all combinations of |
|
@copilot when i'm testing this, I end up with an error creating the global.json and it ends halfway through. |
... Fixed in commit 4d1ae57. The root cause was that the template engine's |
@copilot why can't a |
You raise a valid point. Technically, a The key behavioral difference is in the condition evaluator's I changed to |
|
@copilot I'd prefer to fix the boolean conversion consistency issue - the 'natural' way to define data types should work consistently. |
Fixed in commit 223f5a8. The root cause was in Changes:
|
Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/3a08d95c-8aa4-40b3-827d-65843a267b72 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/bb7321e2-df28-4dc1-8781-6a48f9437b42 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/31064ea9-7627-4134-901b-89b936104671 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
…tors Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/715f5aeb-cabe-46e3-8c35-2bda89707630 Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
…tual exclusion) Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/ffec2f0f-275e-417e-9840-41550b9f33e3 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
… evaluators Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/ffec2f0f-275e-417e-9840-41550b9f33e3 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/ffec2f0f-275e-417e-9840-41550b9f33e3 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
…lting to true Agent-Logs-Url: https://github.com/dotnet/sdk/sessions/62e860c0-a590-4876-9f13-a0782f3bd215 Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
0505e54 to
f2f727e
Compare
There was a problem hiding this comment.
Pull request overview
Adds two new template options to dotnet new globaljson — --allow-prerelease (to set the allowPrerelease field) and --no-sdk-version (to omit the SDK version from the generated file). Also patches the in-repo Microsoft.TemplateEngine.Core boolean evaluators to handle string-typed bool values without throwing InvalidCastException, and adds integration approval snapshots and unit tests for the new behavior.
Changes:
- New
AllowPrereleaseandNoSdkVersiontemplate parameters wired to CLI options, with conditional logic inglobal.jsonto keep JSON well-formed across all combinations. - Template engine fix:
Scope.csAnd/Or/Xorand the shared/Cpp condition finalizers now coerce string and empty-string operands to bool instead of casting directly. - New integration approval snapshots and 4 new
Cpp2EvaluatorTestsunit tests.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| template_feed/.../GlobalJson/.template.config/template.json | Adds AllowPrerelease, AllowPrereleaseLower, NoSdkVersion symbols (declared as bool — see review). |
| template_feed/.../GlobalJson/global.json | Restructures conditionals to emit rollForward, allowPrerelease, version with valid trailing commas. |
| template_feed/.../GlobalJson/.template.config/dotnetcli.host.json | Maps new symbols to --allow-prerelease / --no-sdk-version. |
| template_feed/.../localize/templatestrings.*.json (×14) | Adds English placeholder strings for new symbols (including `choices/true |
| src/TemplateEngine/.../Cpp/Scope.cs | Adds ToBool helper used by Not/And/Or/Xor to handle string and empty operands. |
| src/TemplateEngine/.../Cpp/CppStyleEvaluatorDefinition.cs | Treats empty-string final result as false. |
| src/TemplateEngine/.../Shared/SharedEvaluatorDefinition.cs | Treats empty-string evaluation result as false. |
| test/.../Cpp2EvaluatorTests.cs | Adds 4 tests for string-bool &&/` |
| test/dotnet-new.IntegrationTests/CommonTemplatesTests.cs | Adds 6 new [InlineData] cases and broadens version-scrub guard for --no-sdk-version. |
| test/.../Approvals/DotnetNewHelpTests.CanShowHelpForTemplate_globaljson.verified.txt | Help snapshot updated with new options (renders --allow-prerelease as choice — see review). |
| test/.../Approvals/AllCommonItemsCreate.--allow-prerelease.verified/** | New approval files for the 5 new test combinations. |
| "AllowPrerelease": { | ||
| "type": "parameter", | ||
| "description": "Whether prerelease SDK versions should be considered when selecting the SDK version to use.", | ||
| "displayName": "Allow prerelease", | ||
| "defaultValue": false, | ||
| "defaultIfOptionWithoutValue": true, | ||
| "datatype": "bool" | ||
| }, | ||
| "AllowPrereleaseLower": { | ||
| "type": "generated", | ||
| "generator": "casing", | ||
| "parameters": { | ||
| "source": "AllowPrerelease", | ||
| "toLower": "true" | ||
| }, | ||
| "replaces": "ALLOW_PRERELEASE_VALUE" | ||
| }, |
| { | ||
| "sdk": { | ||
| } | ||
| } |
| "symbols/AllowPrerelease/description": "Whether prerelease SDK versions should be considered when selecting the SDK version to use.", | ||
| "symbols/AllowPrerelease/displayName": "Allow prerelease", | ||
| "symbols/AllowPrerelease/choices/true/description": "Consider prerelease SDK versions when selecting the .NET SDK version to use.", | ||
| "symbols/AllowPrerelease/choices/true/displayName": "Allow prerelease", | ||
| "symbols/AllowPrerelease/choices/false/description": "Don't consider prerelease SDK versions when selecting the .NET SDK version to use.", | ||
| "symbols/AllowPrerelease/choices/false/displayName": "Disallow prerelease", | ||
| "symbols/NoSdkVersion/description": "Exclude the SDK version from the generated global.json.", | ||
| "symbols/NoSdkVersion/displayName": "No SDK version", |
| private static bool ToBool(object? x) | ||
| { | ||
| return x switch | ||
| { | ||
| bool b => b, | ||
| null => false, | ||
| string s when string.IsNullOrEmpty(s) => false, // unspecified choice parameter (defaultValue: "") → false | ||
| string s when bool.TryParse(s, out bool parsed) => parsed, | ||
| _ => Convert.ToBoolean(x) | ||
| }; | ||
| } |
|
@baronfel the current implementation doesn't set the global.json property if all-prerelease is false. I can't switch the check back to != as now it had a default value which means it sets that property even if not specified. It needs to only set a default if the flag is passed in (with no true/false value included) and otherwise be null. Also, I'm not sure if there is a way to require all-prerelease if you're specifying --no-sdk-version as otherwise, you end up with a non-useable globaljson. |
|
@baronfel looks like we lost the cleaner history but my prpoosal is to go back to the state I tested before where everything worked except for an empty --allow-prerelease. So not specifying it worked and setting it to true or false worked. I wonder if we should reset and have copilot start from scratch. |
|
@copilot can you switch back to the solution you had when I made this comment? #54133 (comment) both us and you were unable to solve the behavior and our choice is to either keep investing when we aren't sure we can fix this or accept that state where the feature overall works except you have to specify true/false. I'm ok with that prior state where we ignore the flag unless you specify the boolean value. |
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
- Reverts f2f727e: restores AllowPrerelease as choice type (defaultValue: "", defaultIfOptionWithoutValue: "true") with string-comparison conditions - Removes AllowPrereleaseLower casing generator (not needed with choice type) - Removes failing test case for --allow-prerelease without value - Removes corresponding approval snapshot files Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Reverted to the state before |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
…release-option # Conflicts: # test/TemplateEngine/Microsoft.TemplateEngine.Core.UnitTests/Cpp2EvaluatorTests.cs # test/dotnet-new.IntegrationTests/CommonTemplatesTests.cs Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Resolved in the merge commit. There were two conflicts:
|
dotnet new globaljsonhad no way to setallowPrereleasein the generatedglobal.json, requiring users to manually look up the field name and add it by hand. Additionally, the template always included an SDK version, preventing users from creating a minimalglobal.jsonwith onlyallowPrereleaseset.Template changes
AllowPrereleasechoice parameter ("true"/"false") totemplate.jsonwithdefaultValue: ""(omitted from output when unspecified — fully backward compatible) anddefaultIfOptionWithoutValue: "true"NoSdkVersionbool parameter totemplate.jsonwithdefaultValue: false— when set, the SDK version is omitted from the generatedglobal.json--allow-prereleaseand--no-sdk-versionCLI options indotnetcli.host.jsonrollForward,allowPrerelease,version), using string-comparison conditions (AllowPrerelease!="")Template engine fix
Fixed a boolean conversion consistency bug in
Microsoft.TemplateEngine.Core: theAnd,Or, andXoroperators inScope.csused a direct(bool)xcast that throwsInvalidCastExceptionwhen a parameter value is stored as a string (e.g."false"). These operators now useConvert.ToBoolean(x ?? "False"), consistent with how theNotoperator already handled this case. Empty strings are also handled gracefully (treated asfalse) to prevent errors when choice parameters withdefaultValue: ""are evaluated in boolean context.Usage
Output for
--allow-prerelease false:{ "sdk": { "allowPrerelease": false, "version": "9.0.100" } }Output for
--allow-prerelease false --no-sdk-version:{ "sdk": { "allowPrerelease": false } }Tests
AllCommonItemsCreatetest cases covering--allow-prerelease false,--allow-prerelease true, combined with--sdk-version,--no-sdk-versionalone, and--allow-prerelease false --no-sdk-versionglobaljsonhelp text snapshot to include both new optionsCpp2EvaluatorTestsverifying that&&and||operators correctly handle string-typed boolean variable values