-
Notifications
You must be signed in to change notification settings - Fork 128
Skip C++/CLI assemblies #658
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
Conversation
|
@vatsan-madhavan I'm going to try to push this change into the SDK seeing as we are late in the release and don't have much time to wait. If later we determine a better way to check for C++/CLI assemblies, I'll make the update then. @marek-safar PTAL. |
|
@sbomer , lgtm. We should get confirmation from @tgani-msft when he is back that the C++/CLI detection being used here will remain a reliable contract/pattern over time, and if necessary, change the detection logic based on his feedback. |
…C++/CLI images. See dotnet/linker#651 and dotnet/linker#658. In turn, this results in a failure of dependencies of such assemblies (like System.Diagnostics.Debug.dll, which is required by DirectWriteForwarder.dll) from being identified and included in the ReadyToRun images. These linker hints tell ILLinker to include certain dependencies, which we know to be required by DirectWriteForwarder and System.Printing respectively.
* When producing ReadyToRun images, the ILLinker is configured to skip C++/CLI images. See dotnet/linker#651 and dotnet/linker#658. In turn, this results in a failure of dependencies of such assemblies (like System.Diagnostics.Debug.dll, which is required by DirectWriteForwarder.dll) from being identified and included in the ReadyToRun images. These linker hints tell ILLinker to include certain dependencies, which we know to be required by DirectWriteForwarder and System.Printing respectively. * Add netstandard and mscorlib to the list of dependencies
|
Why skip entirely? Just don't rewrite it, but still analyze its dependencies. |
|
I'd like to do that in the future, but the fix will take some more discussion: #639. The linker supposedly is able to do this, but in reality rewrites more often than you might expect. |
Asking for my edification: Does inspecting an assembly using Mono.Cecil API's rewrite the assembly - or is the rewriting specific to the linker implementation? |
|
@vatsan-madhavan I answered your question in #639 (comment). |
This will skip linking of C++/CLI assemblies. See #651 for context.
There are potentially some issues with skipping the C++/CLI assembly entirely - any calls that the assembly makes to .NET dlls will no longer be analyzed. This may result in some required dependencies being dropped, though I don't know how likely that is in practice. The ideal fix would be to address #639 (or to preserve the ordering of the relevant C++/CLI data when linking as mentioned in #651 (comment)), which would still result in the assembly being analyzed.
I marked this do not merge until we have some more confidence that this is the right fix.