search

dotnet / linker

389 stars 126 forks source link

PublishTrimmed should use a linker that matches the TargetFramework #3029

Closed sbomer closed 1 year ago

sbomer commented 2 years ago

We need a long-term solution for back-compat issues when using a newer SDK to trim apps targeting older TFMs. Some issues encountered so far:

We will hit a similar situation with planned breaking changes in 8.0, where it would be difficult to maintain a back-compat path. Maybe there are more examples, but this one comes to mind:

The solution will most likely (pending the outcome of broader discussion) be to use the 6.0 linker when targeting .net6, etc. This issue tracks the technical side of that proposed solution.

sbomer commented 2 years ago

The current plan is to add an implicit PackageReference in the SDK targets, which would use the standard mechanism for importing targets from NuGet packages.

We could take the version of the PackageReference from a new KnownILLinkPack that we would add to the bundled versions. KnownILLinkPack would completely bypass the logic in ProcessFrameworkReferences that is used for other "runtime packs" like crossgen. This does mean that we would need to duplicate some logic from ProcessFrameworkReferences (into MSBuild .targets) if we want the linker version to respect RuntimeFrameworkVersion and similar properties. For reference, this property is respected by crossgen.

The KnownILLinkPack version should probably be the same as the other runtime packs like crossgen. This version matches the runtime pack, so it would be easier to do this once the linker has migrated into dotnet/runtime and can share the version number. But we probably need to manually update the version number for the 7.0 linker package which does not version with dotnet/runtime.

As for the behavior when targeting net6.0 or lower, my plan is to continue using the 7.0 linker going forward, because that is the configuration we are planning to ship in .NET 7. But if we get negative feedback from this, we could instead use the 6.0 linker. @vitek-karas any thoughts on this?

vitek-karas commented 2 years ago

I think it makes sense to keep 7.0 as-is, unless we get lot of feedback otherwise.

jtschuster commented 2 years ago

Leaving a note for once this is finalized to check on the linker benchmark in dotnet/performance and make sure that the benchmark will still use the latest version of the linker.