[dotnet] Add support for .so files #16887

ylatuya · 2022-11-24T16:39:21Z

Autotools-based project using libtool's -module flag generate plugins with the .so extension that needs to be treated like DynamicLibraries in terms of deployment location and relocation, except they are not linked to the app.

This PR depends on #16706

rolfbjarne · 2022-11-24T21:11:18Z

This PR depends on #16706

OK, let's wait for that one to be complete & merged then, which makes it easier to review and test these changes.

rolfbjarne · 2022-12-16T10:26:26Z

@ylatuya can you rebase this on main and fix the merge conflicts?

ylatuya · 2022-12-16T11:59:20Z

Sure! I will also create a new test for it.

ylatuya · 2022-12-22T09:10:07Z

Integration test added and rebased

rolfbjarne · 2022-12-22T09:20:12Z

/azp run

azure-pipelines · 2022-12-22T09:20:40Z

Azure Pipelines successfully started running 1 pipeline(s).

rolfbjarne · 2022-12-22T09:21:42Z

except they are not linked to the app.

Out of curiosity, why not? It seems to me that .dylibs and .so files should be treated the same way.

ylatuya · 2022-12-22T11:31:37Z

except they are not linked to the app.

Out of curiosity, why not? It seems to me that .dylibs and .so files should be treated the same way.

By definition, libtool modules are libraries that are meant to be dlopened: https://www.gnu.org/software/automake/manual/html_node/Libtool-Modules.html. It's usually plugins such as the SVG loader for gdk-pixbuf, GIO modules, GStreamer plugins, etc... that will be loaded at runtime on-demand.

Thinking about that, we might not want to treat all .dylib files the same way and we should probably add a special type for plugins. We don't want these library plugins linked to the application as it would incur in a huge memory footprint, but we still want them to be relocated/reidentified.

Using GStreamer as example, which ships its plugins as .dylib files (it no longer uses autotools/libtool), all those plugins will be linked to the application and loaded at runtime which would result in a huge memory footprint. Instead, those .dylib files should be identified as plugins and relocated/reidentified but not linked to the application, so that they are loaded by GStreamer itself on demand.

ylatuya · 2023-01-12T10:01:28Z

@rolfbjarne can you review this one please?

rolfbjarne · 2023-01-12T13:39:44Z

/azp run

rolfbjarne · 2023-01-12T13:39:59Z

@rolfbjarne can you review this one please?

Will do

azure-pipelines · 2023-01-12T13:40:18Z

Azure Pipelines successfully started running 1 pipeline(s).

rolfbjarne

This looks good! It would be great if you could make a couple of minor improvements:

