Some missing parts for musl #2071

yshui · 2018-01-31T05:25:38Z

No description provided.

dlang-bot · 2018-01-31T05:25:39Z

Thanks for your pull request, @yshui! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

smaller, focused PRs are easier to review than big ones
try not to mix up refactoring or style changes with bug fixes or feature enhancements
provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

joakim-noah

Everything is scoped to Musl, so shouldn't affect anything else.

ibuclaw · 2018-02-01T07:47:04Z

src/core/sys/posix/signal.d

+        int sigev_notify;
+        void function(sigval) sigev_notify_function;
+        pthread_attr_t *sigev_notify_attributes;
+        char[56-3*long.sizeof] __pad;


Nit, spaces between operators. Otherwise OK.

joakim-noah · 2018-02-03T06:41:01Z

After all your work on using the Musl libc, I thought I'd try it out, so I whipped up an Alpine VPS at my host. I got dmd master built by bootstrapping through ldc ltsmaster and then druntime master built fine, after adding this patch and commenting out some backtrace declarations that aren't there on Alpine.

However, the druntime test runner always segfaults on me with the tests I tried, any idea what's going on? Are you modifying dmd in some way for Musl, other than having it set CRuntime_Musl instead of CRuntime_Glibc like I am? Never mind, there was an issue with invoking the druntime tests as a shared library, compounded by the fact that the makefile ignores SHARED=0 when building the tests. Modifying the makefile directly worked around those issues, am able to run the druntime tests now. Building and running a simple D program works fine for me, tried this one from the tests after modifying it to only use puts/printf from druntime.

joakim-noah · 2018-02-03T09:01:33Z

Alright, finally got all the druntime tests to run on Alpine with Musl, by applying this pull and making a few tweaks. Two test blocks from core.thread that call GC.collect manually had to be disabled, and the test runner still segfaulted on exit after running and passing all other unit tests in core.thread.

When I made the test runner skip core.thread, I only had to disable this assert in core.sync.mutex, just like on Bionic, all the other druntime modules' tests ran fine. When running the additional tests, I only had to disable thread_join from init_fini in release mode, everything else worked after the two changes mentioned above: SHARED turned off and the backtrace declarations commented out.

wilzbach · 2018-02-03T09:27:49Z

@yshui Once the issues that @joakim-noah has pointed out, have been addressed, how about making adding a changelog entry and announcement in the NG?

so I whipped up an Alpine VPS at my host.

As Alpine is getting more and more popular, how about doing this in a Dockerfile which can then be used by everyone who wants to use D applications with Alpine?

The GoLang people do this: https://hub.docker.com/_/golang/
There's already DMD in Docker and compiling an application for alpine could be as simple as:

docker run --rm -ti -v $(pwd):/src dlang-alpine/dmd dmd -run hello_alpine.d

Someone already missed it here: lindt/docker-dmd#1

yshui · 2018-02-04T01:33:04Z

@joakim-noah Yes, shared library doesn't work yet. I would need some time to look into that.

ibuclaw · 2018-02-04T07:35:17Z

Has gdc and ldc compilers been tested with these bindings? That would separate binding problems from just dmd problems.

joakim-noah · 2018-02-04T08:04:42Z

Why would you suspect dmd?

ibuclaw · 2018-02-04T19:45:18Z

Experience. GDC (and maybe LDC) just integrates into the environment if you compile it for that platform.

yshui · 2018-02-05T07:57:08Z

@ibuclaw @joakim-noah I think it's a problem with musl.

joakim-noah · 2018-02-05T13:50:39Z

src/core/sys/posix/sys/stat.d

@@ -1294,11 +1294,11 @@ else version (CRuntime_Musl)
        timespec st_atim;
        timespec st_mtim;
        timespec st_ctim;
-        extern(D)
+        extern(D) @safe @property


I hit this too, wonder why @safe is only needed with this C runtime.

Not sure. Other C runtimes don't use this definition. e.g. in CRuntime_Glibc this is behind a (__USE_MISC || __USE_XOPEN2KB) check, which is false.

yshui · 2018-02-05T15:03:24Z

Erm, at src/rt/sections_elf_shared.d:675, doing the relocation seems to fix the crash.

I'll push the change and see what the buildbot has to say.

Edit: looks like the buildbot doesn't like it :(

yshui · 2018-02-05T15:34:57Z

Looks like the glibc dynamic linker also relocates the entries in the dynamic section, while musl's doesn't. So I scoped the change to musl.

yshui · 2018-02-05T16:21:46Z

test/shared/link assumes library constructors are always run before main constructors, which seems to not be true with musl.

A couple other shared tests also make assumptions that are not true in musl. Otherwise all tests pass.

yshui · 2018-02-07T03:08:23Z

@joakim-noah Can you test again?

joakim-noah · 2018-02-07T20:39:28Z

Ran the druntime tests again with this latest pull, which fixes most of the problems. I still get a segfault in init_fini/thread_join and shared/finalize errors out, when running the tests in release mode. In debug mode, asserts start tripping up in the additional tests for shared libraries. Didn't actually need to remove the backtrace declarations, that was only needed for the bootstrap ldc, which I didn't modify to use CRuntime_Musl.

I went ahead and started running the Phobos tests too, most of them passed with these additions to druntime:

diff --git a/src/core/stdc/math.d b/src/core/stdc/math.d
index 488b9d0a..8a7000bc 100644
--- a/src/core/stdc/math.d
+++ b/src/core/stdc/math.d
@@ -626,7 +626,7 @@ else version( CRuntime_Musl )
         ///
         FP_FAST_FMAL = 0,
     }
-
+pure{
     int __fpclassifyf(float x);
     int __fpclassify(double x);
     int __fpclassifyl(real x);
@@ -730,6 +730,7 @@ else version( CRuntime_Musl )
             : __signbitl(x);
     }
   }
+    }
 }
 else version( MinGW )
 {
diff --git a/src/core/sys/posix/pthread.d b/src/core/sys/posix/pthread.d
index 23df7895..9b76e9db 100644
--- a/src/core/sys/posix/pthread.d
+++ b/src/core/sys/posix/pthread.d
@@ -384,6 +384,7 @@ else version( CRuntime_Bionic )
 else version( CRuntime_Musl )
 {
 
+    enum PTHREAD_MUTEX_INITIALIZER = pthread_mutex_t.init;
 }
 else
 {
diff --git a/src/core/sys/posix/stdlib.d b/src/core/sys/posix/stdlib.d
index 70794da8..186633c6 100644
--- a/src/core/sys/posix/stdlib.d
+++ b/src/core/sys/posix/stdlib.d
@@ -125,6 +125,10 @@ else version( CRuntime_Bionic )
     // Added since Lollipop
     int posix_memalign(void**, size_t, size_t);
 }
+else version( CRuntime_Musl )
+{
+    int posix_memalign(void**, size_t, size_t);
+}
 
 //
 // C Extension (CX)

That first pure block is needed because of a recent change in git master for druntime. This is the patch I used to get most Phobos tests passing:

diff --git a/std/datetime/timezone.d b/std/datetime/timezone.d
index 61416cf9e..4de1fd7c3 100644
--- a/std/datetime/timezone.d
+++ b/std/datetime/timezone.d
@@ -758,7 +758,7 @@ public:
 
     @safe unittest
     {
-        assert(LocalTime().dstName !is null);
+        //assert(LocalTime().dstName !is null);
 
         version(Posix)
         {
diff --git a/std/experimental/allocator/building_blocks/region.d b/std/experimental/allocator/building_blocks/region.d
index 82e550e3c..dfbd6641d 100644
--- a/std/experimental/allocator/building_blocks/region.d
+++ b/std/experimental/allocator/building_blocks/region.d
@@ -830,7 +830,7 @@ version(Posix) struct SbrkRegion(uint minAlign = platformAlignment)
     }
 }
 
-version(Posix) @system unittest
+version(rosix) @system unittest
 {
     // Let's test the assumption that sbrk(n) returns the old address
     const p1 = sbrk(0);
@@ -842,7 +842,7 @@ version(Posix) @system unittest
     sbrk(-4096);
 }
 
-version(Posix) @system unittest
+version(rosix) @system unittest
 {
     import std.typecons : Ternary;
     import std.algorithm.comparison : min;
diff --git a/std/file.d b/std/file.d
index 2b042de5b..9dea2c164 100644
--- a/std/file.d
+++ b/std/file.d
@@ -1346,7 +1346,7 @@ if (isConvertibleToString!R)
     }
 
     testTimes(0);
-    version (linux)
+    version (rinux)
         testTimes(123_456_7);
 
     rmdirRecurse(newdir);
diff --git a/std/socket.d b/std/socket.d
index 6a5e5ff2b..97167ea0a 100644
--- a/std/socket.d
+++ b/std/socket.d
@@ -203,7 +203,7 @@ string formatSocketError(int err) @trusted
             else
                 return "Socket error " ~ to!string(err);
         }
-        else version (CRuntime_Bionic)
+        else version (CRuntime_Musl)
         {
             auto errs = strerror_r(err, buf.ptr, buf.length);
             if (errs == 0)
@@ -2063,7 +2063,7 @@ static if (is(sockaddr_un))
         }
     }
 
-    @safe unittest
+    version(none)@safe unittest
     {
         import core.stdc.stdio : remove;
         import std.file : deleteme;
diff --git a/std/stdio.d b/std/stdio.d
index 16d34f0e9..eab71ad10 100644
--- a/std/stdio.d
+++ b/std/stdio.d
@@ -81,6 +81,11 @@ version (CRuntime_Bionic)
     version = NO_GETDELIM;
 }
 
+version (CRuntime_Musl)
+{
+    version = GENERIC_IO;
+    version = HAS_GETDELIM;
+}
 // Character type used for operating system filesystem APIs
 version (Windows)
 {

One assert, three test blocks, and one linux test function were disabled. Also, the std.net.curl tests don't pass, looks like some of the socket declarations in druntime are wrong and cause runtime exceptions.

Other than that, looking good so far, just more polishing to do. I don't expect you to fix all these issues though, so whenever you're ready, we can get this pull in.

This test make sure that locking will fail after the mutex is destroyed, which is not the case for musl.

yshui · 2018-02-08T13:22:34Z

@joakim-noah pure and missing definitions added.

joakim-noah

Looks good.

joakim-noah · 2018-02-10T21:26:26Z

@wilzbach, this is ready to go. This port still needs some polishing for shared libraries and phobos to pass all their tests, but we can get that in later.

wilzbach

Yes, please go step by step to success!

wilzbach · 2018-02-10T21:40:35Z

src/core/sync/mutex.d

@@ -377,6 +377,7 @@ unittest
    // For example, Bionic doesn't appear to do so, so this test is
    // not run on Android.
    version (CRuntime_Bionic) {} else
+    version (CRuntime_Musl) {} else


Weird, I thought Musl is promoted as drop-in replacement for glibc...

The compatibility only extends to the C source level, plus musl doesn't implement all features.

@safe

commit cb32bd4 Merge: edf321a e714ea0 Author: The Dlang Bot <code+dlang-bot@dawg.eu> Date: Sun Feb 11 15:09:55 2018 +0100 Merge pull request dlang#2085 from ibuclaw/linttrailing posix.mak: Add check for trailing whitespace from phobos merged-on-behalf-of: Sebastian Wilzbach <sebi.wilzbach@gmail.com> commit e714ea0 Author: Iain Buclaw <ibuclaw@gdcproject.org> Date: Sun Feb 11 11:01:26 2018 +0100 posix.mak: Add lint check for trailing whitespace from Phobos commit edf321a Merge: 7653199 fdc4779 Author: The Dlang Bot <code+dlang-bot@dawg.eu> Date: Sat Feb 10 23:42:13 2018 +0100 Merge pull request dlang#2071 from yshui/musl_fixes Some missing parts for musl merged-on-behalf-of: Sebastian Wilzbach <sebi.wilzbach@gmail.com> commit fdc4779 Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Thu Feb 8 08:18:49 2018 -0500 Add pthread definitions for musl commit 9a7be0e Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Thu Feb 8 08:15:30 2018 -0500 Define posix_memalign for musl commit 2c44836 Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Thu Feb 8 08:14:19 2018 -0500 Mark math functions as pure commit 3441362 Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Mon Feb 5 11:53:33 2018 -0500 Add definition of utimes for musl commit ea85b4d Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Mon Feb 5 11:52:23 2018 -0500 Skip a test in core.sync.mutex for musl This test make sure that locking will fail after the mutex is destroyed, which is not the case for musl. commit f08f10b Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Mon Feb 5 10:04:25 2018 -0500 Relocate the strtab for musl as well commit 6078897 Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Mon Feb 5 04:31:52 2018 -0500 Mark stat_t.st_*time functions as @safe commit 4ddf2a1 Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Wed Jan 31 00:24:25 2018 -0500 Add definition of errno for musl commit b37372f Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Wed Jan 31 00:18:35 2018 -0500 Add musl definition to core/sys/posix/sys/utsname.d commit 4b9af0c Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Wed Jan 31 00:16:12 2018 -0500 Add missing sigevent commit 525ddfb Author: Yuxuan Shui <yshuiv7@gmail.com> Date: Wed Jan 31 00:14:55 2018 -0500 Remove duplicate CLOCK_MONOTONIC

joakim-noah · 2018-02-12T14:44:18Z

@yshui, any plans to finish off Phobos? I got that linux test function in std.file to work with Musl by adding declarations for utimensat and AT_FDCWD to druntime, as I had to for Bionic.

yshui · 2018-02-13T04:57:34Z

@joakim-noah Since you had it working, you could go ahead and open a PR, if you want to.

joakim-noah · 2018-02-13T05:12:16Z

OK, just wanted to make sure we weren't duplicating effort. I will try to fix those remaining Phobos tests for Musl, not going to do anything with shared libraries though.

yshui requested review from andralex, CyberShadow, ibuclaw, MartinNowak, schveiguy and wilzbach as code owners January 31, 2018 05:25

joakim-noah approved these changes Feb 1, 2018

View reviewed changes

ibuclaw approved these changes Feb 1, 2018

View reviewed changes

yshui force-pushed the musl_fixes branch from c6fd777 to 9ac0881 Compare February 2, 2018 16:54

wilzbach mentioned this pull request Feb 3, 2018

Create alpine images lindt/docker-dmd#1

Open

joakim-noah reviewed Feb 5, 2018

View reviewed changes

yshui requested a review from Burgos as a code owner February 5, 2018 15:04

yshui force-pushed the musl_fixes branch from eaf9d5e to 42ec10e Compare February 5, 2018 15:33

yshui force-pushed the musl_fixes branch from 42ec10e to 65ffc4c Compare February 5, 2018 15:35

yshui requested review from WalterBright and PetarKirov as code owners February 5, 2018 16:53

yshui added 11 commits February 8, 2018 08:11

Remove duplicate CLOCK_MONOTONIC

525ddfb

Add missing sigevent

4b9af0c

Add musl definition to core/sys/posix/sys/utsname.d

b37372f

Add definition of errno for musl

4ddf2a1

Mark stat_t.st_*time functions as @safe

6078897

Relocate the strtab for musl as well

f08f10b

Skip a test in core.sync.mutex for musl

ea85b4d

This test make sure that locking will fail after the mutex is destroyed, which is not the case for musl.

Add definition of utimes for musl

3441362

Mark math functions as pure

2c44836

Define posix_memalign for musl

9a7be0e

Add pthread definitions for musl

fdc4779

yshui force-pushed the musl_fixes branch from 129ac43 to fdc4779 Compare February 8, 2018 13:19

joakim-noah approved these changes Feb 8, 2018

View reviewed changes

wilzbach approved these changes Feb 10, 2018

View reviewed changes

wilzbach added the auto-merge label Feb 10, 2018

dlang-bot merged commit edf321a into dlang:master Feb 10, 2018

joakim-noah mentioned this pull request Mar 13, 2018

Add Musl flag -libcmusl. dlang/dmd#8020

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some missing parts for musl #2071

Some missing parts for musl #2071

yshui commented Jan 31, 2018

dlang-bot commented Jan 31, 2018

joakim-noah left a comment

ibuclaw Feb 1, 2018

joakim-noah commented Feb 3, 2018 •

edited

Loading

joakim-noah commented Feb 3, 2018

wilzbach commented Feb 3, 2018

yshui commented Feb 4, 2018

ibuclaw commented Feb 4, 2018

joakim-noah commented Feb 4, 2018

ibuclaw commented Feb 4, 2018

yshui commented Feb 5, 2018

joakim-noah Feb 5, 2018

yshui Feb 5, 2018 •

edited

Loading

yshui commented Feb 5, 2018 •

edited

Loading

yshui commented Feb 5, 2018

yshui commented Feb 5, 2018 •

edited

Loading

yshui commented Feb 7, 2018

joakim-noah commented Feb 7, 2018

yshui commented Feb 8, 2018

joakim-noah left a comment

joakim-noah commented Feb 10, 2018

wilzbach left a comment

wilzbach Feb 10, 2018

dnadlinger Feb 10, 2018

joakim-noah commented Feb 12, 2018

yshui commented Feb 13, 2018

joakim-noah commented Feb 13, 2018

Some missing parts for musl #2071

Some missing parts for musl #2071

Conversation

yshui commented Jan 31, 2018

dlang-bot commented Jan 31, 2018

Bugzilla references

joakim-noah left a comment

Choose a reason for hiding this comment

ibuclaw Feb 1, 2018

Choose a reason for hiding this comment

joakim-noah commented Feb 3, 2018 • edited Loading

joakim-noah commented Feb 3, 2018

wilzbach commented Feb 3, 2018

yshui commented Feb 4, 2018

ibuclaw commented Feb 4, 2018

joakim-noah commented Feb 4, 2018

ibuclaw commented Feb 4, 2018

yshui commented Feb 5, 2018

joakim-noah Feb 5, 2018

Choose a reason for hiding this comment

yshui Feb 5, 2018 • edited Loading

Choose a reason for hiding this comment

yshui commented Feb 5, 2018 • edited Loading

yshui commented Feb 5, 2018

yshui commented Feb 5, 2018 • edited Loading

yshui commented Feb 7, 2018

joakim-noah commented Feb 7, 2018

yshui commented Feb 8, 2018

joakim-noah left a comment

Choose a reason for hiding this comment

joakim-noah commented Feb 10, 2018

wilzbach left a comment

Choose a reason for hiding this comment

wilzbach Feb 10, 2018

Choose a reason for hiding this comment

dnadlinger Feb 10, 2018

Choose a reason for hiding this comment

joakim-noah commented Feb 12, 2018

yshui commented Feb 13, 2018

joakim-noah commented Feb 13, 2018

joakim-noah commented Feb 3, 2018 •

edited

Loading

yshui Feb 5, 2018 •

edited

Loading

yshui commented Feb 5, 2018 •

edited

Loading

yshui commented Feb 5, 2018 •

edited

Loading