-
Notifications
You must be signed in to change notification settings - Fork 443
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.
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
[release/6.0] Add alpine to CI for source-build #13074
[release/6.0] Add alpine to CI for source-build #13074
Conversation
omajid
commented
Jan 18, 2022
- Please add description for changes you are making.
- If there is an issue related to this PR, please add the reference.
c763019
to
6d414d6
Compare
This is the current set of errors:
Which should get fixed by dotnet/source-build-reference-packages#319, hopefully. Though not sure how |
6d414d6
to
ad828b1
Compare
Build fails because dotnet/runtime#64304 is not included in the SDK being used to build. I guess this will have to wait unit the next patch release. |
ad828b1
to
0fb4ec6
Compare
Error is in building dotnet/runtime:
The RID should have been This also makes me wonder: if source-build knows the correct RID value, shouldn't we just pass that onto runtime instead of letting it compute a possibly different value? |
This should be fixed now that dotnet/runtime#62942 is merged and more definitively with dotnet/runtime#74504 |
0fb4ec6
to
01527ce
Compare
Hey, @ayakael have you seen this error on alpine?
|
Yes, it was fixed by dotnet/aspnetcore#46735. |
Strange. I am seeing the error while building runtime, not aspnetcore 😖 |
Due to dotnet/runtime#73525, we actually manually set |
Oh, the error is when building the portable version of runtime. RID is |
Apologies, early morning brain fog haha: Yeah, I encountered this since version 6.0.109. The following patch is a workaround: From 79f02a53316f90543d60269d7c06727c376f423b Mon Sep 17 00:00:00 2001
From: Antoine Martin <dev@ayakael.net>
Date: Wed, 17 Sep 2022 18:41:08 +0000
Subject: [PATCH 1/1] No additional runtime id
For some reason, AdditionalRuntimeIdentifiers gets set as '$arch' rather than 'linux-musl-$arch'
Since we know our portable RID exists, just skip ensuring RID existence
---here's something wrong with
diff --git a/src/runtime/src/libraries/Microsoft.NETCore.Platforms/src/Microsoft.NETCore.Platforms.csproj.orig b/src/runtime/src/libraries/Microsoft.NETCore.Platforms/src/Microsoft.NETCore.Platforms.csproj
index 84d4bb8..58f7e01 100644
--- a/src/runtime/src/libraries/Microsoft.NETCore.Platforms/src/Microsoft.NETCore.Platforms.csproj.orig
+++ b/src/runtime/src/libraries/Microsoft.NETCore.Platforms/src/Microsoft.NETCore.Platforms.csproj
@@ -17,8 +17,6 @@
<NoWarn>$(NoWarn);NU5128</NoWarn> <!-- No Dependencies-->
<PackageDescription>Provides runtime information required to resolve target framework, platform, and runtime specific implementations of .NETCore packages.</PackageDescription>
- <!-- When building from source, ensure the RID we're building for is part of the RID graph -->
- <AdditionalRuntimeIdentifiers Condition="'$(DotNetBuildFromSource)' == 'true'">$(AdditionalRuntimeIdentifiers);$(OutputRID)</AdditionalRuntimeIdentifiers>
<ServicingVersion>8</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
</PropertyGroup>
--
2.37.1 I never took the time to debug what's wrong. |
Thanks. I filed dotnet/runtime#84127 since I could reproduce this on all branches of dotnet/runtime using an alpine container. |
To unlock this, do you want me to backport whatever fix comes out of dotnet/runtime#84127 to |
Backported: dotnet/runtime#84442 |
41e1f4e
to
c8be71c
Compare
Current error is:
Looks like aspnetcore is incorrectly using @ayakael do you have a fix for that ? |
Yes! This is caused by dotnet/aspnetcore#39822
Backporting the portable build elimination patch should've fixed this, but as that was scrapped the problem still exists to this day when building aspnetcore with Rid linux-musl-$arch
We've eliminated runtime-portable on Alpine's dotnet6. The fix before the backport was the following:
https://lab.ilot.io/mirrors/aports/-/blob/87d14a808f8a227bfb7e5f0fde955eb0f15dbd03/community/dotnet6-build/build_musl-build-fix.patch
It doesn't work though. It breaks build for non-musl as I never got the musl detection fix to work.
|
@omajid Could you try this patch from within your PR?: diff --git a/src/SourceBuild/tarball/content/Directory.Build.props b/src/SourceBuild/tarball/content/Directory.Build.props
index 797e54fa6..8be93da95 100644
--- a/src/SourceBuild/tarball/content/Directory.Build.props
+++ b/src/SourceBuild/tarball/content/Directory.Build.props
@@ -181,6 +181,7 @@
<PortableRid Condition="'$(TargetOS)' == 'FreeBSD'">freebsd-$(Platform)</PortableRid>
<PortableRid Condition="'$(TargetOS)' == 'OSX'">osx-$(Platform)</PortableRid>
<PortableRid Condition="'$(TargetOS)' == 'Linux'">linux-$(Platform)</PortableRid>
+ <PortableRid Condition="$(TargetRid.StartsWith('linux-musl')) or $(TargetRid.StartsWith('alpine'))">linux-musl-$(Platform)</PortableRid>
<PortableRid Condition="'$(TargetOS)' == 'Windows_NT'">win-$(Platform)</PortableRid>
<TargetRid Condition="'$(PortableBuild)' == 'true' AND '$(PortableRid)' != ''">$(PortableRid)</TargetRid>
</PropertyGroup>
diff --git a/src/SourceBuild/tarball/content/repos/aspnetcore.proj b/src/SourceBuild/tarball/content/repos/aspnetcore.proj
index 480f3c713..9b25187ab 100644
--- a/src/SourceBuild/tarball/content/repos/aspnetcore.proj
+++ b/src/SourceBuild/tarball/content/repos/aspnetcore.proj
@@ -7,6 +7,7 @@
<!-- The arch flag (defaults to x64) overrides any value of TargetArchitecture that we might set -->
<BuildCommandArgs>$(BuildCommandArgs) --arch $(Platform)</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) --no-build-repo-tasks</BuildCommandArgs>
+ <BuildCommandArgs Condition="$(PortableRid.StartsWith('linux-musl'))">$(BuildCommandArgs) --os-name linux-musi</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:BuildNodeJs=false</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:PublishCompressedFilesPathPrefix=$(SourceBuiltAspNetCoreRuntime)</BuildCommandArgs>
<!-- Update to 1.0.0 version of reference assemblies which are built in SBRP instead of the preview.2 version
diff --git a/src/SourceBuild/tarball/content/repos/installer.proj b/src/SourceBuild/tarball/content/repos/installer.proj
index e34337b85..9b42de398 100644
--- a/src/SourceBuild/tarball/content/repos/installer.proj
+++ b/src/SourceBuild/tarball/content/repos/installer.proj
@@ -25,7 +25,7 @@
<BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:Rid=$(TargetRid)</BuildCommandArgs>
<BuildCommandArgs>$(BuildCommandArgs) /p:DOTNET_INSTALL_DIR=$(DotNetCliToolDir)</BuildCommandArgs>
- <BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:AspNetCoreInstallerRid=linux-$(Platform)</BuildCommandArgs>
+ <BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:AspNetCoreInstallerRid=$(PortableRid)</BuildCommandArgs>
<!-- core-sdk always wants to build portable on OSX and FreeBSD -->
<BuildCommandArgs Condition="'$(TargetOS)' == 'FreeBSD'">$(BuildCommandArgs) /p:CoreSetupRid=freebsd-x64 /p:PortableBuild=true</BuildCommandArgs>
<BuildCommandArgs Condition="'$(TargetOS)' == 'OSX'">$(BuildCommandArgs) /p:CoreSetupRid=osx-x64</BuildCommandArgs> I can't test it locally. |
Okay, I've created #17324 that should unblock this. These changes fix building dotnet6 on Alpine Linux without eliminating runtime-portable. |
c8be71c
to
2cda917
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing these changes in the internal tarball build pipeline to verify things work. This doesn't get run by the PR.
@@ -9,6 +9,7 @@ parameters: | |||
|
|||
# The following parameters aren't expected to be passed in rather they are used for encapsulation | |||
# ----------------------------------------------------------------------------------------------- | |||
alpine317Container: mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.17-20230214152931-7f13c75 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want this to be latest. Following the pattern of the other image names is not what we want. I'll update those separately.
alpine317Container: mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.17-20230214152931-7f13c75 | |
alpine317Container: mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.17 |
@omajid - Finally got everything tested internally. I've pushed the necessary changes that will make this work. |