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

Add error for incorrect aot publish #46070

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Jan 17, 2025

For the following project (note it uses the nativeaot runtime pack):

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
    <PublishAotUsingRuntimePack>true</PublishAotUsingRuntimePack>
    <SelfContained>true</SelfContained>
  </PropertyGroup>

</Project>

This command produces an error:

> dotnet build -t:Publish
Restore complete (0.6s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  ispublishing failed with 2 error(s) (0.7s) → bin/Debug/net10.0/linux-x64/ispublishing.dll
    EXEC : error : Failed to load assembly 'System.Private.StackTraceMetadata'
    /home/svbomer/.nuget/packages/microsoft.dotnet.ilcompiler/10.0.0-alpha.1.25052.4/build/Microsoft.NETCore.Native.targets(316,5): error MSB3073: The command ""/home/svbomer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/10.0.0-alpha.1.25052.4/tools/ilc" @"obj/Debug/net10.0/linux-x64/native/ispublishing.ilc.rsp"" exited with code 1.

This is due to ILC being passed assemblies from the coreclr runtime pack (which doesn't include S.P.StackTraceMetadata) instead of the nativeaot framework assemblies.

This adds a better error message for this unsupported scenario.

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Where do you see the north star with the native AOT runtime packs? Do we expect PublishAotUsingRuntimePack is here to stay? Do we expect dotnet publish to be the only way to trigger a publish (this would be problematic for e.g. WinAppSDK that just runs the Publish target).

Just wondering if it's worth it to build experiences around it and use up an error code for this (the only people who would find out about PublishAotUsingRuntimePack are iDevice/Bionic targeting users since we don't document it anywhere else).

LGTM modulo this question.

Copy link
Member

@ivanpovazan ivanpovazan left a comment

Choose a reason for hiding this comment

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

The error LGTM, thanks!

Regarding:

Where do you see the north star with the native AOT runtime packs? Do we expect PublishAotUsingRuntimePack is here to stay? Do we expect dotnet publish to be the only way to trigger a publish (this would be problematic for e.g. WinAppSDK that just runs the Publish target).

I think the north star is resolving: dotnet/runtime#87060 and removing the need for setting PublishAotUsingRuntimePack where supported ways of publishing are:

  • dotnet publish or
  • dotnet build /p:_IsPublishing=true ...

We had discussions before on this topic in: #37872

@sbomer
Copy link
Member Author

sbomer commented Jan 17, 2025

Agreed, I would like to move towards using runtime packs everywhere, make dotnet publish (or equivalent from VS) the only supported way to do this, and expand the scope of this error message when we do that. I haven't looked at the WinAppSDK usage yet, but maybe we can carve out a special path for it that doesn't error.

@sbomer sbomer merged commit 1b7971d into dotnet:main Jan 17, 2025
35 of 38 checks passed
@MichalStrehovsky
Copy link
Member

dotnet publish or
dotnet build /p:_IsPublishing=true ...

It's not very desirable to make dotnet build /p:_IsPublishing=true ... "supported" - the property is prefixed by an underscore and that sends a mixed message. And sending people purely to dotnet publish might be problematic in cases when dotnet is not part of the workflow. I think it's very likely we'll need to have a supported way to run the Publish target without dotnet publish and the north star message will include both remediations.

I asked about the north star and alignment of this with that because translating this message to a dozen languages and documenting it at https://learn.microsoft.com/en-us/dotnet/core/tools/sdk-errors/ only to have to redo it felt like just extra work.

@sbomer
Copy link
Member Author

sbomer commented Jan 21, 2025

Agreed, I think having a public IsPublishing is a good long-term solution. Redoing the error message is some extra work, but I think introducing it now will help move us in the right direction.

rolfbjarne added a commit to xamarin/xamarin-macios that referenced this pull request Feb 10, 2025
…ublishing.

.NET now shows an error if PublishAotUsingRuntimePack=true and PublishAot=true, but we're not actually publishing:

> Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(231): error NETSDK1225: Native compilation is not supported when invoking the Publish target directly. Try running dotnet publish.

Ref: dotnet/sdk#46070
rolfbjarne added a commit to xamarin/xamarin-macios that referenced this pull request Feb 10, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ublishing.

.NET now shows an error if PublishAotUsingRuntimePack=true and PublishAot=true, but we're not actually publishing:

> Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Publish.targets(231): error NETSDK1225: Native compilation is not supported when invoking the Publish target directly. Try running dotnet publish.

Ref: dotnet/sdk#46070
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants