Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix MobileILStrip handling of unmanaged dlls #21098

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

jkurdek
Copy link
Member

@jkurdek jkurdek commented Aug 22, 2024

Fixes dotnet/runtime#101967
Follow up to the runtime change dotnet/runtime#106267

@jkurdek jkurdek added run-mtouch-tests Run the mtouch tests do-not-merge Do not merge this pull request labels Aug 22, 2024
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@jkurdek
Copy link
Member Author

jkurdek commented Aug 22, 2024

@rolfbjarne it seems to me after inspection of binlogs that ILstrip was not run for any of the test. I can see the _RunAotCompiler property being false.

@rolfbjarne
Copy link
Member

@rolfbjarne it seems to me after inspection of binlogs that ILstrip was not run for any of the test. I can see the _RunAotCompiler property being false.

The bots were probably all x64 bots.

I'll try locally and see what I get.

@rolfbjarne
Copy link
Member

It seems the ResolveAssemblyReferences task just removes the reference to NativeLibrary.dll:

/Users/rolf/work/maccore/net9.0/xamarin-macios/builds/downloads/dotnet-sdk-9.0.100-rc.2.24420.1/sdk/9.0.100-rc.2.24420.1/Microsoft.Common.CurrentVersion.targets(2413,5): warning MSB3246: Resolved file has a bad image, no metadata, or is otherwise inaccessible. Invalid number of sections declared in PE header. Invalid number of sections declared in PE header. [/Users/rolf/work/maccore/net9.0/xamarin-macios/tests/xharness/bin/Debug/tmp-test-dir/monotouch-test1066/monotouch-test.csproj]

So it never reaches the ILStrip target.

Binlog:
build-iOS_Unified64-20240902_191140.binlog.zip

@jkurdek jkurdek removed the do-not-merge Do not merge this pull request label Sep 3, 2024
@jkurdek jkurdek marked this pull request as ready for review September 3, 2024 21:07
@jkurdek
Copy link
Member Author

jkurdek commented Sep 3, 2024

Thanks @rolfbjarne, the dll was referenced incorrectly. I did verify locally that without changing the MobileILStrip task we get a failure, and that the proposed fix works.

Copy link
Contributor

github-actions bot commented Sep 3, 2024

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@jkurdek jkurdek self-assigned this Sep 4, 2024
@vs-mobiletools-engineering-service2

This comment has been minimized.

@jkurdek jkurdek force-pushed the fix/il-strip-unmanaged-assembly branch from a05b593 to 57dfadb Compare September 4, 2024 20:38
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@jkurdek jkurdek changed the title Add unmanaged dll to monotouch-test Fix MobileILStrip handling of unmanaged dlls Sep 20, 2024
@vs-mobiletools-engineering-service2

This comment has been minimized.

1 similar comment
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [CI Build] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

.NET (No breaking changes)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 99 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 40 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 7 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 7 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

@rolfbjarne rolfbjarne merged commit d5a7b39 into net9.0 Sep 20, 2024
38 checks passed
@rolfbjarne rolfbjarne deleted the fix/il-strip-unmanaged-assembly branch September 20, 2024 18:01
@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [CI Build] Windows Integration Tests failed ❌

❌ Failed ❌

Pipeline on Agent
Hash: 55ff968412cf92acccd5a687d72a313ad5c5c975 [PR build]

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
run-mtouch-tests Run the mtouch tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants