Fixes for .NET 6 linker #11739
Merged
Fixes for .NET 6 linker #11739
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. GitHub pagesResults can be found in the following github pages (it might take some time to publish): |
spouliot
reviewed
May 31, 2021
tools/linker/CoreTypeMapStep.cs
Outdated
| } | ||
|
|
||
| foreach (var reference in assembly.MainModule.AssemblyReferences) { | ||
| if (!Configuration.AssembliesByName.TryGetValue (reference.Name, out AssemblyDefinition resolvedReference)) |
There was a problem hiding this comment.
That works because you enforced the (assembly) weak name to be unique earlier.
Not sure it will work, unless everything was resolved (to latest version) before the linker is given the list (but then that check would not be needed)
There was a problem hiding this comment.
The linker also expects weak assembly names to be unique. You're right that the check was unnecessary - I forgot that GetLoadedAssembly is available.
This still relies on the workaround to load assemblies up-front: https://github.com/xamarin/xamarin-macios/blob/882dde132a0b893c1e929969d7472e93ec7745c8/tools/dotnet-linker/Steps/CollectAssembliesStep.cs#L19
I thought there was a way to get rid of this by resolving assemblies ourselves, but that doesn't appear to be the case - I filed dotnet/linker#2073 about it.
rolfbjarne
reviewed
May 31, 2021
- Avoid unnecessary tracking of loaded assemblies (Use GetLoadedAssembly instead) - Create extension method on LinkContext to avoid conditional code - Rename dispatchers to reflect when they run
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results29 tests failed, 81 tests passed.Failed tests
Pipeline on Agent XAMBOT-1100.BigSur |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results29 tests failed, 81 tests passed.Failed tests
Pipeline on Agent XAMBOT-1100.BigSur' |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results24 tests failed, 86 tests passed.Failed tests
Pipeline on Agent XAMBOT-1094.BigSur |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 109 tests passed.Failed tests
Pipeline on Agent XAMBOT-1101.BigSur |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results2 tests failed, 108 tests passed.Failed tests
Pipeline on Agent XAMBOT-1101.BigSur' |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 109 tests passed.Failed tests
Pipeline on Agent XAMBOT-1100.BigSur' |
By using the already-loaded Assembly closure
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 109 tests passed.Failed tests
Pipeline on Agent XAMBOT-1097.BigSur' |
|
Remaining work depends on dependency updates:
|
spouliot
approved these changes
Jun 11, 2021
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results2 tests failed, 113 tests passed.Failed tests
Pipeline on Agent XAMBOT-1094.BigSur' |
rolfbjarne
requested changes
Jun 14, 2021
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
|
I'm testing this locally. |
- Undo whitespace changes - Move comment to a more appropriate place
|
Thanks a lot! |
rolfbjarne
approved these changes
Jun 14, 2021
|
Looking at
None are blockers (or regressions from current) so the PR can be merged (once bots complete the current build). |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 114 tests passed.Failed tests
Pipeline on Agent XAMBOT-1098.BigSur' |
unrelated, known failure -> https://github.com/xamarin/maccore/issues/2454 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was having some trouble running tests locally, so triggering CI to see what happens.