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

[WIP] source-build: don't skip building Native Aot artifacts. #88520

Closed
wants to merge 4 commits into from
Closed

[WIP] source-build: don't skip building Native Aot artifacts. #88520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9d2e46
source-build: don't skip building Native Aot artifacts.
tmds Jul 7, 2023
0bd9415
Add LinkerArgs for openssl only for Linux target.
tmds Jul 7, 2023
3024212
Force portable OpenSSL when NativeAOT.
tmds Jul 11, 2023
ebe9520
ILCompiler: remove unneeded NativeAotSupported Condition.
tmds Aug 16, 2023
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
1 change: 1 addition & 0 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@
<NoPgoOptimize Condition="'$(NoPgoOptimize)' == '' and '$(DotNetBuildFromSource)' == 'true'">true</NoPgoOptimize>
<EnableNgenOptimization Condition="'$(EnableNgenOptimization)' == '' and '$(DotNetBuildFromSource)' == 'true'">false</EnableNgenOptimization>
<EnableNgenOptimization Condition="'$(EnableNgenOptimization)' == '' and ('$(Configuration)' == 'Release' or '$(Configuration)' == 'Checked')">true</EnableNgenOptimization>
<NativeAotSupported Condition="('$(TargetOS)' == 'windows' or '$(TargetOS)' == 'linux' or '$(TargetOS)' == 'osx' or '$(TargetOS)' == 'maccatalyst' or '$(TargetOS)' == 'iossimulator' or '$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvossimulator' or '$(TargetOS)' == 'tvos' or '$(TargetOS)' == 'freebsd') and ('$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'arm64')">true</NativeAotSupported>
<!-- Enable NuGet static graph evaluation to optimize incremental restore -->
<RestoreUseStaticGraphEvaluation>true</RestoreUseStaticGraphEvaluation>
<!-- Turn off end of life target framework checks as we intentionally build older .NETCoreApp configurations. -->
Expand Down
3 changes: 0 additions & 3 deletions eng/Subsets.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@
</PropertyGroup>

<PropertyGroup>
<!-- CLR NativeAot only builds in a subset of the matrix -->
<NativeAotSupported Condition="('$(TargetOS)' == 'windows' or '$(TargetOS)' == 'linux' or '$(TargetOS)' == 'osx' or '$(TargetOS)' == 'maccatalyst' or '$(TargetOS)' == 'iossimulator' or '$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvossimulator' or '$(TargetOS)' == 'tvos' or '$(TargetOS)' == 'freebsd') and ('$(TargetArchitecture)' == 'x64' or '$(TargetArchitecture)' == 'arm64')">true</NativeAotSupported>

<!-- If we're building clr.nativeaotlibs and not building the CLR runtime, compile libraries against NativeAOT CoreLib -->
<UseNativeAotCoreLib Condition="'$(TestNativeAot)' == 'true' or ($(_subset.Contains('+clr.nativeaotlibs+')) and !$(_subset.Contains('+clr.native+')) and !$(_subset.Contains('+clr.runtime+')))">true</UseNativeAotCoreLib>
</PropertyGroup>
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ The .NET Foundation licenses this file to you under the MIT license.
<LinkerArg Include="-static-pie" Condition="'$(StaticExecutable)' == 'true' and '$(PositionIndependentExecutable)' != 'false'" />
<LinkerArg Include="-dynamiclib" Condition="'$(_IsApplePlatform)' == 'true' and '$(NativeLib)' == 'Shared'" />
<LinkerArg Include="-shared" Condition="'$(_IsApplePlatform)' != 'true' and '$(NativeLib)' == 'Shared'" />
<!-- source-build links directly with openssl (-lssl -lcrypto) -->
Copy link
Member

Choose a reason for hiding this comment

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

I assume that you will want to have libicu in this list as well. Is that correct?

I do not think that we want to pass in these libraries to the linker by default. We will need to have a new property for non-portable build output and set it as necessary.

What do you want the default for dotnet publish to be in distro-provided dotnet? Do you want it to produce the non-portable output default or portable output by default? This is general question for all targets, not specific to native AOT. Does it produce portable or non-portable output today?

Copy link
Member

@am11 am11 Jul 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In the first PropertyGroup, we can explicitly define an internal property:

<_NonPortableBuild Condition="'$(NETCoreSdkRuntimeIdentifier)' != '$(NETCoreSdkPortableRuntimeIdentifier)'">true</_NonPortableBuild>

After that, make decisions based on that property: Condition="'$(_NonPortableBuild)' == 'true'"; without locking down to linux.

Another problem with the current approach is that it disregards static / customization options: https://github.com/dotnet/runtime/blob/60799cc5705fce35d04612dc24bdba46ed5a4f0e/src/coreclr/nativeaot/docs/compiling.md#using-statically-linked-icu

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you want the default for dotnet publish to be in distro-provided dotnet? Do you want it to produce the non-portable output default or portable output by default? This is general question for all targets, not specific to native AOT. Does it produce portable or non-portable output today?

Since non-portable and portable have different traits, we want this to be a deliberate choice by the user.

Similar to choosing between the .NET bundled runtime packs or the portable Microsoft NuGet packages, the UX for this is the user specify the non-portable rid (instead of the portable rid).

We should then add the linker flags based on the selected runtime.

Copy link
Member Author

@tmds tmds Jul 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I assume that you will want to have libicu in this list as well. Is that correct?

I didn't need to add it to get this compiled on my machine, and ldd doesn't show it on my Fedora .NET .so files.
source-build may be using icu in the same way as the portable builds.

Copy link
Member Author

Choose a reason for hiding this comment

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

Similar to choosing between the .NET bundled runtime packs or the portable Microsoft NuGet packages, the UX for this is the user specify the non-portable rid (instead of the portable rid).

I'm taking a look at the packages involved in a native aot publish using the preview 5 SDK.
I see Microsoft.DotNet.ILCompiler which is the tooling, and runtime.linux-x64.Microsoft.DotNet.ILCompiler which contains the runtime.

For source-build, the first should be identical to the Microsoft one.
And for the second, we want to produce a non-portable rid package, for example: runtime.fedora.38-x64.Microsoft.DotNet.ILCompiler

The extra linker flags describe what is in the second package, so we should include them in that package when it gets built from source.

The first package then needs a mechanism to pick those up.

@jkotas does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

The split between RID neutral and RID specific packages is only a download size optimization. It is not designed to allow composing RID specific packages of different origin.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the SDK finds the rid specific package explicitly based on the information from BundledVersions.props which we control for source-build:

    <KnownILCompilerPack Include="Microsoft.DotNet.ILCompiler"
                         TargetFramework="net8.0"
                         ILCompilerPackNamePattern="runtime.**RID**.Microsoft.DotNet.ILCompiler"
                         ILCompilerPackVersion="8.0.0-preview.5.23280.8"
                         ILCompilerRuntimeIdentifiers="linux-musl-x64;linux-x64;win-x64;linux-arm;linux-arm64;linux-musl-arm;linux-musl-arm64;osx-arm64;osx-x64;win-arm;win-arm64;win-x86"
                         />

Copy link
Member

Choose a reason for hiding this comment

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

Yes and ILCompilerRuntimeIdentifiers has hardcoded finite lists of RIDs. What happens for RIDs outside this hardcoded list?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and ILCompilerRuntimeIdentifiers has hardcoded finite lists of RIDs. What happens for RIDs outside this hardcoded list?

For some of the lists it will use the runtime graph to find the most appropriate rid from the finite set.

It's clear some changes are needed for the SDK, Microsoft.DotNet.ILCompiler, and runtime.<rid>.Microsoft.DotNet.ILCompiler to work together when built from source.

I'm going to avoid the linker flags issue for now by making the build use the openssl portable dlopen when source-build and nativeaot are combined, and see what else comes up.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to avoid the linker flags issue for now by making the build use the openssl portable dlopen when source-build and nativeaot are combined, and see what else comes up.

Added 3024212 for this.

<LinkerArg Include="-lssl" />
<LinkerArg Include="-lcrypto" />
Copy link
Member Author

@tmds tmds Jul 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This relates to #66859.

I've unconditionally added these flags because there is no condition that tells us if the build links with OpenSSL (source-build non-portable) or loads it dynamically (dlopen).

In the latter case the flags are not needed. Perhaps they don't cause issues either (on platforms where .NET dlopens OpenSSL).

<!-- binskim warning BA3001 PIE disabled on executable -->
<LinkerArg Include="-pie -Wl,-pie" Condition="'$(_IsApplePlatform)' != 'true' and '$(NativeLib)' == '' and '$(StaticExecutable)' != 'true' and '$(PositionIndependentExecutable)' != 'false'" />
<LinkerArg Include="-Wl,-no-pie" Condition="'$(_IsApplePlatform)' != 'true' and '$(NativeLib)' == '' and '$(StaticExecutable)' != 'true' and '$(PositionIndependentExecutable)' == 'false'" />
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
<!-- BEGIN: Workaround for https://github.com/dotnet/runtime/issues/67742 -->
<PropertyGroup Condition="'$(BuildingInsideVisualStudio)' != 'true'">
<PublishDir>$(RuntimeBinDir)ilc-published/</PublishDir>
<!-- Can't use NativeAOT in source build yet https://github.com/dotnet/runtime/issues/66859 -->
<NativeAotSupported Condition="'$(DotNetBuildFromSource)' == 'true'">false</NativeAotSupported>
<!-- Disable native AOT on FreeBSD when cross building from Linux. -->
<NativeAotSupported Condition="'$(TargetOS)' == 'freebsd' and '$(CrossBuild)' == 'true'">false</NativeAotSupported>
<PublishAot Condition="'$(NativeAotSupported)' == 'true'">true</PublishAot>
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/tools/aot/ILCompiler/ILCompiler.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@

<!-- CoreDisTools are used in debugging visualizers. -->
<IncludeCoreDisTools Condition="'$(Configuration)' != 'Release' and '$(CrossHostArch)' == ''">true</IncludeCoreDisTools>
<!-- source-build doesn't use ObjWriter for the ILCompiler. the end-user will end up pulling Microsoft-built bits for NativeAOT anyway. -->
<IncludeObjWriter Condition="'$(DotNetBuildFromSource)' != 'true'">true</IncludeObjWriter>
<IncludeObjWriter Condition="'$(NativeAotSupported)' == 'true'">true</IncludeObjWriter>
tmds marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<Import Project="$(RepositoryEngineeringDir)coredistools.targets" Condition="'$(DotNetBuildFromSource)' != 'true' and '$(IncludeCoreDisTools)' == 'true'" />
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/tools/aot/crossgen2/crossgen2.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

<PropertyGroup>
<OutputPath>$(RuntimeBinDir)crossgen2</OutputPath>
<!-- Can't use NativeAOT in source build yet https://github.com/dotnet/runtime/issues/66859 -->
<NativeAotSupported Condition="'$(DotNetBuildFromSource)' == 'true'">false</NativeAotSupported>
<!-- Trimming is not currently working, but set the appropriate feature flags for NativeAOT -->
<PublishTrimmed Condition="'$(NativeAotSupported)' == 'true'">true</PublishTrimmed>
<RuntimeIdentifiers Condition="'$(_IsPublishing)' != 'true' and '$(DotNetBuildFromSource)' != 'true'">linux-x64;linux-musl-x64;linux-arm;linux-musl-arm;linux-arm64;linux-musl-arm64;freebsd-x64;freebsd-arm64;osx-x64;osx-arm64;win-x64;win-x86;win-arm64</RuntimeIdentifiers>
Expand Down
2 changes: 0 additions & 2 deletions src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
<ShouldVerifyClosure>false</ShouldVerifyClosure>
<!-- Publish crossgen2 as a single-file app on native-OS builds. Cross-OS NativeAOT compilation is not supported yet -->
<NativeAotSupported Condition="'$(CrossBuild)' == 'true' and '$(TargetOS)' != '$(HostOS)'">false</NativeAotSupported>
<!-- Can't use NativeAOT in source build yet https://github.com/dotnet/runtime/issues/66859 -->
<NativeAotSupported Condition="'$(DotNetBuildFromSource)' == 'true'">false</NativeAotSupported>
</PropertyGroup>

<Target Name="PublishCrossgen"
Expand Down
Loading