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

Add linux-musl-x64 runtime support #497

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

botondbotos
Copy link

Possible solution for #496

<Link>libpact_ffi.so</Link>
<PackagePath>runtimes/linux-musl-x64/native</PackagePath>
<Pack>true</Pack>
<CopyToOutputDirectory Condition="'$(IsLinux)'">PreserveNewest</CopyToOutputDirectory>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an additional condition to only unpack on musl targets, which last time I tried to do this wasn't possible.

The libc and musl versions are both just called libpact_ffi.so and so their names conflict and it's unclear which one you actually get.

If you get the musl version where previously you get the libc version then this is a breaking change.

It's the same reason why the Mac version has two conditions below, so you don't end up with the ARM version on x86 and vice versa.

@adamrodger
Copy link
Contributor

The CI additionally would need updating to test the new target

@YOU54F
Copy link
Member

YOU54F commented Apr 23, 2024

Nice,

I had a go at this myself as part of a mission to bring alpine/musl support to the Pact ecosystem - see linked issue

master...YOU54F:pact-net:feat/musl

added a gh test in the branch, tried to test arm64 under qemu but no 🎲

master...YOU54F:pact-net:feat/trimmed_binaries

There still doesn't seem to be a nice way to detect musl via .NET

I used a file system lookup for the musl shared lib, which varies on the arch of the target, and I've only tested against alpine based images.

From looking at a few different languages, most tend to ldd <executable> like /bin/sh and grep for musl, unless its a compiled language and compilation is set by flags (like rust). I couldn't find a way in the .NET sln to run a command, so went for a musl shared lib look up instead.

For rust based executables, (not the pact_ffi shared libs), we are beginning to roll out the building of them statically with musl, rather than glibc, so we can support a single executable for all linux users.

Unfortunatly, we cannot do that with the ffi libs in languages that read on the shared, not static libs (PHP, .NET, Ruby)

@adamrodger
Copy link
Contributor

Yep that was the same problem I had when I originally raised a PR for this and had to abandon it @YOU54F.

There doesn't seem to be a reliable way to determine libc vs musl at NuGet restore time to make sure the correct shared library is unpacked, and if you get the wrong one it just won't work with no real workaround. That would be a disaster for existing users, hence why we need extensive CI for that situation.

It's got to be totally reliable and it can't break existing libc distros, and that's where the problem lies.

@adamrodger
Copy link
Contributor

It may be more pragmatic on Alpine to install something like https://github.com/Stantheman/gcompat to provide glibc compatibility so that the existing shared library will work, although I've not tested that.

@botondbotos
Copy link
Author

It may be more pragmatic on Alpine to install something like https://github.com/Stantheman/gcompat to provide glibc compatibility so that the existing shared library will work, although I've not tested that.

Unfortunately, gcompat doesn't seem to work: https://github.com/botondbotos/pact-net-glibc-vs-musl/blob/main/Dockerfile.gcompat
I managed to make it run by patching libpact_ffi.so with https://github.com/pact-foundation/pact-reference/releases/download/libpact_ffi-v0.4.19/libpact_ffi-linux-x86_64-musl.so.gz

@botondbotos
Copy link
Author

botondbotos commented Apr 29, 2024

Refactored the project to use runtime specific native packages as shown at https://github.com/Mizux/dotnet-native.
Managed to run the tests under samples/OrdersApi:

./pack.sh
docker build -f Dockerfile.alpine -t pact-net-alpine .
docker build -f Dockerfile.debian -t pact-net-debian .

The runtimes folder seems to contain all native libraries.

On Alpine

docker run --rm  pact-net-alpine tree samples/OrdersApi/Consumer.Tests/bin/Debug/net8.0/runtimes

Output:

samples/OrdersApi/Consumer.Tests/bin/Debug/net8.0/runtimes
├── linux-musl-x64
│   └── native
│       └── libpact_ffi.so
├── linux-x64
│   └── native
│       └── libpact_ffi.so
├── osx-arm64
│   └── native
│       └── libpact_ffi.dylib
├── osx-x64
│   └── native
│       └── libpact_ffi.dylib
├── win
│   └── lib
│       ├── net6.0
│       │   ├── System.Diagnostics.EventLog.Messages.dll
│       │   └── System.Diagnostics.EventLog.dll
│       └── netstandard2.0
│           └── System.Security.Cryptography.ProtectedData.dll
└── win-x64
    └── native
        └── pact_ffi.dll

On Debian

docker run --rm  pact-net-debian tree samples/OrdersApi/Consumer.Tests/bin/Debug/net8.0/runtimes

Output:

samples/OrdersApi/Consumer.Tests/bin/Debug/net8.0/runtimes
|-- linux-musl-x64
|   `-- native
|       `-- libpact_ffi.so
|-- linux-x64
|   `-- native
|       `-- libpact_ffi.so
|-- osx-arm64
|   `-- native
|       `-- libpact_ffi.dylib
|-- osx-x64
|   `-- native
|       `-- libpact_ffi.dylib
|-- win
|   `-- lib
|       |-- net6.0
|       |   |-- System.Diagnostics.EventLog.Messages.dll
|       |   `-- System.Diagnostics.EventLog.dll
|       `-- netstandard2.0
|           `-- System.Security.Cryptography.ProtectedData.dll
`-- win-x64
    `-- native
        `-- pact_ffi.dll

Is this approach worth pursuing?

@adamrodger
Copy link
Contributor

PactNet used to have architecture specific packages but a key design goal of the major changes in 4.x was to move away from this approach.

In practice it's very awkward to use for consumers - for example if you run the tests locally on Windows/Mac but your CI runs in Linux (as is very common) then you've got to reference multiple native packages and add conditions to your dependencies, which is a bad devex.

I don't see the cost of reintroducing all the complexity both within PactNet and especially for our users being worth adding musl support. At the end of the day this is a test library, so it's much less of a burden to run your tests in a glibc env instead, until such a time that detecting running on musl is more robust and can use the existing packaging mechanism.

It may be worth you creating an issue on the .Net team to add a feature to make detecting musl easier. It's definitely not an easy task to do really robustly though. Like even just detecting certain heuristics is hard because of things like cross compilation envs.

@botondbotos
Copy link
Author

botondbotos commented Apr 30, 2024

In practice it's very awkward to use for consumers - for example if you run the tests locally on Windows/Mac but your CI runs in Linux (as is very common) then you've got to reference multiple native packages and add conditions to your dependencies, which is a bad devex.

There's only need to reference a single package, PactNet: https://github.com/pact-foundation/pact-net/pull/497/files#diff-bc4f87702468a93737635b96e0dc7fc5ec46a8fab44ebc9f249a17091de6fd60R16.
The PactNet package depends on all the other packages containing the native pact ffi libraries. The packages containing the native libraries should never be referenced directly.
This packaging model helps generate the correct deps.json so that the correct assembly can be picked up by the runtime. This is demonstrated by the Dockerfile.alpine and Dockerfile.debian containers.

... At the end of the day this is a test library, so it's much less of a burden to run your tests in a glibc env instead ...

Some teams prefer to run tests on the exact same platform with exactly the same dependencies as what gets deployed to production. Exceptions could be made, and contract tests could be run on a sperate platform, but that adds extra complexity to the CI pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants