Skip to content

Change Helix Sdk IncludeDotNetCli default to the version used to build#5748

Merged
alexperovich merged 5 commits into
dotnet:masterfrom
alexperovich:useCurrentSdkForIncludeDotNetCli
Jul 7, 2020
Merged

Change Helix Sdk IncludeDotNetCli default to the version used to build#5748
alexperovich merged 5 commits into
dotnet:masterfrom
alexperovich:useCurrentSdkForIncludeDotNetCli

Conversation

@alexperovich
Copy link
Copy Markdown
Member

No description provided.

<DotNetCliPackageType Condition=" '$(DotNetCliPackageType)' == '' ">runtime</DotNetCliPackageType>
<DotNetCliVersion Condition=" '$(DotNetCliVersion)' == '' AND '$(DotNetCliPackageType)' == 'runtime' ">2.1.5</DotNetCliVersion>
<DotNetCliVersion Condition=" '$(DotNetCliVersion)' == '' AND '$(DotNetCliPackageType)' == 'sdk' ">2.1.403</DotNetCliVersion>
<DotNetCliPackageType Condition=" '$(DotNetCliPackageType)' == '' ">sdk</DotNetCliPackageType>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is changing defaults needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any "This is the runtime version for this sdk" in the properties. In order to make this work using the current sdk version as the default I had to switch it to using "sdk" rather than "runtime". That isn't a full break because the sdk can do anything the runtime can do.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a $(BundledNETCoreAppPackageVersion) property to get the runtime bundled with the current SDK.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the version that can be passed to dotnet-install.ps1 when installing the runtime? If so I can use that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! dotnet/runtime uses it as the value of DotNetCliVersion for the CoreCLR Helix test runs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, i've switched the default back to 'runtime' and it chooses the right property based on that.

Copy link
Copy Markdown
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct this won't be validated completely until the change is merged and we update the Arcade versions in dotnet/aspnetcore#23585❔ (Regular validation ring would help only if another repo is already using $(IncludeDotNetCli) and not setting $(DotNetCliVersion).)

@alexperovich
Copy link
Copy Markdown
Member Author

I validated it locally, and the PR build will use it.

@alexperovich alexperovich merged commit 9bd0f76 into dotnet:master Jul 7, 2020
@alexperovich alexperovich deleted the useCurrentSdkForIncludeDotNetCli branch July 7, 2020 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants