Make System.Transactions.Local trimmable on Windows

[eerhardt]

#72051 added IsTrimmable=false to System.Transactions.Local, which is an assembly in the shared framework. This means it is referenced for every app. Users don't opt-in and can't opt-out of referencing it in their app.

As #74506 points out, this caused a size regression on WASM. Subsequently #74828 fixed it to only be IsTrimmable=false on non-Windows.

However, Windows is still a problem. Setting IsTrimmable=false on any shared framework library is not correct. Instead, we need to be marking the unsafe APIs with [RequiresUnreferencedCode] and need to be using trimmer descriptors as necessary in order to make the library build not trim needed, unreferenced code.

As part of this, we should probably add a rule to our build that says "All shared fx libraries should be 'IsTrimmable=true' on all platforms."

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

---

#72051 added IsTrimmable=false to System.Transactions.Local, which is an assembly in the shared framework. This means it is referenced for every app. Users don't opt-in and can't opt-out of referencing it in their app.

As #74506 points out, this caused a size regression on WASM. Subsequently #74828 fixed it to only be IsTrimmable=false on non-Windows.

However, Windows is still a problem. Setting IsTrimmable=false on any shared framework library is not correct. Instead, we need to be marking the unsafe APIs with [RequiresUnreferencedCode] and need to be using trimmer descriptors as necessary in order to make the library build not trim needed, unreferenced code.

As part of this, we should probably add a rule to our build that says "All shared fx libraries should be 'IsTrimmable=true' on all platforms."

It is possible (I haven't confirmed) that not being able to trim a shared fx library will cause ILLinker warnings in every publish when PublishTrimmed or PublishAOT is true.

Author:	eerhardt
Assignees:	-
Labels:	area-System.Transactions, linkable-framework
Milestone:	-

[jkotas]

The library was marked as IsTrimmable=false due to COM interop. We should not need to mark any APIs with [RequiresUnreferencedCode]. We just need to make the COM interop compatible with trimming.

[roji]

@eerhardt @jkotas the distributed transactions part of System.Transactions - which is what does the COM interop - is something we ported mainly in order to unblock .NET Framework users and allow them to move to modern .NET. We don't expect to invest in this component in the future; if a significant need is signaled for cross-platform distributed transactions (#71769), we'll consider implementing new support for that, but that would likely be a separate API, and would most likely not do COM interop in any direct way.

So I generally don't see us investing a lot in Sys.Tx going forward, and would definitely avoid the effort to port the COM interop here to be trimming-compatible (via ComWrappers, if I understand correctly), unless it's absolutely necessary.

This means it is referenced for every app. Users don't opt-in and can't opt-out of referencing it in their app.

Does this mean that any Windows user gets an untrimmed System.Transactions regardless of whether they use any type in that assembly? I was under the impression that at least assembly-level trimming should work.

Note that System.Transactions does have an unfortunate design whereby distributed transaction code paths - which require the COM interop - aren't clearly distinguishable (e.g. by the linker) from those which aren't. In other words, some users use Sys.Tx only as a way to manage non-distributed transactions in an ambient way (via TransactionScope), and Sys.Tx implicitly escaltes to a distributed transaction needed (i.e. when more than one database is enlisted in the transaction). That makes it impossible to put [RequiresUnreferencedCode] only on the code paths which require the COM interop.

/cc @ajcvickers @AaronRobinsonMSFT

[vitek-karas]

I think it's OK if we decide to not support trimming for Sys.Tx (depends on scenarios and resource availability and so - also if we get ComWrappers source generated it might become a lot easier). In that case we should annotate the APIs which can lead to the COM usage with RequiresUnreferencedCodeAttribute (and probably also RequiresDynamicCodeAttribute) - so that developers which try to trim an app using Sys.Tx will get a warning.

[jkotas]

depends on scenarios

System.Data.Common references System.Transactions.Local.

In the current state, I believe that pretty much any database access will generate trim warnings due to System.Transactions.Local reference.

[roji]

@jkotas that's true, but is there a reason we can't just leave the assembly with IsTrimmable=false? Wouldn't the entire assembly be trimmed away if the user doesn't use any Sys.Tx (or System.Data) APIs? If so, that sounds like it could be an acceptable state of affairs until we have ComWrappers source generation etc.

[jkotas]

Wouldn't the entire assembly be trimmed away if the user doesn't use any Sys.Tx (or System.Data) APIs?

In full trimming mode, the entire assembly is trimmed away if the user doesn't use any Sys.Tx. Full trimming mode is the default for console apps .NET 7.

The problem only exists in partial trimming mode. Blazor uses partial trimming mode and that's why it showed up there.

It would not be enough just mark it as IsTrimmable=false. The trimming also runs during the library build itself and trimming the Com interop interfaces would break them. You would have to mark the assembly IsTrimmable=false during the libraries build but IsTrimmable=true for shipping somehow.

[roji]

Thanks for the info, I wasn't aware that IsTrimmable=true was necessary even for assembly trimming (in partial mode).

I'm not sure what our options here are... I think it's probably too late to switch to ComWrappers for 7.0. If this can be somehow handled by marking the assembly IsTrimmable=false during the libraries build but IsTrimmable=true for shipping, as you suggest, maybe that's the way to go. Otherwise, having Sys.Tx not be trimmed when on Windows in partial trimming mode may be acceptable for 7.0, I don't know.