Alpine Linux compatibility for pony #1844

dipinhora · 2017-04-18T18:44:32Z

Changes related to getting pony compiling and running on Alpine
Linux Edge.

Define _GNU_SOURCE so musl glibc compatibility is enabled.
Makefile changes to detect Alpine Linux and link in libexecinfo
Changes to stop using glibc specific __GNUC_PREREQ macro
Changes to genexe linking to link in libexecinfo if on Alpine
or FreeBSD (this is likely also needed but untested)
Changes to move Linux cpu affinity setting outside of thread
creation function and into same set affinity function as for
other platforms. This also includes enabling asio thread affinity
setting for non-linux platforms.
Rename SCHED_BATCH to PONY_SCHED_BATCH to avoid conflict with
musl.
Set this_scheduler thread-local variable to NULL on
ponyint_sched_init to avoid segfault in running codegen test
suite.

Resolves #1729

NOTE: This is unlikely to make pony compatible with all musl based distributions/compiling attempts because pony still relies on execinfo.h and musl doesn't provide that. Some distributions include libexecinfo (as Alpine did) but others (ubuntu for example) may not. According this post (http://www.openwall.com/lists/musl/2015/04/10/2) in the musl mailing list, the appropriate longer term solution might be to use libunwind.

/cc @sylvanc (for a review of the affinity changes in particular and everything in general)
/cc @Praetonus (for a review of the this_scheduler and execinfo stuff in particular and everything in general)

Tested using the following in docker run --privileged -it --rm alpine:edge sh on an Ubuntu 16.04 VM:

