Skip to content
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

Merged
merged 11 commits into from
Jan 13, 2023

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Dec 9, 2022

This pull request updates the following dependencies

From https://github.com/dotnet/linker

  • Subscription: 0d6d6ae4-f71f-4395-53e6-08d8e409d87d
  • Build: 20230112.1
  • Date Produced: January 12, 2023 8:47:58 AM UTC
  • Commit: 10bb5ae1476941dbdfe3afa327db01750232db8a
  • Branch: refs/heads/main

…208.1

Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.22608.1
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-codeflow for labeling automated codeflow label Dec 9, 2022
…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
@build-analysis build-analysis bot mentioned this pull request Jan 3, 2023
…104.1

Microsoft.NET.ILLink.Tasks
 From Version 7.0.100-1.22606.1 -> To Version 8.0.100-1.23054.1
@lewing
Copy link
Member

lewing commented Jan 5, 2023

failures look widespread

@vitek-karas
Copy link
Member

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
Once that flows into this PR it should start working again.

@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
@sbomer
Copy link
Member

sbomer commented Jan 5, 2023

Looking into it

@ViktorHofer
Copy link
Member

The dependency update should be unblocked now.

@radical radical closed this Jan 9, 2023
@radical radical reopened this Jan 9, 2023
…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
@MichalStrehovsky
Copy link
Member

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

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?

@ViktorHofer ViktorHofer merged commit 5af6f72 into main Jan 13, 2023
@ViktorHofer ViktorHofer deleted the darc-main-da01b8fa-5534-4e0f-a42f-9ca32655f30b branch January 13, 2023 08:15
@vitek-karas
Copy link
Member

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.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-codeflow for labeling automated codeflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants