-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[main] Update dependencies from dotnet/linker #79449
[main] Update dependencies from dotnet/linker #79449
Conversation
…208.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22608.1
…209.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22609.1
…212.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22612.1
…212.2 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22612.2
…219.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22619.1
…102.2 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23052.2
…104.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23054.1
failures look widespread |
Cecil is completely broken. This change (the upstream version of that change) jbevain/cecil#888 shows that it actually breaks even Cecil's own tests. The produced IL is somehow corrupted on the byte level. Using latest linker from main on a hello world ends up with an app which can't even start the runtime (probably fails to load CoreLib). And it crashes crossgen as seen in the CI here. We've reverted the change to new Cecil in linker here: dotnet/linker#3174 @sbomer - dotnet/cecil#55 introduced the break, can you please look into it? Technically it would be great if we actually tried to run trimmed code in the linker repo, but given that linker will move to runtime repo in the next couple of weeks it's probably moot point (well, once we switch the runtime to use live builds of linker anyway). /cc @tlakollo just as an FYI |
…105.2 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23055.2
Looking into it |
The dependency update should be unblocked now. |
…109.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23059.1
…110.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23060.1
…112.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23062.1
I'm surprised none of the tests in the illinker repo caught this. It would be good to understand why. I was helping @tlakollo troubleshoot this exact same (🥲) issue today because he ran into it when linker from the dotnet/runtime repo was getting consumed in the SDK repo. It took a bit to connect the dots. The runtime repo hasn't picked up the Cecil fix yet, so that explains things. The method bodies were completely wrong - illinker testing usually doesn't look at the method bodies, but at least the constant propagation tests do. I would expect us to see it there - were we just unlucky? |
Either that or Cecil could read the data it wrote... not sure. I didn't dig into it, mainly because the real test is to actually run the assembly - we discussed doing that many times, but never happened. And now when linker is in runtime it sort of decreases in priority a bit, because we will end up running linked code immediately in the CI. |
This pull request updates the following dependencies
From https://github.com/dotnet/linker