Update the documentation in https://github.com/xamarin/xamarin-macios/blob/main/dotnet/BundleContents.md with the new behavior.
Add a test case for *.so files to the BundleStructure test here:
- https://github.com/xamarin/xamarin-macios/tree/main/tests/dotnet/BundleStructure
- This would just be adding a NoneQ.so file in each subdirectory of the test project (so for iOS add the file here: https://github.com/xamarin/xamarin-macios/tree/main/tests/dotnet/BundleStructure/iOS, and likewise for the other platforms) - no need to add anything to the *.csproj, because these files should be added automatically to the build.
- And then fix the unit test accordingly: https://github.com/xamarin/xamarin-macios/blob/main/tests/dotnet/UnitTests/BundleStructureTest.cs, probably somewhere around here: https://github.com/xamarin/xamarin-macios/blob/49d13fc0efe5e462a4a5273d26fe4245f19b8389/tests/dotnet/UnitTests/BundleStructureTest.cs#L130-L131

mandel-macaque · 2023-01-27T18:52:24Z

@ylatuya would be great to land this, can you address @rolfbjarne comment?

Some projects like GStreamer, GIO or GdkPixbuf provide plugins as dynamic libraries that are loaded on demand at runtime. These libraries have usually the .dylib extension except for autotools-based projects using libtool's -module flag that generate libraries with the .so extension. These libraries need to be re-identified and deployed but that are not linked to the application.

ylatuya · 2023-02-01T15:13:08Z

I am still unable to run the complete test suite locally so I can't run test before pushing and I need to rely in the CI but

Are you able to build xamarin-macios locally? If so, what happens if you run make run-unit-tests in the tests/dotnet directory?

I am able to build xamarin-macios. When I run the tests with make run-unit-tests it takes a lot to build them, some might get stuck for minutes so I end up using TEST_FILTER="--filter BuildAndExecuteAppWithNativeDynamicLibrariesInPackageReference" I have also other errors in the build stage of some of the tests, I didn't have time to look deeper

I can't see the errors since the files are not publicly available.

This is the failure:

    Failed BuildAndExecuteAppWithNativeDynamicLibrariesInPackageReference(MacOSX,"osx-x64") [17 s]
    Error Message:
     There is a library
    Expected: file or directory exists
    But was:  "/Users/builder/azdo/_work/1/s/xamarin-macios/tests/dotnet/AppWithNativeDynamicLibrariesInPackageReference/macOS/bin/Debug/net7.0-macos/osx-x64/AppWithNativeDynamicLibrariesInPackageReference.app/Contents/MonoBundle/subdir/libtest.so"
    Stack Trace:
     at Xamarin.Tests.DotNetProjectTest.AssertThatDylibExistsAndIsReidentified(String appPath, String dylibRelPath) in /Users/builder/azdo/_work/1/s/xamarin-macios/tests/dotnet/UnitTests/ProjectTest.cs:line 1246
     at Xamarin.Tests.DotNetProjectTest.BuildAndExecuteAppWithNativeDynamicLibrariesInPackageReference(ApplePlatform platform, String runtimeIdentifier) in /Users/builder/azdo/_work/1/s/xamarin-macios/tests/dotnet/UnitTests/ProjectTest.cs:line 1221

I had a fix locally for this one that I forgot to commit.

What is the error of the other failing test? The BCL one?

rolfbjarne · 2023-02-01T15:18:42Z

What is the error of the other failing test? The BCL one?

That's a random issue unrelated to your PR.

ylatuya · 2023-02-01T16:42:47Z

What is the error of the other failing test? The BCL one?

That's a random issue unrelated to your PR.

Ok! The failing test should be passing now.

rolfbjarne · 2023-02-01T16:44:58Z

/azp run

azure-pipelines · 2023-02-01T16:45:25Z

Azure Pipelines successfully started running 1 pipeline(s).

rolfbjarne

This is looking better and better! Just a few really minor things left now.

tests/test-libraries/libraries/Makefile

rolfbjarne · 2023-02-02T08:56:47Z

tests/dotnet/BundleStructure/shared.csproj

+			<!-- Bundled, not linked with, install_name_tool must have been executed -->
+			<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+		</None>
 	</ItemGroup>


The NoneQ.so file you added doesn't seem to be used anywhere, so let's try to do that:

Suggested change

</ItemGroup>

<None Update="NoneQ.so">



<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>

</None>

</ItemGroup>

this will require changes in the BundleStructureTest.cs file as well.

I tried replicating the same structure as NoneQ, that has NoneQ.dylib and libNoneQ.dylib, so I created NoneQ.so and libNoneQ.so. It seems that NoneQ.dylib is not checked either and that's the reason why I simply added NoneQ.so. I though they just need to be there and simply ignored, since they are not real libraries, it's just a text file with the file extension.

rolfbjarne · 2023-02-02T15:04:34Z

/azp run

azure-pipelines · 2023-02-02T15:05:01Z

Azure Pipelines successfully started running 1 pipeline(s).

vs-mobiletools-engineering-service2 · 2023-02-02T16:46:39Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

✅ Generator diff

Generator diff is empty

Pipeline on Agent
Hash: 9a38405c7559c00a892959cf8d8e69acb6314381 [PR build]

vs-mobiletools-engineering-service2 · 2023-02-02T16:48:37Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: 9a38405c7559c00a892959cf8d8e69acb6314381 [PR build]

vs-mobiletools-engineering-service2 · 2023-02-02T16:52:46Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1042.Monterey'
Hash: 9a38405c7559c00a892959cf8d8e69acb6314381 [PR build]

vs-mobiletools-engineering-service2 · 2023-02-02T19:59:43Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 225 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 25 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 9a38405c7559c00a892959cf8d8e69acb6314381 [PR build]

rolfbjarne · 2023-02-03T08:16:51Z

Thanks a lot for your contribution!

rolfbjarne · 2023-02-03T08:17:10Z

/sudo backport release/7.0.2xx

vs-mobiletools-engineering-service2 · 2023-02-03T08:17:31Z

Backport Job to branch release/7.0.2xx Created! The magic is happening here

vs-mobiletools-engineering-service2 · 2023-02-03T08:19:27Z

Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=7281842 for more details.

ylatuya · 2023-02-03T18:40:30Z

Thanks a lot for your contribution!

It was a great learning experience. Thanks a lot for your help during the process!

Autotools-based project using libtool's -module flag generate plugins with the .so extension that needs to be treated like DynamicLibraries in terms of deployment location and relocation, except they are not linked to the app. This PR depends on #16706 Backport of #16887 Co-authored-by: Andoni Morales Alastruey <ylatuya@gmail.com>

ylatuya requested review from emaf, mauroa and rolfbjarne as code owners November 24, 2022 16:39

rolfbjarne self-assigned this Nov 24, 2022

rolfbjarne added the community Community contribution ❤ label Dec 19, 2022

ylatuya force-pushed the support_so_files branch from 5dae230 to 1c918d1 Compare December 22, 2022 09:09

This comment has been minimized.

Sign in to view

ylatuya force-pushed the support_so_files branch from 1c918d1 to f35921e Compare December 22, 2022 11:05

ylatuya force-pushed the support_so_files branch from f35921e to b227188 Compare December 22, 2022 15:25

ylatuya force-pushed the support_so_files branch from b227188 to 28d8b36 Compare January 12, 2023 10:01

This comment has been minimized.

Sign in to view

rolfbjarne requested changes Jan 18, 2023

View reviewed changes

mandel-macaque approved these changes Jan 27, 2023

View reviewed changes

ylatuya force-pushed the support_so_files branch from aeae7c0 to e1ef824 Compare February 1, 2023 15:12

This comment has been minimized.

Sign in to view

rolfbjarne requested changes Feb 2, 2023

View reviewed changes

test: add .so file to the BundleStructureTest

9a38405

ylatuya force-pushed the support_so_files branch from e1ef824 to 9a38405 Compare February 2, 2023 12:01

rolfbjarne approved these changes Feb 3, 2023

View reviewed changes

rolfbjarne changed the title ~~Add support for .so files~~ [dotnet] Add support for .so files Feb 3, 2023

rolfbjarne merged commit 97ab148 into dotnet:main Feb 3, 2023

vs-mobiletools-engineering-service2 mentioned this pull request Feb 3, 2023

[release/7.0.2xx] [dotnet] Add support for .so files #17440

Merged

[dotnet] Add support for .so files #16887

[dotnet] Add support for .so files #16887

Uh oh!

Conversation

ylatuya commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rolfbjarne commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rolfbjarne commented Dec 16, 2022

Uh oh!

ylatuya commented Dec 16, 2022

Uh oh!

ylatuya commented Dec 22, 2022

Uh oh!

rolfbjarne commented Dec 22, 2022

Uh oh!

azure-pipelines bot commented Dec 22, 2022

Uh oh!

rolfbjarne commented Dec 22, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

ylatuya commented Dec 22, 2022

Uh oh!

ylatuya commented Jan 12, 2023

Uh oh!

rolfbjarne commented Jan 12, 2023

Uh oh!

rolfbjarne commented Jan 12, 2023

Uh oh!

azure-pipelines bot commented Jan 12, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne left a comment

Choose a reason for hiding this comment

Uh oh!

mandel-macaque commented Jan 27, 2023

Uh oh!

ylatuya commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rolfbjarne commented Feb 1, 2023

Uh oh!

ylatuya commented Feb 1, 2023

Uh oh!

rolfbjarne commented Feb 1, 2023

Uh oh!

azure-pipelines bot commented Feb 1, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rolfbjarne Feb 2, 2023

Choose a reason for hiding this comment

Uh oh!

ylatuya Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rolfbjarne commented Feb 2, 2023

Uh oh!

azure-pipelines bot commented Feb 2, 2023

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 2, 2023

✅ API diff for current PR / commit

✅ API diff vs stable

✅ Generator diff

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 2, 2023

ylatuya commented Nov 24, 2022 •

edited

Loading

rolfbjarne commented Nov 24, 2022 •

edited

Loading

ylatuya commented Feb 1, 2023 •

edited

Loading

ylatuya Feb 2, 2023 •

edited

Loading

ylatuya commented Feb 3, 2023 •

edited

Loading