Skip to content

Fix Alpine SB legs #19623

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

Jump to bottom
Merged
merged 1 commit into from
May 1, 2024
Merged

Fix Alpine SB legs #19623

merged 1 commit into from
May 1, 2024

Conversation

mthalman
Copy link
Member

@mthalman mthalman commented May 1, 2024

The changes from #19222 caused a regression for source build Alpine build legs, causing the following error:

/vmr/src/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj : error NU1100: Unable to resolve 'runtime.linux-musl-x64.Microsoft.DotNet.ILCompiler (>= 9.0.0-preview.4.24218.7)' for 'net9.0'. PackageSourceMapping is enabled, the following source(s) were not considered: prebuilt, previously-source-built, reference-packages, source-built-command-line-api, source-built-emsdk, source-built-source-build-externals, source-built-transport-arcade, source-built-transport-cecil, source-built-transport-emsdk. [/vmr/src/runtime/Build.proj]
/vmr/src/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj : error NU1100: Unable to resolve 'runtime.linux-musl-x64.Microsoft.DotNet.ILCompiler (>= 9.0.0-preview.4.24218.7)' for 'net9.0/linux-musl-x64'. PackageSourceMapping is enabled, the following source(s) were not considered: prebuilt, previously-source-built, reference-packages, source-built-command-line-api, source-built-emsdk, source-built-source-build-externals, source-built-transport-arcade, source-built-transport-cecil, source-built-transport-emsdk. [/vmr/src/runtime/Build.proj]
/vmr/src/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj : error NU1100: Unable to resolve 'Microsoft.NETCore.App.Runtime.linux-musl-x64 (= 9.0.0-preview.4.24218.7)' for 'net9.0'. PackageSourceMapping is enabled, the following source(s) were not considered: prebuilt, previously-source-built, reference-packages, source-built-command-line-api, source-built-emsdk, source-built-source-build-externals, source-built-transport-arcade, source-built-transport-cecil, source-built-transport-emsdk. [/vmr/src/runtime/Build.proj]
/vmr/src/runtime/src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj : error NU1100: Unable to resolve 'Microsoft.NETCore.App.Host.linux-musl-x64 (= 9.0.0-preview.4.24218.7)' for 'net9.0'. PackageSourceMapping is enabled, the following source(s) were not considered: prebuilt, previously-source-built, reference-packages, source-built-command-line-api, source-built-emsdk, source-built-source-build-externals, source-built-transport-arcade, source-built-transport-cecil, source-built-transport-emsdk. [/vmr/src/runtime/Build.proj]

This is due to the change made in dotnet/dotnet@546a6bb#diff-e98dbe2aa8231e9d6264210a63f02e97b5e26de2cca16db7ee471a2257f32c8f to define targetOS for the Alpine build legs. That change was intended to make this check succeed: https://github.com/dotnet/dotnet/blob/00d6710677d75715dedef053b70a34b09af8bc47/test/tests.proj#L14.

The fix is to remove the setting of targetOS and update the test condition to not apply for source build since source build doesn't use portable RIDs.

@mthalman mthalman requested review from a team as code owners May 1, 2024 18:29
MichaelSimons
MichaelSimons reviewed May 1, 2024
View reviewed changes
src/SourceBuild/content/test/tests.proj
@@ -11,7 +11,7 @@
<!-- Skip scenario tests if the portable OS (determined from the host machine) is different from the target OS
since the tests require the ability to execute the built SDK. An example of where this would be disabled is
cross-build of using Mariner to build for Alpine (linux vs linux-musl). -->
<_RunScenarioTests Condition="'$(BuildOS)' != 'windows' and '$(__PortableTargetOS.ToLowerInvariant())' != '$(TargetOS.ToLowerInvariant())'">false</_RunScenarioTests>
<_RunScenarioTests Condition="'$(BuildOS)' != 'windows' and '$(DotNetBuildSourceOnly)' == 'false' and '$(__PortableTargetOS.ToLowerInvariant())' != '$(TargetOS.ToLowerInvariant())'">false</_RunScenarioTests>
Copy link
Member

Choose a reason for hiding this comment

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

Can someone help me understand why __PortableTargetOS is initialized to linux-musl on alpine while targetOS is linux? I'm trying to understand if the PortableTargetOS comparison against TargetOS is correct.

Given this is blocking. We should merge and circle back if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you asking in the context of SB?

Copy link
Member

Choose a reason for hiding this comment

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

Are you asking in the context of SB?

No, in general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's look at an example. In UB cross build scenarios of building Alpine from Mariner, __PortableTargetOS is set to linux here: https://github.com/dotnet/arcade/blob/be933308b9024d798a9a22c0b8f3c8e3616ffbd8/eng/common/native/init-distro-rid.sh#L111

And TargetOS is set to linux-musl as a build property here:

installer/eng/pipelines/templates/jobs/vmr-build.yml

Line 327 in d092a8a

extraBuildProperties="$extraBuildProperties /p:TargetOS=${{ parameters.targetOS }}"

This build property doesn't impact the value of targetOS in the init script that __PortableTargetOS is derived from.

/cc @ViktorHofer

@mthalman mthalman enabled auto-merge (squash) May 1, 2024 18:43
@mthalman mthalman merged commit d092a8a into dotnet:main May 1, 2024
@mthalman mthalman deleted the portable-rid-fix branch May 1, 2024 19:32
ViktorHofer
ViktorHofer reviewed May 3, 2024
View reviewed changes
src/SourceBuild/content/test/tests.proj
@@ -11,7 +11,7 @@
<!-- Skip scenario tests if the portable OS (determined from the host machine) is different from the target OS
since the tests require the ability to execute the built SDK. An example of where this would be disabled is
cross-build of using Mariner to build for Alpine (linux vs linux-musl). -->
<_RunScenarioTests Condition="'$(BuildOS)' != 'windows' and '$(__PortableTargetOS.ToLowerInvariant())' != '$(TargetOS.ToLowerInvariant())'">false</_RunScenarioTests>
<_RunScenarioTests Condition="'$(BuildOS)' != 'windows' and '$(DotNetBuildSourceOnly)' == 'false' and '$(__PortableTargetOS.ToLowerInvariant())' != '$(TargetOS.ToLowerInvariant())'">false</_RunScenarioTests>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we ever set DotNetBuildSourceOnly to false. This should probably be changed to '$(DotNetBuildSourceOnly)' != 'true'".

@mthalman mthalman mentioned this pull request May 3, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ViktorHofer ViktorHofer ViktorHofer left review comments

@MichaelSimons MichaelSimons MichaelSimons approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mthalman @ViktorHofer @MichaelSimons