Install build tools/dependencies:
apk add --update alpine-sdk libressl-dev cmake binutils-gold llvm llvm-dev pcre2-dev less libexecinfo-dev
Add: #undef open64
to: /usr/include/llvm/Analysis/TargetLibraryInfo.h (around line 28)
This is only needed until the changes in {main,testing}/{llvm,llvm3.9,llvm4}: Additional musl related fixes alpinelinux/aports#1273
are merged.
Install coreutils for process monitor tests to pass or else they have an issue with /bin/cat being a link:
apk add --update coreutils
git clone https://github.com/ponylang/ponyc
make test (this will fail due to the unresolved PIC/PIE issues; see Always use --pic when using GCC on Linux #1811 and Unable to link Pony programs with gcc defaulting to PIE #1484)
./build/release/ponyc -d --pic packages/stdlib
./stdlib

jemc · 2017-04-18T18:54:07Z

src/libponyc/codegen/genexe.c

@@ -277,6 +278,9 @@ static bool link_exe(compile_t* c, ast_t* program,
 #ifdef PONY_USE_LTO
    "-flto -fuse-linker-plugin "
 #endif
+#if defined(PLATFORM_IS_ALPINE_LINUX) || defined(PLATFORM_IS_FREEBSD)


Quick thought: could PLATFORM_IS_ALPINE_LINUX be named PLATFORM_IS_MUSL_LINUX, and still be correct?

@jemc This is unlikely to be correct. See the note in my main PR comment about libexecinfo/execinfo.h. Additionally, there is no way to detect that the libc being used is musl (see: http://wiki.musl-libc.org/wiki/FAQ#Q:_why_is_there_no_MUSL_macro_.3F).

The way this is currently detected is via a Makefile check for the existence of /etc/alpine-release and is tied to the Alpine Linux distribution.

Highly disagree with this, as it also breaks cross-compilation. This should be properly detected through a test checking if a symbol in <execinfo.h> can be resolved by the linker without any additional libraries, and then with -lexecinfo if it can't.

@shizmob I agree that the best solution is to do a test and then base a decision on that however that was going to add a level of complexity into the ponyc linking code that I wasn't comfortable with. If others agree that's the route desired, then we can implement the appropriate solution.

Can you give me an example set of steps/commands to reproduce where it breaks cross-compilation?

killerswan · 2017-04-18T19:05:56Z

Cooool!

sylvanc · 2017-04-19T19:40:32Z

libexecinfo is for generating backtraces. We do indeed need it in the makefile for ponyc and the tests and benchmarks, for pony_assert. But I don't think we need it when building pony programs, so I think it can be removed, fixing the cross-compilation problem and eliminating the PLATFORM_IS_ALPINE_LINUX flag.

dipinhora · 2017-04-19T21:25:35Z

@sylvanc Maybe I'm misunderstanding things but pony_assert is used in parts of libponyrt in addition to libponyc. This means that programs compiled using ponyc and linking against libponyrt have references to the backtrace and backtrace_symbols functions and fail with the following errors (these errors are the reason I added in the logic for libexecinfo and the PLATFORM_IS_ALPINE_LINUX define):

Linking ./stdlib
Warning: environment variable $CC undefined, using cc as the linker
src/libponyrt/platform/ponyassert.c:42: error: undefined reference to 'backtrace'
src/libponyrt/platform/ponyassert.c:43: error: undefined reference to 'backtrace_symbols'
collect2: error: ld returned 1 exit status

I agree that the PLATFORM_IS_ALPINE_LINUX define is not ideal. Maybe we need to change the use of pony_assert in libponyrt? Or maybe an alternate solution might be to use libunwind instead of execinfo.h as that musl mailing list post suggests?

dipinhora · 2017-04-19T23:06:48Z

@sylvanc I've made a change to how the #if condition for pony_assert works to ensure that pony_assert for libponyrt maps to normal assert instead of the custom pony_assert_fail that prints backtraces. This means that we are back to the old behavior for libponyrt where there are no assertions or backtraces for release builds and normal assert based assertions for debug builds. This should have no impact on the libponyc assertions with backtraces using the new pony_assert_fail logic.

This has removed the dependency on the backtrace functions in libponyrt so I have removed the define for PLATFORM_IS_ALPINE_LINUX and the associated logic to link in libexecinfo. This should resolve the concerns related to the define and cross compiling.

Please let me know if this is an acceptable solution or if you have further suggestions for improving things.

kaniini · 2017-04-24T04:56:32Z

hi, i merged the llvm changes needed in alpine. are you guys interested in integrating ponyc into alpine itself officially, yet?

sylvanc · 2017-05-03T19:49:07Z

@kaniini, we are super interested in integrating ponyc into alpine itself officially! Sorry for the late response on this. What's the process?

sylvanc · 2017-05-03T19:51:09Z

@dipinhora this looks great! Let's merge it.

kaniini · 2017-05-03T23:56:03Z

@sylvanc right now just somebody showing up with an APKBUILD willing to maintain the packaging. we prefer upstream devs (or those with close ties to upstreams) where possible.

jemc · 2017-05-10T19:42:37Z

@kaniini - thanks for the info. I don't know if anyone is in a good position to step up and volunteer for maintaining the APKBUILD, but it's something I might do at some point in the future, if no one else has stepped up before then.

jemc · 2017-07-17T03:23:22Z

Is there a reason this hasn't been merged yet?

SeanTAllen · 2017-07-19T19:39:38Z

@dipinhora can you rebase this so we can merge?

Changes related to getting pony compiling and running on Alpine Linux Edge. * Define `_GNU_SOURCE` so musl glibc compatibility is enabled. * Makefile changes to detect Alpine Linux and link in `libexecinfo`. * Makefile changes to define PONY_NO_ASSERT for libponyrt-pic. * Changes to stop using glibc specific `__GNUC_PREREQ` macro. * Changes to move Linux cpu affinity setting outside of thread creation function and into same set affinity function as for other platforms. This also includes enabling asio thread affinity setting for non-linux platforms. * Rename `SCHED_BATCH` to `PONY_SCHED_BATCH` to avoid conflict with musl. * Set `this_scheduler` thread-local variable to `NULL` on `ponyint_sched_init` to avoid segfault in running codegen test suite. * Change `#if` for `pony_assert` to use normal `assert` for builds that define `PONY_NO_ASSERT` (`libponyrt` and `libponyrt-pic` only as of this commit) instead of the custom `pony_assert_fail` that prints backtraces. Resolves ponylang#1729

dipinhora · 2017-07-20T14:43:59Z

Rebased.

Praetonus

Looks good to me.

jemc reviewed Apr 18, 2017

View reviewed changes

SeanTAllen mentioned this pull request Apr 18, 2017

Last Week in Pony - April 25, 2017 ponylang/ponylang-website#56

Closed

jemc requested review from sylvanc and Praetonus April 18, 2017 19:28

jemc added the needs discussion during sync label Apr 19, 2017

SeanTAllen removed the needs discussion during sync label Apr 19, 2017

dipinhora force-pushed the PR_musl branch 2 times, most recently from f1cd4d0 to 9d79bd6 Compare April 19, 2017 23:01

SeanTAllen added the needs discussion during sync label Apr 25, 2017

sylvanc approved these changes May 3, 2017

View reviewed changes

SeanTAllen mentioned this pull request May 10, 2017

Last Week in Pony - May 14, 2017 ponylang/ponylang-website#69

Closed

SeanTAllen removed the needs discussion during sync label May 10, 2017

SeanTAllen added the needs discussion during sync label Jun 28, 2017

SeanTAllen removed the needs discussion during sync label Jul 19, 2017

dipinhora force-pushed the PR_musl branch from 9d79bd6 to 707bd60 Compare July 20, 2017 14:41

Praetonus approved these changes Jul 20, 2017

View reviewed changes

SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Jul 20, 2017

SeanTAllen mentioned this pull request Jul 20, 2017

Last Week in Pony - July 23, 2017 ponylang/ponylang-website#112

Closed

SeanTAllen merged commit 1e1c484 into ponylang:master Jul 20, 2017

ponylang-main added a commit that referenced this pull request Jul 20, 2017

Update CHANGELOG for PR #1844 [skip ci]

8b4a3e3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alpine Linux compatibility for pony #1844

Alpine Linux compatibility for pony #1844

dipinhora commented Apr 18, 2017

jemc Apr 18, 2017

dipinhora Apr 18, 2017

shizmob Apr 19, 2017 •

edited

Loading

dipinhora Apr 19, 2017 •

edited

Loading

killerswan commented Apr 18, 2017

sylvanc commented Apr 19, 2017

dipinhora commented Apr 19, 2017

dipinhora commented Apr 19, 2017

kaniini commented Apr 24, 2017

sylvanc commented May 3, 2017

sylvanc commented May 3, 2017

kaniini commented May 3, 2017

jemc commented May 10, 2017

jemc commented Jul 17, 2017

SeanTAllen commented Jul 19, 2017

dipinhora commented Jul 20, 2017

Praetonus left a comment

Alpine Linux compatibility for pony #1844

Alpine Linux compatibility for pony #1844

Conversation

dipinhora commented Apr 18, 2017

jemc Apr 18, 2017

Choose a reason for hiding this comment

dipinhora Apr 18, 2017

Choose a reason for hiding this comment

shizmob Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

dipinhora Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

killerswan commented Apr 18, 2017

sylvanc commented Apr 19, 2017

dipinhora commented Apr 19, 2017

dipinhora commented Apr 19, 2017

kaniini commented Apr 24, 2017

sylvanc commented May 3, 2017

sylvanc commented May 3, 2017

kaniini commented May 3, 2017

jemc commented May 10, 2017

jemc commented Jul 17, 2017

SeanTAllen commented Jul 19, 2017

dipinhora commented Jul 20, 2017

Praetonus left a comment

Choose a reason for hiding this comment

shizmob Apr 19, 2017 •

edited

Loading

dipinhora Apr 19, 2017 •

edited

Loading