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

Jump to bottom

[Homebrew/macOS] .NET 6 and 7 build patches/workarounds #2795

Closed
5 tasks done
cho-m opened this issue Mar 9, 2022 · 10 comments
Closed
5 tasks done

[Homebrew/macOS] .NET 6 and 7 build patches/workarounds #2795

cho-m opened this issue Mar 9, 2022 · 10 comments
Assignees
@MichaelSimons
Labels
area-patch-removal Removing patches for contributing repos from source-build

Comments

@cho-m
Copy link

cho-m commented Mar 9, 2022 •
edited by MichaelSimons
Loading

Opening an issue to track various patches/workarounds we do in Homebrew for macOS build to find out which are already fixed, if we need to upstream some, to discuss better ways to fix, and/or if should be part of documentation.

Some prior discussion in #2719 and 6.0.102 work in Homebrew PR Homebrew/homebrew-core#95339 (EDIT: 6.0.103 work in PR Homebrew/homebrew-core#96612)

  • Build failure on macOS ARM due to osx-x64 override. The following is the patch we use to allow using osx-arm64. If acceptable, I can create PR in dotnet/installer:

    Patch 
    diff --git a/src/SourceBuild/tarball/content/repos/installer.proj b/src/SourceBuild/tarball/content/repos/installer.proj
index 712d7cd14..31d54866c 100644
--- a/src/SourceBuild/tarball/content/repos/installer.proj
+++ b/src/SourceBuild/tarball/content/repos/installer.proj
@@ -7,7 +7,7 @@
   <PropertyGroup>
     <OverrideTargetRid>$(TargetRid)</OverrideTargetRid>
-    <OverrideTargetRid Condition="'$(TargetOS)' == 'OSX'">osx-x64</OverrideTargetRid>
+    <OverrideTargetRid Condition="'$(TargetOS)' == 'OSX'">osx-$(Platform)</OverrideTargetRid>
     <OSNameOverride>$(OverrideTargetRid.Substring(0, $(OverrideTargetRid.IndexOf("-"))))</OSNameOverride>
     <RuntimeArg>--runtime-id $(OverrideTargetRid)</RuntimeArg>
@@ -28,7 +28,7 @@
     <BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:AspNetCoreSharedFxInstallerRid=linux-$(Platform)</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>
+    <BuildCommandArgs Condition="'$(TargetOS)' == 'OSX'">$(BuildCommandArgs) /p:CoreSetupRid=osx-$(Platform)</BuildCommandArgs>
     <BuildCommandArgs Condition="'$(TargetOS)' == 'Linux'">$(BuildCommandArgs) /p:CoreSetupRid=$(TargetRid)</BuildCommandArgs>
     <!-- Consume the source-built Core-Setup and toolset. This line must be removed to source-build CLI without source-building Core-Setup first. -->
diff --git a/src/SourceBuild/tarball/content/repos/runtime.proj b/src/SourceBuild/tarball/content/repos/runtime.proj
index f3ed143f8..2c62d6854 100644
--- a/src/SourceBuild/tarball/content/repos/runtime.proj
+++ b/src/SourceBuild/tarball/content/repos/runtime.proj
@@ -3,7 +3,7 @@
   <PropertyGroup>
     <OverrideTargetRid>$(TargetRid)</OverrideTargetRid>
-    <OverrideTargetRid Condition="'$(TargetOS)' == 'OSX'">osx-x64</OverrideTargetRid>
+    <OverrideTargetRid Condition="'$(TargetOS)' == 'OSX'">osx-$(Platform)</OverrideTargetRid>
     <OverrideTargetRid Condition="'$(TargetOS)' == 'FreeBSD'">freebsd-x64</OverrideTargetRid>
     <OverrideTargetRid Condition="'$(TargetOS)' == 'Windows_NT'">win-x64</OverrideTargetRid>

  • Build failure on macOS due to missing ILAsm/ILDAsm. This seems to be common issue on Linux too. The odd thing though is that RuntimeOsxArm64MicrosoftNETCoreILAsmVersion exists but RuntimeOsxX64MicrosoftNETCoreILAsmVersion doesn't, so needed to use Linux's version for x64. Not sure if this should be added, or if this is a .NET packaging issue that will be fixed. (EDIT: In 6.0.103, need to use Linux versions for osx-arm64 too) e.g.

    Patch (v6.0.102) 
    diff --git a/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj b/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
index 0a2fcff17..9033ff11a 100644
--- a/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
+++ b/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
@@ -23,6 +23,14 @@
     <PackageReference Include="runtime.linux-x64.Microsoft.NETCore.ILDAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILDAsmVersion)" />
     <PackageReference Include="runtime.linux-x64.Microsoft.NETCore.TestHost" Version="$(RuntimeLinuxX64MicrosoftNETCoreTestHostVersion)" />
     <PackageReference Include="runtime.linux-x64.runtime.native.System.IO.Ports" Version="$(RuntimeLinuxX64RuntimeNativeSystemIOPortsVersion)" />
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.ILAsm" Version="$(RuntimeOsxArm64MicrosoftNETCoreILAsmVersion)" />
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.ILDAsm" Version="$(RuntimeOsxArm64MicrosoftNETCoreILDAsmVersion)" />
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.TestHost" Version="$(RuntimeOsxArm64MicrosoftNETCoreTestHostVersion)" />
+    <PackageReference Include="runtime.osx-arm64.runtime.native.System.IO.Ports" Version="$(RuntimeOsxArm64RuntimeNativeSystemIOPortsVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.ILAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILAsmVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.ILDAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILDAsmVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.TestHost" Version="$(RuntimeLinuxX64MicrosoftNETCoreTestHostVersion)" />
+    <PackageReference Include="runtime.osx-x64.runtime.native.System.IO.Ports" Version="$(RuntimeLinuxX64RuntimeNativeSystemIOPortsVersion)" />
    </ItemGroup>

    <Target Name="BuildBoostrapPreviouslySourceBuilt" AfterTargets="Restore">
    Patch (v6.0.103) 
    diff --git a/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj b/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
index 14921a48f..3a34e8749 100644
--- a/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
+++ b/src/SourceBuild/tarball/content/scripts/bootstrap/buildBootstrapPreviouslySB.csproj
@@ -33,6 +33,14 @@
     <!-- There's no nuget package for runtime.linux-musl-x64.runtime.native.System.IO.Ports
     <PackageReference Include="runtime.linux-musl-x64.runtime.native.System.IO.Ports" Version="$(RuntimeLinuxX64RuntimeNativeSystemIOPortsVersion)" />
     -->
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.ILAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILAsmVersion)" />
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.ILDAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILDAsmVersion)" />
+    <PackageReference Include="runtime.osx-arm64.Microsoft.NETCore.TestHost" Version="$(RuntimeLinuxX64MicrosoftNETCoreTestHostVersion)" />
+    <PackageReference Include="runtime.osx-arm64.runtime.native.System.IO.Ports" Version="$(RuntimeLinuxX64RuntimeNativeSystemIOPortsVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.ILAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILAsmVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.ILDAsm" Version="$(RuntimeLinuxX64MicrosoftNETCoreILDAsmVersion)" />
+    <PackageReference Include="runtime.osx-x64.Microsoft.NETCore.TestHost" Version="$(RuntimeLinuxX64MicrosoftNETCoreTestHostVersion)" />
+    <PackageReference Include="runtime.osx-x64.runtime.native.System.IO.Ports" Version="$(RuntimeLinuxX64RuntimeNativeSystemIOPortsVersion)" />
   </ItemGroup>

   <Target Name="BuildBoostrapPreviouslySourceBuilt" AfterTargets="Restore">

  • Error MSB4018 while building 'installer in tarball' due to trying to find aspnetcore-runtime-internal v6.0.0 rather than current. Need to find out why this is happening. The hack done is change MicrosoftAspNetCoreAppRuntimePackageVersion to MicrosoftAspNetCoreAppRuntimewinx64PackageVersion inside src/installer.*/src/redist/targets/GenerateLayout.targets

  • Needed to introduce GNU sed rather than default BSD sed. Not too important and can keep as build dependency, but may want to document if it remains hard requirement. The problem is due to sed -i usage in https://github.com/dotnet/arcade/blob/main/eng%2FSourceBuild.props#L38

  • Temporary issue but need to manually remove src/nuget-client.*/eng/source-build-patches/0001-Rename-NuGet.Config*.patch since macOS is case-insensitive OS. Would be good to be able to identify patches that are platform-compatible and have source-build skip them. Otherwise, Homebrew can continue removing incompatible patches as necessary.
@dotnet-issue-labeler dotnet-issue-labeler bot added area-patch-removal Removing patches for contributing repos from source-build untriaged labels Mar 9, 2022
@cho-m cho-m mentioned this issue Mar 9, 2022
Closed
6 tasks
@cho-m cho-m changed the title [Homebrew] .NET 6 build patches/workarounds [Homebrew/macOS] .NET 6 build patches/workarounds Mar 9, 2022
@MichaelSimons
Copy link
Member

[Triage] @crummel, please take a look at these, comment which ones should be pushed upstream and note if there are any related tracking issues.

@crummel
Copy link
Contributor

crummel commented Mar 24, 2022

  • Build failure on macOS ARM due to osx-x64 override. The following is the patch we use to allow using osx-arm64. If acceptable, I can create PR in dotnet/installer:

This looks good to me as an installer patch.

  • Build failure on macOS due to missing ILAsm/ILDAsm. This seems to be common issue on Linux too. The odd thing though is that RuntimeOsxArm64MicrosoftNETCoreILAsmVersion exists but RuntimeOsxX64MicrosoftNETCoreILAsmVersion doesn't, so needed to use Linux's version for x64. Not sure if this should be added, or if this is a .NET packaging issue that will be fixed. (EDIT: In 6.0.103, need to use Linux versions for osx-arm64 too) e.g.

The missing package is probably because we aren't building for OSX, so the packages never get fed into the previously-source-built set of packages. I believe this means you'll be using prebuilt versions of ILAsm/ILDAsm in at least your first build; after that you can save off the produced packages and use them for future builds (this is similar to what Fedora does). In the (very-) long-term we will probably build on OSX again but we don't have the bandwidth to look at it right now. So in summary I don't think you need this as a permanent patch if you're going to be collecting the packages and managing your own previously-source-built. Otherwise I think this is fine to upstream to installer.

  • Error MSB4018 while building 'installer in tarball' due to trying to find aspnetcore-runtime-internal v6.0.0 rather than current. Need to find out why this is happening. The hack done is change MicrosoftAspNetCoreAppRuntimePackageVersion to MicrosoftAspNetCoreAppRuntimewinx64PackageVersion inside src/installer.*/src/redist/targets/GenerateLayout.targets
    • EDIT 1: May not be needed anymore in 6.0.103
    • EDIT 2: Actual, based on Homebrew PR, still needed on osx-x64 as looking for v6.0.2.

This is interesting and I wonder if it's another RID-related problem. ASP.NET was updated in 6.0.103 so it's odd that it's looking for 6.0.2. If you have a copy of your artifacts/obj/<arch>/Release/PackageVersions.props.pre.installer.xml handy that might be interesting to look at.

We're also fine with changing this to BSD sed syntax (i.e. -ibak) since it's still compatible with GNU sed. This could possibly be considered a regression actually, we used to do it in 3.1.

  • Temporary issue but need to manually remove src/nuget-client.*/eng/source-build-patches/0001-Rename-NuGet.Config*.patch since macOS is case-insensitive OS. Would be good to be able to identify patches that are platform-compatible and have source-build skip them. Otherwise, Homebrew can continue removing incompatible patches as necessary.

Yes, this is a bad patch and I hope to remove it soon - the PR is dotnet/arcade#7549.

Thanks for finding these issues and writing them up! Let us know if you run into any more issues and feel free to ping me or dotnet/source-build-internal if you open any PRs.

@MichaelSimons MichaelSimons moved this to 6.0 BackLog in .NET Source Build Mar 24, 2022
@crummel
Copy link
Contributor

crummel commented Mar 31, 2022

Hi @cho-m, would you mind opening a separate issue for the RID-related stuff (the ASP.NET and ILAsm versions)? Then after the rest of the PRs are up we can close this one and chase those down separately. Thanks!

@MichaelSimons MichaelSimons moved this from 6.0 BackLog to 7.0 Backlog in .NET Source Build Apr 28, 2022
@MichaelSimons MichaelSimons moved this from 7.0 Backlog to 8.0 Backlog in .NET Source Build Sep 7, 2022
@crummel
Copy link
Contributor

crummel commented Sep 8, 2022

Hi @cho-m, have you had any time to look at this? Thanks again for tracking these issues down!

@cho-m
Copy link
Author

cho-m commented Oct 17, 2022

Sorry I haven't had much time to respond. I'll try to work on creating PRs and separate issues by this week or next.

I will note that GNU vs BSD sed issue is probably fixed in 7.0.0 with dotnet/arcade@b8007ee

While there is a new issue with dotnet-install script on 6.0 -- dotnet/install-scripts#318

@cho-m
Copy link
Author

cho-m commented Nov 10, 2022 •
edited
Loading

One thing for dotnet/installer patch, what is the original value of TargetRid on macOS? Is it possible to just skip the OverrideTargetRid like Linux instead?

EDIT: Just tried it and I see why. It has macOS version number (osx.13-arm64) which causes issues.

@cho-m cho-m changed the title [Homebrew/macOS] .NET 6 build patches/workarounds [Homebrew/macOS] .NET 6 and 7 build patches/workarounds Dec 7, 2022
@MichaelSimons MichaelSimons moved this from Needs Review to Backlog in .NET Source Build Jun 8, 2023
This was referenced Jun 23, 2023
Closed
Closed
@richlander
Copy link
Member

Perhaps we should have a call on this to see if we can come up with a plan.

@cho-m @fel1x-developer @asbjornu

Sound good?

@mmitche
Copy link
Member

mmitche commented Sep 13, 2023

I'd like to be included too!

@directhex directhex mentioned this issue Sep 19, 2023
Merged
@MichaelSimons
Copy link
Member

The need to remove the src/nuget-client.*/eng/source-build-patches/0001-Rename-NuGet.Config*.patch is being addressed in NuGet/NuGet.Client#5414.

@MichaelSimons MichaelSimons moved this from Backlog to In Progress in .NET Source Build Sep 20, 2023
@MichaelSimons MichaelSimons self-assigned this Sep 20, 2023
@MichaelSimons
Copy link
Member

All issues in the original write-up have been fixed in .NET 8.0. Closing.

@github-project-automation github-project-automation bot moved this from In Progress to Done in .NET Source Build Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MichaelSimons MichaelSimons

Labels
area-patch-removal Removing patches for contributing repos from source-build
Projects
.NET Source Build
Archived in project
Milestone
No milestone
Development

No branches or pull requests
5 participants
@richlander @crummel @MichaelSimons @mmitche @cho-m