Skip to content
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.

Sign up for GitHub

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

Add support for additional DotNetRuntime #7158

Merged
merged 10 commits into from
Mar 31, 2021
Merged

Add support for additional DotNetRuntime #7158

Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
29b508e
Add support for additional DotNetRuntime
HaoK Mar 26, 2021
0bbfad8
Update
HaoK Mar 31, 2021
92e20a5
Use package channel
HaoK Mar 31, 2021
0be539a
Cleanup props file
HaoK Mar 31, 2021
444eedc
Set defaults
HaoK Mar 31, 2021
41530d1
PR feedback
HaoK Mar 31, 2021
70698e6
Add notes for AdditionalDotNetPackage
HaoK Mar 31, 2021
6e5ffd6
PR feedback
HaoK Mar 31, 2021
6a3afaa
Update Readme.md
HaoK Mar 31, 2021
1e43db3
Update Readme.md
HaoK Mar 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/Microsoft.DotNet.Helix/Sdk/tools/dotnet-cli/DotNetCli.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,27 @@
<HelixPreCommands Condition="!$(IsPosixShell)">$(HelixPreCommands);set DOTNET_ROOT=%HELIX_CORRELATION_PAYLOAD%\$(DotNetCliDestination);set DOTNET_CLI_TELEMETRY_OPTOUT=1</HelixPreCommands>
</PropertyGroup>
</Target>

<Target Name="AddAdditionalRuntimes"
Condition="@(AdditionalDotNetPackage->Count()) != 0"
AfterTargets="Build"
Outputs="%(AdditionalDotNetPackage.Identity)">
<PropertyGroup>
<_channel>%(AdditionalDotNetPackage.Channel)</_channel>
<_channel Condition=" '$(_channel)' == '' ">Current</_channel>
<_package>%(AdditionalDotNetPackage.PackageType)</_package>
Copy link
Member

Choose a reason for hiding this comment

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

since you're reading PackageType, consider _packageType

<_package Condition=" '$(_package)' == '' ">runtime</_package>
</PropertyGroup>

<Message Text = "FindDotNetCliPackage '%(AdditionalDotNetPackage.Identity)' '$(DotNetCliRuntime)' '$(_package)' '$(_channel)' "/>
Copy link
Member

Choose a reason for hiding this comment

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

I think this is cruft from my example? If not, you could improve the text like "Adding package ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

Up to you, I found it nice for debugging, I can remove it or update the text, what do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

If it's up to me I definitely favor logging it with normal verbosity, something like

<Message Text = "Adding correlation payload for additional .NET Core package: Version: '%(AdditionalDotNetPackage.Identity)'  Runtime: '$(DotNetCliRuntime)' Type: '$(_package)' Channel: '$(_channel)' "/>

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, updated

<FindDotNetCliPackage Version="%(AdditionalDotNetPackage.Identity)" Runtime="$(DotNetCliRuntime)" PackageType="$(_package)" Channel="$(_channel)">
<Output TaskParameter="PackageUri" PropertyName="DotNetCliPackageUri"/>
</FindDotNetCliPackage>
<ItemGroup>
<HelixCorrelationPayload Include="dotnet-additional">
<Uri>$(DotNetCliPackageUri)</Uri>
<Destination>$(DotNetCliDestination)</Destination>
</HelixCorrelationPayload>
</ItemGroup>
</Target>
</Project>