Skip to content

Added: Assembly Trimming Annotations for SharpGen.Runtime #222

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.

Sign up for GitHub

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

Jump to bottom
Merged
jkoritzinsky merged 5 commits into from
Jun 26, 2022
Merged

Added: Assembly Trimming Annotations for SharpGen.Runtime #222

jkoritzinsky merged 5 commits into from
Jun 26, 2022

Conversation

@Sewer56
Copy link
Contributor

@Sewer56 Sewer56 commented Jun 26, 2022

The following PR adds an initial set of IL Trimmer Annotations for SharpGen.Runtime (and what is formerly SharpGen.Runtime.COM).

Motivation

Enable IL trimming, or at the very least safe trimming of libraries that depend on SharpGen.Runtime.

Ideally, my personal end goal is to contribute to @amerkoleci 's Vortice.Windows, by adding Assembly Trimming support to the library.

Changes

  • Added net6.0 target for SharpGen.Runtime with full trimming support.
  • Added partial trim support for net5.0, where Interface trim is handled by preserving all.
  • Added SharpGen.Runtime.Trim.Dummy project for testing trimmability at publish time. [The analyzer isn't perfect yet]

Areas of Note

  • I didn't mark the assembly as trimmable as I am not yet 100% certain that these annotations alone are sufficient.
  • Initial arbitrary tests so far seem to be yielding good results.

jkoritzinsky
jkoritzinsky approved these changes Jun 26, 2022
View reviewed changes
Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jkoritzinsky jkoritzinsky merged commit a25a71f into SharpGenTools:main Jun 26, 2022
@amerkoleci
Copy link
Contributor

amerkoleci commented Jun 26, 2022

Can you include also https://github.com/SharpGenTools/SharpGenTools/blob/main/SharpGen.Runtime.COM/SharpGen.Runtime.COM.csproj with those additions please? Overall looks goos

@Sewer56
Copy link
Contributor Author

Sewer56 commented Jun 26, 2022

Can you include also https://github.com/SharpGenTools/SharpGenTools/blob/main/SharpGen.Runtime.COM/SharpGen.Runtime.COM.csproj with those additions please? Overall looks goos

I was actually confused about that part for a sec when working on this PR.

I think that's a leftover from an earlier version, because a while back it seems like SharpGen.Runtime.COM was merged with the main SharpGen.Runtime package.

image

If you look at the code generated by newer versions of SharpGenTools, you'll see that it extends from SharpGen.Runtime.ComObject, which would be in SharpGen.Runtime

@andrew-boyarshin
Copy link
Contributor

andrew-boyarshin commented Jun 26, 2022

No, SharpGen.Runtime.COM is still a thing (for IStream and various other utilities), but it's true that IUnknown and ComObject are now in SharpGen.Runtime to simplify architecture and build infra.

@Sewer56
Copy link
Contributor Author

Sewer56 commented Jun 26, 2022

In this case I'll probably do SharpGen.Runtime.COM later today or tomorrow aswell.

Then get back to annotating Vortice.Windows.
Gonna be a fun 2 next days.

@Sewer56
Copy link
Contributor Author

Sewer56 commented Jun 26, 2022 •
edited
Loading

I noticed I forgot to conditional exclude packages in SharpGen.Runtime for net6.0 target in this PR.
I'll throw it in as part of the next one (it's already in my trimming-com branch); seems like SharpGen.Runtime.COM is gonna be pretty easy to tackle.

@Sewer56 Sewer56 mentioned this pull request Jun 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Sewer56 @amerkoleci @andrew-boyarshin @jkoritzinsky