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

Set PublishTrimmed in NativeAOT targets #87304

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

MichalStrehovsky
Copy link
Member

Looks like this is still needed. Fixes #87303.

Cc @dotnet/ilc-contrib

Looks like this is still needed. Fixes dotnet#87303.
@ghost
Copy link

ghost commented Jun 9, 2023

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

Issue Details

Looks like this is still needed. Fixes #87303.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Jun 9, 2023

How does it fix the test failure?

@MichalStrehovsky
Copy link
Member Author

The test is not supposed to run - it is conditioned on not being trimmed, basically:

[ConditionalFact(nameof(AllowsCustomResourceTypes))]
public static void ResourceManagerLoadsCorrectReader()

We're not setting the right feature switches.

@MichalStrehovsky
Copy link
Member Author

The publishtrimmed was dropped 6 hours ago on my (apparently) bad advice in #87135.

@jkotas
Copy link
Member

jkotas commented Jun 9, 2023

I assume that the problem is that we are not including the standard SDK publish targets and settings when AOT compiling tests in dotnet/runtime. So a potential alternative fix would be to add this to our test targets. But it does not hurt to have it in the shipping targets.

@MichalStrehovsky
Copy link
Member Author

I assume that the problem is that we are not including the standard SDK publish targets and settings when AOT compiling tests in dotnet/runtime. So a potential alternative fix would be to add this to our test targets. But it does not hurt to have it in the shipping targets.

Ah, yes, that's very likely the problem. Maybe doing it in the shipping targets is still better because I don't know how the targets will be set up for the MAUI cases.

@jkotas jkotas merged commit 742f0a8 into dotnet:main Jun 9, 2023
@MichalStrehovsky MichalStrehovsky deleted the fix87303 branch June 9, 2023 04:19
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants