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

BundledVersions: update portable rid logic. #14647

Merged
merged 1 commit into from
Nov 15, 2022
Merged

BundledVersions: update portable rid logic. #14647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions src/SourceBuild/tarball/content/repos/installer.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
<OverrideTargetRid Condition="'$(TargetOS)' == 'OSX'">osx-x64</OverrideTargetRid>
<OSNameOverride>$(OverrideTargetRid.Substring(0, $(OverrideTargetRid.IndexOf("-"))))</OSNameOverride>

<!-- Determine target portable rid based on bootstrap SDK's portable rid -->
<_platformIndex>$(NETCoreSdkPortableRuntimeIdentifier.LastIndexOf('-'))</_platformIndex>
<PortableOS>$(NETCoreSdkPortableRuntimeIdentifier.Substring(0, $(_platformIndex)))</PortableOS>
Copy link

Choose a reason for hiding this comment

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

It'd be nice if <PortableOS> could be set only if it is empty. Thus:
<PortableOS Condition="'$(PortableOS)' == ''">

This ensures that we can override the PortableOS if, for example, it was previously the wrong one.

Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures that we can override the PortableOS if, for example, it was previously the wrong one.

Do you know can override it if by setting /p:PortableOS, for example in SourceBuild.props, even when the Condition is not present?

Copy link

@ayakael ayakael Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In depends where that override flows down. I suspect that in SourceBuild.props, any non-conditionnal setting of PortableOS would override what's in installer.proj. For Alpine, our version 6.0.110 of the SDK regressed in that --use-current-runtime came out as alpine-3.17-x64. Thus, we had to override whatever would come out of $(NETCoreSdkPortableRuntimeIdentifier), as it wouldn't be linux-musl-x64. For building 6.0.111, we set /p:PortableOS=linux-musl as an argument to build.sh, including the conditional setting of PortableOS.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ayakael is it good for you as is, or do you still want me to add the Condition?

Copy link

Choose a reason for hiding this comment

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

It'd be more prudent, but it's fine as-is. Once the new Portable rid flows down the condition isn't necessary anyways.


<RuntimeArg>--runtime-id $(OverrideTargetRid)</RuntimeArg>
<RuntimeArg Condition="'$(TargetOS)' == 'Linux'">--runtime-id $(TargetRid)</RuntimeArg>

Expand All @@ -22,6 +26,7 @@
-->
<BuildCommandArgs>$(BuildCommandArgs) /p:NETCoreAppMaximumVersion=99.9</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:OSName=$(OSNameOverride)</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:PortableOSName=$(PortableOS)</BuildCommandArgs>
<BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:Rid=$(TargetRid)</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:DOTNET_INSTALL_DIR=$(DotNetCliToolDir)</BuildCommandArgs>

Expand Down
16 changes: 9 additions & 7 deletions src/redist/targets/GetRuntimeInformation.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@
<HostOSName Condition=" '$(HostOSName)' == '' AND $([MSBuild]::IsOSPlatform('OSX')) ">osx</HostOSName>
<HostOSName Condition=" '$(HostOSName)' == '' AND $([MSBuild]::IsOSPlatform('FREEBSD')) ">freebsd</HostOSName>
<HostOSName Condition=" '$(HostOSName)' == '' AND '$(IsLinux)' == 'True' ">linux</HostOSName>


<OSName Condition=" '$(OSName)' == '' AND $(Rid) != '' ">$(Rid.Substring(0, $(Rid.LastIndexOf('-'))))</OSName>
<OSName Condition=" '$(OSName)' == '' ">$(HostOSName)</OSName>

<Rid Condition=" '$(Rid)' == '' ">$(OSName)-$(Architecture)</Rid>
<PortableOSName Condition=" '$(PortableOSName)' == '' ">$(OSName)</PortableOSName>

<Rid>$(OSName)-$(Architecture)</Rid>

<PortableRid>$(PortableOSName)-$(Architecture)</PortableRid>
</PropertyGroup>

<PropertyGroup>
Expand All @@ -24,12 +29,9 @@
</PropertyGroup>

<PropertyGroup>
<ProductMonikerRid Condition=" '$(Rid)' == 'ubuntu.16.04-x64' OR
'$(Rid)' == 'rhel.6-x64' OR
'$(Rid)' == 'linux-musl-x64' ">$(Rid)</ProductMonikerRid>
<ProductMonikerRid Condition=" '$(ProductMonikerRid)' == '' ">$(OSName)-$(Architecture)</ProductMonikerRid>
<ProductMonikerRid Condition=" '$(ProductMonikerRid)' == '' ">$(Rid)</ProductMonikerRid>

<PortableProductMonikerRid Condition=" '$(PortableProductMonikerRid)' == '' ">$(HostOSName)-$(Architecture)</PortableProductMonikerRid>
<PortableProductMonikerRid Condition=" '$(PortableProductMonikerRid)' == '' ">$(PortableRid)</PortableProductMonikerRid>

<ArtifactNameSdk>dotnet-sdk-internal$(PgoTerm)</ArtifactNameSdk>
<ArtifactNameCombinedHostHostFxrFrameworkSdk>dotnet-sdk$(PgoTerm)</ArtifactNameCombinedHostHostFxrFrameworkSdk>
Expand Down