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

fix: Fixes missing constructor in trimmed builds #1970

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

Blackclaws
Copy link
Contributor

When trimming, the whole call chain for generics needs to be annotated with the same or broader scope of dynamically accessed members. See also: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/fixing-warnings#dynamicallyaccessedmembers

This adds the required annotations up the call chain for the service extensions

Closes: #1969

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 6, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Blackclaws / name: Felix Winterhalter (5b41786)

@JamesNK
Copy link
Member

JamesNK commented Dec 6, 2022

Thanks. I wonder why trimming analysis didn't catch this?

TClient on the core method is annotated. Analysis should complain that the TClient passed from the calling methods to the core method isn't annotated with PublicConstructors.

@Blackclaws
Copy link
Contributor Author

Thanks. I wonder why trimming analysis didn't catch this?

TClient on the core method is annotated. Analysis should complain that the TClient passed from the calling methods to the core method isn't annotated with PublicConstructors.

I does catch it. The problem is you need to actually publish a self-contained trimmed app for the trim analyzer to run correctly. See also: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming#show-all-warnings-with-sample-application

So basically you have to add a sample application that references the library as a root assembly even if it does nothing and publish that to see the actual warnings.

@JamesNK
Copy link
Member

JamesNK commented Dec 7, 2022

Got it. There is a sample app in the tests for this purpose, but it didn't use the client factory. I'll update it.

@JamesNK JamesNK merged commit e28a43f into grpc:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with Trimming - Missing annotation in GrpcClientServiceExtensions
2 participants