Skip to content

Conversation

@sbomer
Copy link
Member

@sbomer sbomer commented Jul 25, 2019

The embedded xml files should be respected from copy assemblies the same way they are for link assemblies. I'm actually not sure why we respect them for AddBypassNGenUsed - I would expect that for assemblies that do not end up being used (even if they have action CopyUsed or AddBypassNGenUsed), we don't add roots from xml.

Necessary to make dotnet/wpf#1387 work. We're adding roots to WPF assemblies to work around the linker's inability to process C++/CLI assemblies (#658).

@marek-safar PTAL.

@sbomer
Copy link
Member Author

sbomer commented Jul 26, 2019

Tell mode form:

Description

This fix will let the linker read embedded xml root descriptors that come from "copy" assemblies - which is everything outside the core framework in the .NET Core use case. This allows WPF to ship assemblies with roots to keep dependencies of unanalyzed C++/CLI assemblies.

Customer Impact

Together with dotnet/wpf#1387, System.Diagnostics.Debug, netstandard, and mscorlib will be kept in every WPF app. This will make more WPF apps that use DirectWriteForwarder.dll or System.Printing.dll work with PublishTrimmed=true.

Regression?

Not a regression.

Risk

Low risk. Potentially some WPF users will be unhappy that these dependencies are being kept even if they aren't strictly necessary for their apps. However I expect that most apps will need these - we've had a number of reports where very simple apps (with single controls) required System.Diagnostics.Debug, and netstandard and mscorlib shouldn't cause much of a problem because they are fairly small.

@vatsan-madhavan
Copy link
Member

System.Diagnostics.Debug is touched in a module-constructor in DirectWriteForwarder; DirectWriteForwarder is used in the rendering of any (every) text - this dependency is certain to be touched in even a simple application.

@vatsan-madhavan
Copy link
Member

@sbomer, @marek-safar, We'd love to be able to test a core-sdk build that contains fixed bits before snapping Preview 8 from master -> release/3.0 on Monday. Can we get this fix in core-sdk sometime today?

@sbomer sbomer force-pushed the xmlFromCopyAssemblies branch from 614e270 to a224ccd Compare July 26, 2019 21:38
@sbomer
Copy link
Member Author

sbomer commented Jul 26, 2019

@marek-safar I think this is good to go once ci is green (the new test passes on my machine). PTAL.

@marek-safar marek-safar merged commit 18ff3f4 into dotnet:master Jul 29, 2019
tkapin pushed a commit to tkapin/runtime that referenced this pull request Jan 31, 2023
* Respect embedded xml files from copy assemblies

* Add testcase

* Remove reference from CopyLibrary -> Library


Commit migrated from dotnet/linker@18ff3f4
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.

3 participants