source-built NativeAOT doesn't work with system brotli

[tmds]

When source-building .NET with the brotli system lib, the source-built NativeAOT fails to compile an application that uses brotli.

The compilation failure is due to missing flags for brotli.

  console failed with 2 error(s) (12.2s) → bin/Release/net9.0/fedora.40-x64/console.dll
    clang : error : linker command failed with exit code 1 (use -v to see invocation)
    /var/home/tmds/.nuget/packages/microsoft.dotnet.ilcompiler/9.0.0-rc.2.24423.10/build/Microsoft.NETCore.Native.targets(376,5): error MSB3073: The command ""clang" "obj/Release/net9.0/fedora.40-x64/native/console.o" -o "bin/Release/net9.0/fedora.40-x64/native/console" -Wl,--version-script=obj/Release/net9.0/fedora.40-x64/native/console.exports -Wl,--export-dynamic -gz=zlib -fuse-ld=bfd /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libbootstrapper.o /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libRuntime.WorkstationGC.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libeventpipe-disabled.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libRuntime.VxsortEnabled.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libstandalonegc-disabled.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/sdk/libstdc++compat.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/framework/libSystem.Native.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/framework/libSystem.Globalization.Native.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/framework/libSystem.IO.Compression.Native.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/framework/libSystem.Net.Security.Native.a /tmp/dotnet/packs/runtime.fedora.40-x64.Microsoft.DotNet.ILCompiler/9.0.0-rc.2.24423.10/framework/libSystem.Security.Cryptography.Native.OpenSsl.a -g -Wl,-rpath,'$ORIGIN' -Wl,--build-id=sha1 -Wl,--as-needed -pthread -lssl -lcrypto -ldl -lz -lrt -lm -pie -Wl,-pie -Wl,-z,relro -Wl,-z,now -Wl,--eh-frame-hdr -Wl,--discard-all -Wl,--gc-sections" exited with code 1.

In #97536, we've added such flags for OpenSSL:

runtime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets

Lines 166 to 169 in 9a31a5b

	<ItemGroup Condition="'$(StaticOpenSslLinking)' != 'true' and Exists('$(IlcSdkPath)nonportable.txt')">
	<NativeSystemLibrary Include="ssl" />
	<NativeSystemLibrary Include="crypto" />
	</ItemGroup>

We need to additionally have:

      <NativeSystemLibrary Include="brotlienc" />
      <NativeSystemLibrary Include="brotlidec" />

.NET can be either built with system brotli or the bundled broti. The above is only needed when built with system brotli.

Microsoft.NETCore.Native.Unix.targets is in the Microsoft.DotNet.ILCompiler package, which means it may come from nuget.org, so we can't just patch it as part of source-building .NET.

For the OpenSSL flags, #97536 introduced a nonportable.txt breadcrumb that is stored in the source-built .NET so Microsoft.DotNet.ILCompiler knows when to include the OpenSSL flags.

For brotli, we need something similar.

If we can consider brotli an acceptable system dependency for source-build platforms, we may consider to have non-portable builds always use brotli and use the nonportable.txt breadcrumb.

cc @jkotas @omajid @MichaelSimons

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[jkotas]

@jkoritzinsky Do you expect that we will start including separate brotli libs for native aot package as part of #106887 (comment) ?

If we can consider brotli an acceptable system dependency for source-build platforms, we may consider to have non-portable builds always use brotli and use the nonportable.txt breadcrumb.

I do not see a problem with it, in the meantime at least.

[tmds]

in the meantime at least.

Yes, this would be the simple option that works for .NET 9.

@dotnet/distro-maintainers @MichaelSimons @omajid do you have feedback on source-built .NET to require the system to provide the brotli library (rather than using the one bundled with .NET). This would be a problem if your distro does not include brotli.

[omajid]

All the targets I care about (RHEL 8 +, Fedora) provide a system brotli.

[ayakael]

Alpine Linux also provides a system brotli (v1.1.0)

[corngood]

do you have feedback on source-built .NET to require the system to provide the brotli library

nixpkgs has it, so no problem there.

[MichaelSimons]

I have no objection to depending on the system library. We should document how to configure source-build to use the bundled version.

[omajid]

We should document how to configure source-build to use the bundled version

I think that's the issue here: with NativeAOt we don't really have a proper mechanism to do this (outside of patching source code/build-scripts).

[tmds]

Yes, the suggestion is to require system brotli for non-portable without providing an option to use the bundled one.

.NET providing bundled sources for libraries is nice, but it's common for package maintainers to get their dependencies from system packages. Not providing a bundled option is not as limiting as it may seem.

[jkoritzinsky]

@jkoritzinsky Do you expect that we will start including separate brotli libs for native aot package as part of #106887 (comment) ?

If we can consider brotli an acceptable system dependency for source-build platforms, we may consider to have non-portable builds always use brotli and use the nonportable.txt breadcrumb.

I do not see a problem with it, in the meantime at least.

Yes, I'd like to start including the brotli libs separately like we do for zlib. We could still support using the system libs for non-portable scenarios even in that case.

[agocke]

I need more details to consider a NET 9.0 backport here. This would be an ask mode change so we need details on the impact, risk, and potential alternatives.

[omajid]

@mateusrodrigues @dviererbe Are you folks comfortable using system brotli on Ubuntu?

[mateusrodrigues]

@omajid yes, Ubuntu provides brotli so that would be the preferable approach.

[jkoritzinsky]

In #107166, I'm updating how we consume our brotli dependency to be more similar to how we consume zlib-ng. As a result, the NativeAOT SDK support for consuming brotli will match how we designed the zlib-ng consumption (using system brotli when the NativeAOT target RID's libraries does not provide brotli).

I believe this approach will provide more flexibility for distros (as well as being quite easy to implement). Let me know what you think!

[tmds]

using system brotli when the NativeAOT target RID's libraries does not provide brotli.

@jkoritzinsky this sounds exactly like what we need: a mechanism that enables us to know what system libraries a NativeAOT target RID needs from the system.

This would be the simple option that works for .NET 9.

In code, it is something like:

diff --git a/eng/DotNetBuild.props b/eng/DotNetBuild.props
index 3569781b9af..ce13d6ca5b0 100644
--- a/eng/DotNetBuild.props
+++ b/eng/DotNetBuild.props
@@ -88,7 +88,7 @@
 
       <!-- Handle system libraries -->
       <UseSystemLibs Condition="'$(UseSystemLibs)' != ''">+$(UseSystemLibs)+</UseSystemLibs>
-      <InnerBuildArgs Condition="$(UseSystemLibs.Contains('+brotli+'))">$(InnerBuildArgs) --cmakeargs -DCLR_CMAKE_USE_SYSTEM_BROTLI=true</InnerBuildArgs>
+      <InnerBuildArgs Condition="'$(PortableBuild)' != 'true'">$(InnerBuildArgs) --cmakeargs -DCLR_CMAKE_USE_SYSTEM_BROTLI=true</InnerBuildArgs>
       <InnerBuildArgs Condition="$(UseSystemLibs.Contains('+libunwind+'))">$(InnerBuildArgs) --cmakeargs -DCLR_CMAKE_USE_SYSTEM_LIBUNWIND=true</InnerBuildArgs>
       <!-- TODO: llvm-libunwind -->
       <!-- TODO: LinuxTracepoints -->
diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
index d2696ffbcad..2f3a44a6317 100644
--- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
+++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
@@ -166,6 +166,8 @@ The .NET Foundation licenses this file to you under the MIT license.
     <ItemGroup Condition="'$(StaticOpenSslLinking)' != 'true' and Exists('$(IlcSdkPath)nonportable.txt')">
       <NativeSystemLibrary Include="ssl" />
       <NativeSystemLibrary Include="crypto" />
+      <NativeSystemLibrary Include="brotlienc" />
+      <NativeSystemLibrary Include="brotlidec" />
     </ItemGroup>
 
     <ItemGroup Condition="'$(StaticOpenSslLinking)' == 'true' and '$(NativeLib)' != 'Static'">

I need more details to consider a NET 9.0 backport here. This would be an ask mode change so we need details on the impact, risk, and potential alternatives.

@agocke for the above change, this is my assessment:

Risk/Impact:

I think we consider the risk for the end-user low as the change only impacts source-build NativeAOT and not the default portable NativeAOT experience. (Source-built NativeAOT is a new feature with .NET 9 and used when the user specifies the source-built RID when publishing their app with NativeAOT.)

A (distro) .NET maintainer will need to change their build so it provides the system brotli development package. Without the system brotli development package, .NET won't build (when built using a non-portable SDK).

@dotnet/distro-maintainers If you would like to check if .NET builds fine with system brotli, you can install the brotli development packages, and pass the following argument to the vmr build script: --with-system-libs brotli.

[tmds]

In #107166, I'm updating how we consume our brotli dependency to be more similar to how we consume zlib-ng. As a result, the NativeAOT SDK support for consuming brotli will match how we designed the zlib-ng consumption (using system brotli when the NativeAOT target RID's libraries does not provide brotli).

@jkoritzinsky We'd like to have this working for .NET 9. Is this PR something that is likely to be backported?

[jkoritzinsky]

I don't know if this PR will meet the .NET 9 bar. I purposefully split the brotli upgrade as I wasn't sure if changing how we consume brotli would be acceptable this close to the .NET 9 release.

[agocke]

I think we should take a change to just use nonportable.txt for .NET 9. I'm not sure the brotli source code refactor meets the bar.

[agocke]

I took a short at this here: #107225