-
Notifications
You must be signed in to change notification settings - Fork 531
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
Properly Detect & Handle Musl Systems #1657
Changes from 2 commits
3037ecc
b0b7596
4364a3f
08810d1
8b6e573
d5ecc56
624fb2a
c6c31ea
ed541a3
164a7f5
9506288
d068e57
c9ccdf1
2df2e5a
6ccc082
c34ba02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,8 @@ namespace SkiaSharp | |
{ | ||
internal static class PlatformConfiguration | ||
{ | ||
private static readonly Lazy<bool> isMuslLazy = new Lazy<bool>(IsMuslImplementation); | ||
|
||
public static bool IsUnix { get; } | ||
|
||
public static bool IsWindows { get; } | ||
|
@@ -26,6 +28,8 @@ internal static class PlatformConfiguration | |
|
||
public static bool Is64Bit { get; } | ||
|
||
public static bool IsMusl => IsLinux && isMuslLazy.Value; | ||
|
||
static PlatformConfiguration () | ||
{ | ||
#if WINDOWS_UWP | ||
|
@@ -49,6 +53,36 @@ static PlatformConfiguration () | |
#endif | ||
|
||
Is64Bit = IntPtr.Size == 8; | ||
|
||
isMuslLazy = new Lazy<bool>(IsMuslImplementation); | ||
} | ||
|
||
private static bool IsMuslImplementation() | ||
{ | ||
try | ||
{ | ||
var cpu = RuntimeInformation.ProcessArchitecture; | ||
switch (cpu) | ||
{ | ||
case Architecture.X86: | ||
return AccessCheck("/lib/libc.musl-x86.so.1", 0) == 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existence of that file does not indicate the libc variant that's used by the current process. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😢 Is there any way to check for Musl? Is there some specific thing that exists in Musl or glibc that we can use to detect? Maybe there is some thing we call that returns a string that says musl or not? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if I told you that technically one system can have both? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you could try to DllImport There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, since that check happens at runtime, I'd suggest to expose an API for selecting the proper binary and some SkiaSharp-specific environment variable (e. g.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Let me add that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only added 1. as I don't want a library to be reaching out into the environment. This can of course be added by a consuming library or by the final app. However, the API that I am making public is the existing // use default fallback
PlatformConfiguration.IsMuslOverride = null;
// is Musl
PlatformConfiguration.IsMuslOverride = true;
// is NOT Musl
PlatformConfiguration.IsMuslOverride = false; If there is ever another libc, we can just add a new If SkiaSharp is detecting Musl incorrectly, then it should be seen as a bug and hopefully will get reported and fixed so the override should remain mostly unused. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I am making the API internal for now and can be accessed via reflection. I have tests to make sure it stays. The reason for this is that this lookup system only exists on mono and we are really starting to move away from that. I added the API for the existing apps that need to control the lookup, but with .NET Core and .NET 5+, all the lookup is done via the runtime features. And if there is some pluggable system that needs multiple versions of libSkiaSharp in memory, then this is still not an issue because since .NET Core 3.0 there is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it should actually be using the |
||
case Architecture.X64: | ||
return AccessCheck("/lib/libc.musl-x86_64.so.1", 0) == 0; | ||
case Architecture.Arm: | ||
return AccessCheck("/lib/libc.musl-armv7.so.1", 0) == 0; | ||
case Architecture.Arm64: | ||
return AccessCheck("/lib/libc.musl-aarch64.so.1", 0) == 0; | ||
default: | ||
return false; | ||
} | ||
} | ||
catch | ||
{ | ||
return false; | ||
} | ||
} | ||
|
||
[DllImport("libc.so", EntryPoint = "access")] | ||
private static extern unsafe int AccessCheck([MarshalAs (UnmanagedType.LPStr)] string path, int mode); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a Musl system, first check the
musl-*
directories.