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.

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.1xx] Fix build against musl #13410

Closed
wants to merge 1 commit into from

Conversation

ayakael
Copy link

@ayakael ayakael commented Mar 14, 2022

Description

Proper implementation of changes discussed in dotnet/source-build#2782

Includes aspnetcore patch that is a workaround till dotnet/aspnetcore#37400 is addressed.

Related PRs

dotnet/command-line-api#1673

Made as part of Alpine Linux dotnet6 packaging project, see dotnet/source-build#2782

@ayakael ayakael requested a review from a team as a code owner March 14, 2022 02:11
@ayakael
Copy link
Author

ayakael commented Mar 14, 2022

problem: I thought my implementation of OSPlatformIsMuslCheck worked, but alas it doesn't. Can't have elements in Directory.Build.props, either. Anyways have any pointers to check if building with musl without using ldd?

@ayakael
Copy link
Author

ayakael commented Mar 15, 2022

@omajid My understanding of Directory.Build.props syntax is rather limited. I'm trying to integrate your OSPlatformIsMuslCheck logic from dotnet/source-build-reference-packages#318 so that OSPlatformIsMusl is true globally, but as <Exec> can only exist within <Target>, so I added

  <Target Name="GetOSPlatformIsMuslCheck">
    <Exec IgnoreExitCode="true" Command="ldd --version 2&gt;&amp;1 | grep -q musl">
      <Output TaskParameter="ExitCode" PropertyName="OSPlatformIsMuslCheck" />
    </Exec>
    <PropertyGroup>
      <OSPlatformIsMusl Condition="$(OSPlatformIsMuslCheck) == '0'">true</OSPlatformIsMusl>
    </PropertyGroup>
  </Target>

in Directory.Build.props, and set <Project InitialTargets="GetOSPlatformIsMuslCheck"> at the header. I see

Target "GetOSPlatformIsMuslCheck" in file "/var/build/dotnet6/testing/dotnet6-bootstrap/src/source-build-tarball-6.0.102/Directory.Build.props" from project "/var/build/dotnet6/testing/dotnet6-bootstrap/src/source-build-tarball-6.0.102/repos/aspnetcore.proj" (entry point):                                                              
Task "Exec"                                                                                                                                                                        
  Task Parameter:Command=ldd --version 2>&1 | grep -q musl
  Task Parameter:IgnoreExitCode=True                                                                                                       
  ldd --version 2>&1 | grep -q musl
  Output Property: OSPlatformIsMuslCheck=0                             
Done executing task "Exec".                                              
Set Property: OSPlatformIsMusl=true                                                                                                              
Task "Message"                                                                                                                                                                     
  Task Parameter:Text=OSPlatformIsMusl = true                                            
  OSPlatformIsMusl = true                                                                                                                  
Done executing task "Message".
Done building target "GetOSPlatformIsMuslCheck" in project "aspnetcore.proj". 

But, within aspnetcore.proj <BuildCommandArgs Condition="'$(OSPlatformIsMusl)' == 'true'">$(BuildCommandArgs) --os-name linux-musl</BuildCommandArgs> isn't applied....

@omajid
Copy link
Member

omajid commented Apr 12, 2022

I think this is a timing ordering issue. The property values are read first, and then the task runs to set the property to the correct value.

@crummel @lbussell @MichaelSimons any suggestions on how we can initialize properties that depend on values we get by running a target?

@MichaelSimons
Copy link
Member

I think this is a timing ordering issue. The property values are read first, and then the task runs to set the property to the correct value.

How about defining a InitTargetRid task that encapsulates the logic necessary to compute the RIDs and invoke it as part of InitBuild in build.proj?

FYI - There is a conflicting RID related change that should be pulled in first.

@MichaelSimons
Copy link
Member

@ayakael - Is this something you are still able to contribute to?

@ayakael
Copy link
Author

ayakael commented Aug 19, 2022

@MichaelSimons To be honest trying to figure this out is, for me, like throwing spaguetti at the wall to see if it sticks. In other words, when it comes to .proj files and co, I don't know what I'm doing. So whatever implementation you might have will most certainly be more production ready than what I can propose.

As you can see, my last push is broken. Building installer gives me the following error:

/var/build/dotnet6/community/dotnet6-build/src/source-build-tarball-6.0.108/tools-local/init-build.proj : error MSB4025: The project file could not be loaded. Could not load file or assembly 'System.Security.Permissions, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.
/var/build/dotnet6/community/dotnet6-build/src/source-build-tarball-6.0.108/tools-local/init-build.proj : error MSB4025:

This is probably because I'm trying to write the InitTargetRid like one would write a property group, but indeed it's not working fo rme.

@MichaelSimons
Copy link
Member

@ayakael, I'll take a look and see if I can help out. Just to set expectations, it will more than likely take me a bit due to other priorities.

@ayakael
Copy link
Author

ayakael commented Aug 19, 2022

@MichaelSimons I appreciate that :) There is no rush, I've got a patch going for the Alpine package that works fine, it's just not portable as it forces musl build. Building with that patch on Fedora would force musl and break the build. This PR is simply to get this to an upstream state to help ya'll out for when Microsoft moves over to the community source-build (eventually).

@ayakael ayakael changed the title Fix build against musl [release/6.0.1xx] Fix build against musl Sep 11, 2022
@ayakael
Copy link
Author

ayakael commented Sep 26, 2022

WIth #14549, this is no longer necessary for source-build use-case. linux-musl-$arch is still broken though, due to OSName and HostOSName still defaulting to linux when it should be linux-musl on musl.

@MichaelSimons Is this use-case something that should be fixed?

@ayakael
Copy link
Author

ayakael commented Sep 27, 2022

Note that #14549 against rc1, the use-current-runtime test fails with error log:

^[[1;32m>>>^[[1;0m ^[[1;1mdotnet7-build^[[1;0m: Dumping logfile-use-current-runtime.log
<feff># Standard Output:                                                  
Executing /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/test.sh with arguments 7.0.0-rc.1.22426.10 in working directory /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime
FAIL: --use-current-runtime does not use portable rid.            
Exit Code: 1                                                                                                                                                        
# Standard Error:                                                 
++ ../runtime-id --portable                                       
+ PORTABLE_RID=linux-musl-x64                                     
++ dotnet build --use-current-runtime                                        
+ DOTNET_BUILD_OUTPUT='                                                           
Welcome to .NET 7.0!                                                                        
---------------------   
SDK Version: 7.0.100-rc.1.22477.1
                                                                                                                                                           
----------------
Installed an ASP.NET Core HTTPS development certificate.                                             
To trust the certificate run '\''dotnet dev-certs https --trust'\'' (Windows and macOS only).                                                                   
Learn about HTTPS: https://aka.ms/dotnet-https
----------------                                                         
Write your first app: https://aka.ms/dotnet-hello-world
Find out what'\''s new: https://aka.ms/dotnet-whats-new                        
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core                                                                                                                       
Use '\''dotnet --help'\'' to see available commands or visit: https://aka.ms/dotnet-cli                                                                                                       
--------------------------------------------------------------------------------------                                                                                              
MSBuild version 17.4.0-preview-22428-01+14c24b2d3 for .NET
  Determining projects to restore...                                           
  Restored /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj (in 1.59 sec).
  RuntimeIdentifier is linux-x64                                                                                                                                                              
  RuntimeIdentifier is linux-x64                                       
/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-bunny-71880bd94711519f7b786248a88a827a401207a2/release/sdk/7.0.100-rc.1.22477.1/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj]
  console -> /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/bin/Debug/net6.0/linux-x64/console.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:03.33'
+ echo + '
Welcome to .NET 7.0!
---------------------
SDK Version: 7.0.100-rc.1.22477.1

----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate run '\''dotnet dev-certs https --trust'\'' (Windows and macOS only).
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Use '\''dotnet --help'\'' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
MSBuild version 17.4.0-preview-22428-01+14c24b2d3 for .NET
  Determining projects to restore...
  Restored /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj (in 1.59 sec).
  RuntimeIdentifier is linux-x64
  RuntimeIdentifier is linux-x64
/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-bunny-71880bd94711519f7b786248a88a827a401207a2/release/sdk/7.0.100-rc.1.22477.1/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj]
  console -> /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/bin/Debug/net6.0/linux-x64/console.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:03.33'
+ echo + '
Welcome to .NET 7.0!
---------------------
SDK Version: 7.0.100-rc.1.22477.1

----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate run '\''dotnet dev-certs https --trust'\'' (Windows and macOS only).
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what'\''s new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use '\''dotnet --help'\'' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
MSBuild version 17.4.0-preview-22428-01+14c24b2d3 for .NET
  Determining projects to restore...
  Restored /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj (in 1.59 sec).
  RuntimeIdentifier is linux-x64
  RuntimeIdentifier is linux-x64
/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-bunny-71880bd94711519f7b786248a88a827a401207a2/release/sdk/7.0.100-rc.1.22477.1/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/console.csproj]
  console -> /builds/ayakael/aports/testing/dotnet7-build/src/dotnet-regular-tests-ddc8576cc4ffccf6b102eed55125b2596336e684/use-current-runtime/bin/Debug/net6.0/linux-x64/console.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:03.33'grep 
-q 'RuntimeIdentifier is linux-musl-x64'
+ echo 'FAIL: --use-current-runtime does not use portable rid.'
+ exit 1

Thus, there needs to be a fix implemented, but it'll be much smaller now that aspnetcore is non-portable.

@ayakael
Copy link
Author

ayakael commented Nov 13, 2022

Closing in favor of #14816

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants