Fix AOT warnings in Azure.Core #44136

live1206 · 2024-05-19T03:47:03Z

Resolves #44127

Change the usage of BinaryData from
https://github.com/dotnet/runtime/blob/3b95ae9494c6d1580b2ccbf06db7b04af1a265b1/src/libraries/System.Memory.Data/src/System/BinaryData.cs#L88-L92
to
https://github.com/dotnet/runtime/blob/3b95ae9494c6d1580b2ccbf06db7b04af1a265b1/src/libraries/System.Memory.Data/src/System/BinaryData.cs#L140
Change from
https://github.com/dotnet/runtime/blob/3b95ae9494c6d1580b2ccbf06db7b04af1a265b1/src/libraries/System.Memory.Data/src/System/BinaryData.cs#L424-L427
to
https://github.com/dotnet/runtime/blob/3b95ae9494c6d1580b2ccbf06db7b04af1a265b1/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.Parse.cs#L96-L99
To be able to use JsonNode, upgrade System.Text.Json to 6.0.0 and its dependencies to 6.0.0

The AOT warnings now running from https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp/test-aot-compat.ps1 are

ILC : Trim analysis warning IL2026: [Azure.Core]Azure.Core.Serialization.DynamicData: Using member 'Azure.Core.Serialization.DynamicData.DynamicDataJsonConverter.DynamicDataJsonConverter()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Using DynamicData or DynamicDataConverter is not compatible with trimming due to reflection-based serialization. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj::TargetFramework=net7.0]
ILC : Trim analysis warning IL2026: [Azure.Core]Azure.Core.Json.MutableJsonDocument: Using member 'Azure.Core.Json.MutableJsonDocument.MutableJsonDocumentConverter.MutableJsonDocumentConverter()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Using MutableJsonDocument or MutableJsonDocumentConverter is not compatible with trimming due to reflection-based serialization. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj::TargetFramework=net7.0]
ILC : AOT analysis warning IL3050: Microsoft.Extensions.DependencyInjection.ServiceLookup.IEnumerableCallSite.ServiceType.get: Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj::TargetFramework=net7.0]
/_/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/BinaryExpression.cs(2239): AOT analysis warning IL3050: System.Linq.Expressions.Expression.GetResultTypeOfShift(Type,Type): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj::TargetFramework=net7.0]
/_/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs(28): AOT analysis warning IL3050: System.Dynamic.Utils.TypeUtils.GetNullableType(Type): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp\Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj::TargetFramework=net7.0]
Actual warning count is: 5

Other options are:

Use JsonElement, which we need to traverse it every time we look up a value, not efficient as JsonObject.
Remove NextLinkOperationImplementation.cs from shared core, so that we can use internal members of RehdyrationToken in it directly
Make internal members in RehydrationToken public as in Make RehydrationToken members public #43549

azure-sdk · 2024-05-19T04:02:26Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

Azure.Core.Experimental
Azure.Messaging.WebPubSub.Client
Microsoft.Azure.Messaging.EventGrid.CloudNativeCloudEvents
Azure.Messaging.EventGrid
Azure.Security.Attestation
Azure.ResourceManager.Attestation
Azure.Messaging.EventGrid.Namespaces
Azure.ResourceManager.EventGrid

live1206 · 2024-05-20T03:49:51Z

@jsquire
The easiest fix I found is to use JsonNode as in this PR, which only exists in System.Text.Json >= 6.0.0.
After upgrading System.Text.Json and its dependencies System.Text.Encodings.Web to 6.0.0, got below build error, details

sdk/batch/Microsoft.Azure.Batch/src/Microsoft.Azure.Batch.csproj(0,0): Error NU1202: Package System.Text.Encodings.Web 6.0.0 is not compatible with net452 (.NETFramework,Version=v4.5.2). Package System.Text.Encodings.Web 6.0.0 supports:
  - net461 (.NETFramework,Version=v4.6.1)
  - net6.0 (.NETCoreApp,Version=v6.0)
  - netcoreapp3.1 (.NETCoreApp,Version=v3.1)
  - netstandard2.0 (.NETStandard,Version=v2.0)

Will we consider to upgrade the .net framework version for Microsoft.Azure.Batch since it's too old?

sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs

eng/Packages.Data.props

jsquire · 2024-05-20T15:13:42Z

eng/Packages.Data.props

@@ -99,10 +99,10 @@
    <PackageReference Update="System.Security.Cryptography.ProtectedData" Version="4.7.0" />
    <PackageReference Update="System.Threading.Channels" Version="4.7.1" />
    <PackageReference Update="System.Threading.Tasks.Extensions" Version="4.5.4" />
-    <PackageReference Update="System.Text.Json" Version="4.7.2" />
-    <PackageReference Update="System.Text.Encodings.Web" Version="4.7.2" />
+    <PackageReference Update="System.Text.Json" Version="6.0.0" />


The v6.x line are confirmed safe with Azure Functions and PowerShell.

jsquire · 2024-05-20T15:14:15Z

eng/Packages.Data.props

@@ -99,10 +99,10 @@
    <PackageReference Update="System.Security.Cryptography.ProtectedData" Version="4.7.0" />
    <PackageReference Update="System.Threading.Channels" Version="4.7.1" />
    <PackageReference Update="System.Threading.Tasks.Extensions" Version="4.5.4" />
-    <PackageReference Update="System.Text.Json" Version="4.7.2" />
-    <PackageReference Update="System.Text.Encodings.Web" Version="4.7.2" />
+    <PackageReference Update="System.Text.Json" Version="6.0.0" />


Let's move to the latest version in the 6.x line rather than 6.0.0 for all of these.

Updated System.Text.Json to 6.0.9, 6.0.0 is the only stable version for the other two.

jsquire · 2024-05-20T15:15:41Z

sdk/core/Azure.Core.Experimental/src/Azure.Core.Experimental.csproj

@@ -13,6 +13,7 @@
  <ItemGroup>
    <PackageReference Include="Azure.Core" />
    <PackageReference Include="Microsoft.CSharp" />
+    <PackageReference Include="System.Text.Json" />


This is most likely needed in the short-term due to the package bump. Please add a comment here documenting that and open an issue to remove this once the next version of Azure.Core is released.

Created #44165 for tracking

jsquire · 2024-05-20T15:17:10Z

sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryKeyVaultSecret.Serialization.cs

@@ -35,7 +35,7 @@ void IUtf8JsonSerializable.Write(Utf8JsonWriter writer)
            }
            DataFactoryLinkedServiceReference? store = default;
            DataFactoryElement<string>? secretName = default;
-            Optional<DataFactoryElement<string>> secretVersion = default;
+            DataFactoryElement<string>? secretVersion = default;


@JoshLove-msft: Would you offer thoughts on the data factory bits?

jsquire · 2024-05-20T15:17:50Z

sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj

@@ -11,6 +11,7 @@
    <PackageReference Include="NUnit3TestAdapter" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" />
    <PackageReference Include="Moq" />
+    <PackageReference Include="System.Text.Json" />


Same comment as for Experimental. Please track this via comment and issue, as this should be a short-term need.

jsquire · 2024-05-20T15:19:02Z

sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs

@@ -122,7 +126,7 @@ internal class NextLinkOperationImplementation : IOperation
                finalStateVia = OperationFinalStateVia.Location;
            }

-            string headerSourceStr = GetContentFromRehydrationToken(lroDetails, "headerSource");
+            string headerSourceStr = GetContentFromRehydrationToken(lroDetails, "headerSource")!;


If we're sure that the outcome here is not null, why is GetContentFromRehydrationToken marked as nullable?

Because the nullability depends on the original input, for instance, lastKnownLocation is nullable, we can still get null from RehydrationToken, other fields are not nullable we can ensure they are not null here.

jsquire · 2024-05-20T15:25:45Z

@jsquire The easiest fix I found is to use JsonNode as in this PR, which only exists in System.Text.Json >= 6.0.0. After upgrading System.Text.Json and its dependencies System.Text.Encodings.Web to 6.0.0, got below build error, details

The Azure.Core team has been discussing/wanting to bump the T2 references to the 6.x line for the framework packages, including System.Text.Json and have done our due diligence to ensure that they won't cause problems with the Functions in-process environment nor PowerShell. This is something that we've agreed that we want to do, but we haven't yet had a forcing function to make it a priority.

We'll need to do a very careful test pass, both CI and Live, for a large portion of the T2 packages across the data and management planes to ensure that the bump doesn't cause regressions. As long as we do that, we should be good with the version bumps.

Will we consider to upgrade the .net framework version for Microsoft.Azure.Batch since it's too old?

No, we cannot change the target frameworks for the T1 package. That said, we shouldn't be bumping the T1 package references. There's a reason why we keep two distinct sets. Let's revert the T1 section back which will ensure that Batch stays stable.

live1206 · 2024-05-21T09:47:28Z

After upgrading the packages, there is a version conflict error for Microsoft.Bcl.AsyncInterfaces. Right now Azure.Core is referencing Microsoft.Bcl.AsyncInterfaces.1.1.1, but the global reference is Microsoft.Bcl.AsyncInterfaces.6.0.0. Details

/opt/hostedtoolcache/dotnet/sdk/7.0.101/Microsoft.Common.CurrentVersion.targets(2352,5): Error MSB3277: Found conflicts between different versions of "Microsoft.Bcl.AsyncInterfaces" that could not be resolved.
There was a conflict between "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" and "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51".
    "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" was chosen because it was primary and "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" was not.
    References which depend on "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" [/mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll].
        /mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll
          Project file item includes which caused reference "/mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll".
            /mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll
    References which depend on "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" [].
        /mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll
          Project file item includes which caused reference "/mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll".
            /mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll

Feel like this is an issue for Microsoft.Bcl.AsyncInterfaces itself since the same scenario works fine for System.Text.Json.

Tried to release an alpha version of Azure.Core, everything works in #44172

live1206 · 2024-05-22T07:28:31Z

After upgrading the packages, there is a version conflict error for Microsoft.Bcl.AsyncInterfaces. Right now Azure.Core is referencing Microsoft.Bcl.AsyncInterfaces.1.1.1, but the global reference is Microsoft.Bcl.AsyncInterfaces.6.0.0. Details

/opt/hostedtoolcache/dotnet/sdk/7.0.101/Microsoft.Common.CurrentVersion.targets(2352,5): Error MSB3277: Found conflicts between different versions of "Microsoft.Bcl.AsyncInterfaces" that could not be resolved.
There was a conflict between "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" and "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51".
    "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" was chosen because it was primary and "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" was not.
    References which depend on "Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" [/mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll].
        /mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll
          Project file item includes which caused reference "/mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll".
            /mnt/vss/_work/1/.nuget/packages/microsoft.bcl.asyncinterfaces/1.1.1/ref/netstandard2.1/Microsoft.Bcl.AsyncInterfaces.dll
    References which depend on "Microsoft.Bcl.AsyncInterfaces, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51" [].
        /mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll
          Project file item includes which caused reference "/mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll".
            /mnt/vss/_work/1/s/artifacts/bin/Azure.ResourceManager.Attestation/Debug/netstandard2.0/Azure.ResourceManager.Attestation.dll

Feel like this is an issue for Microsoft.Bcl.AsyncInterfaces itself since the same scenario works fine for System.Text.Json.

Tried to release an alpha version of Azure.Core, everything works in #44172

The only option to fix this I found is to upgrade Microsoft.Bcl.AsyncInterfaces to 6.0.0 in #44185. Then release a new version of Azure.Core and apply it in this PR.

jsquire · 2024-05-22T13:38:09Z

sdk/personalizer/Azure.AI.Personalizer/src/Azure.AI.Personalizer.csproj

@@ -22,6 +22,8 @@

  <ItemGroup>
    <PackageReference Include="Azure.Core" />


Rather than adding temp references to STJ in these packages, I'd strongly suggest that you use a temporary project reference to Azure.Core to align all of the transitive dependencies.

live1206 · 2024-05-22T14:45:34Z

/azp run net - eventgrid - tests

azure-pipelines · 2024-05-22T14:45:49Z

Azure Pipelines successfully started running 1 pipeline(s).

live1206 · 2024-05-22T15:42:01Z

/azp run net - eventgrid - tests

azure-pipelines · 2024-05-22T15:42:16Z

Azure Pipelines successfully started running 1 pipeline(s).

live1206 · 2024-05-27T07:29:35Z

fixing in #44208 instead

live1206 added 3 commits May 19, 2024 00:18

Fix AOT warnings for Azure.Core

172025e

Upgrade System.Text.Json to 6.0.0 to use JsonObject

fb52e90

fix tests

4f3bb5c

github-actions bot added the Azure.Core label May 19, 2024

live1206 marked this pull request as ready for review May 19, 2024 03:48

live1206 requested review from JoshLove-msft, christothes, annelo-msft, KrzysztofCwalina, tg-msft, jsquire, pallavit, m-nash, ArthurMa1978 and a team as code owners May 19, 2024 03:48

fix nullable after upgrading system.text.json

6a8fa4c

use BinaryData constructor instead of FromString, they are the same

8c3e00f

jsquire requested review from m-redding and eerhardt May 20, 2024 13:54

eerhardt reviewed May 20, 2024

View reviewed changes

sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs Outdated Show resolved Hide resolved

string readability

10b62d1

jsquire reviewed May 20, 2024

View reviewed changes

live1206 added 4 commits May 21, 2024 10:44

Address comments

2c626d6

fix build

f2434bc

fix test

f22f57d

fix build

01cbd98

live1206 requested a review from vicancy as a code owner May 21, 2024 05:28

live1206 requested review from JialinXin and Kishp01 as code owners May 21, 2024 05:28

fix more build error

cec377b

live1206 requested review from anilba06, gkostal and a team as code owners May 21, 2024 06:51

live1206 force-pushed the fix-core-aot branch 2 times, most recently from 38fb07c to cec377b Compare May 21, 2024 09:41

Azure deleted a comment from azure-pipelines bot May 21, 2024

jsquire reviewed May 22, 2024

View reviewed changes

live1206 requested review from hallipr, weshaggard, benbp, archerzz and ArcturusZhang as code owners May 22, 2024 14:43

Add direct reference to fix version conflicts

c0275fa

live1206 force-pushed the fix-core-aot branch from 955a4bd to c0275fa Compare May 22, 2024 15:40

live1206 closed this May 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix AOT warnings in Azure.Core #44136

Fix AOT warnings in Azure.Core #44136

live1206 commented May 19, 2024 •

edited

azure-sdk commented May 19, 2024 •

edited

live1206 commented May 20, 2024 •

edited

jsquire May 20, 2024

jsquire May 20, 2024

live1206 May 21, 2024

jsquire May 20, 2024

live1206 May 21, 2024

jsquire May 20, 2024

jsquire May 20, 2024

jsquire May 20, 2024

live1206 May 21, 2024

jsquire commented May 20, 2024

live1206 commented May 21, 2024 •

edited

live1206 commented May 22, 2024

jsquire May 22, 2024

live1206 commented May 22, 2024

azure-pipelines bot commented May 22, 2024

live1206 commented May 22, 2024

azure-pipelines bot commented May 22, 2024

live1206 commented May 27, 2024

		@@ -22,6 +22,8 @@

		<ItemGroup>
		<PackageReference Include="Azure.Core" />

Fix AOT warnings in Azure.Core #44136

Fix AOT warnings in Azure.Core #44136

Conversation

live1206 commented May 19, 2024 • edited

azure-sdk commented May 19, 2024 • edited

live1206 commented May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsquire commented May 20, 2024

live1206 commented May 21, 2024 • edited

live1206 commented May 22, 2024

Choose a reason for hiding this comment

live1206 commented May 22, 2024

azure-pipelines bot commented May 22, 2024

live1206 commented May 22, 2024

azure-pipelines bot commented May 22, 2024

live1206 commented May 27, 2024

live1206 commented May 19, 2024 •

edited

azure-sdk commented May 19, 2024 •

edited

live1206 commented May 20, 2024 •

edited

live1206 commented May 21, 2024 •

edited