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

Linux c++ support using musl-cross-make toolchains #231

Merged
merged 42 commits into from
Oct 31, 2023

Conversation

DubbleClick
Copy link
Contributor

@DubbleClick DubbleClick commented Oct 16, 2023

This PR will

closes #148
closes #212
closes #217
closes #234
closes #94

Tasks

src/SPC/doctor/item/LinuxMuslCheck.php Outdated Show resolved Hide resolved
src/SPC/doctor/item/LinuxMuslCheck.php Outdated Show resolved Hide resolved
@DubbleClick
Copy link
Contributor Author

I will see to fix pgsql compilation and add imap back once this is merged, so it doesn't become a giant PR again. No point creating a PR doing these things now and changing them later when this is merged.

@crazywhalecc crazywhalecc added enhancement New feature or request os/linux Things only for Linux OS labels Oct 16, 2023
@crazywhalecc
Copy link
Owner

Is there any difference between /lib and /lib64? The distributions I have seen so far (64-bit) do not distinguish between the two and put all libraries in /lib.

@crazywhalecc crazywhalecc added the mixed PR This PR contains multiple updates label Oct 16, 2023
@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 16, 2023

Is there any difference between /lib and /lib64? The distributions I have seen so far (64-bit) do not distinguish between the two and put all libraries in /lib.

Some libraries build into /lib64 for me.
Libxml2 and snappy are two examples.

Edit: I just realised we're using cmake to build them. We can specific the dir to /lib with -DCMAKE_INSTALL_LIBDIR (or however it was called). I'll undo the pkgconfig commit and change the libdir instead.

@DubbleClick DubbleClick force-pushed the cplus branch 2 times, most recently from e06bab3 to a90cc14 Compare October 17, 2023 14:19
@crazywhalecc crazywhalecc added the kind/dependency Issues related to dependencies label Oct 18, 2023
@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 19, 2023

I'm split on the decision whether to add musl to PATH, or to treat it as a library and specify absolute paths when we specify cc/cxx/ld/ar. What do you think? Of course adding it to PATH is easier when the user wants to use the tools outside of spc too, but by specify full path we don't rely on the users environment and/or don't have to change it.

What do you think? @crazywhalecc

$arch = arch2gnu(php_uname('m'));
$this->setOptionIfNotExist('cc', "{$arch}-linux-musl-gcc");
$this->setOptionIfNotExist('cxx', "{$arch}-linux-musl-g++");
$this->setOptionIfNotExist('ar', "{$arch}-linux-musl-ar");
$this->setOptionIfNotExist('ld', "{$arch}-linux-musl-ld");
$this->setOptionIfNotExist('library_path', "LIBRARY_PATH=/usr/local/musl/{$arch}-linux-musl/lib");
$this->setOptionIfNotExist('ld_library_path', "LD_LIBRARY_PATH=/usr/local/musl/{$arch}-linux-musl/lib");

becomes

$arch = arch2gnu(php_uname('m'));
$path = SOURCE_PATH . "/musl-cross-make/{$arch}";
$this->setOptionIfNotExist('cc', "{$path}/bin/{$arch}-linux-musl-gcc");
$this->setOptionIfNotExist('cxx', "{$path}/bin/{$arch}-linux-musl-g++");
$this->setOptionIfNotExist('ar', "{$path}/bin/{$arch}-linux-musl-ar");
$this->setOptionIfNotExist('ld', "{$path}/bin/{$arch}/{$arch}-linux-musl/bin/ld.gold");
$this->setOptionIfNotExist('library_path', "LIBRARY_PATH={$path}/{$arch}-linux-musl/lib");
$this->setOptionIfNotExist('ld_library_path', "LD_LIBRARY_PATH={$path}/{$arch}-linux-musl}/lib");

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 19, 2023

In the past, I have tried to avoid compiling the toolchains necessary for the compilation itself in user mode. Because whether you use a downloaded binary and add it to PATH, or manually compile a gcc, they will consume a lot of time and disk space and it is intolerable. Using package management can speed things up, especially in Alpine environments where they can be installed directly.

However, if musl-cross-make is stable and feasible now, we could add this pre-dependent library when detecting non-Alpine environments, which will only extend the time for compiling PHP in these distributions. At this time it is more appropriate to become the latter, but we also need to consider compatibility with other libraries (so this will be another big project)

@crazywhalecc
Copy link
Owner

But anyway, I don't want to make this very complicated. Maybe it would be cool to support purely static compilation for glibc-based distributions, but I don't want to have to make extensive changes to every library that already implements the compilation command. The final impact on the compiler is best implemented only in LinuxBuilder.php.

@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 19, 2023

In the past, I have tried to avoid compiling the toolchains necessary for the compilation itself in user mode. Because whether you use a downloaded binary and add it to PATH, or manually compile a gcc, they will consume a lot of time and disk space and it is intolerable. Using package management can speed things up, especially in Alpine environments where they can be installed directly.

However, if musl-cross-make is stable and feasible now, we could add this pre-dependent library when detecting non-Alpine environments, which will only extend the time for compiling PHP in these distributions. At this time it is more appropriate to become the latter, but we also need to consider compatibility with other libraries (so this will be another big project)

I don't plan to have everyone compile musl-cross-make themselves, downloading the archive you see in MuslFix takes like 5 seconds. And it doesn't happen on Alpine, only on glibc based distros. The install command you see is only for the musl-wrapper (which is needed so that compiler tests that compile dynamically work). It only takes about 15 seconds for me, 30 seconds on a 4 core ARM VM.

But anyway, I don't want to make this very complicated. Maybe it would be cool to support purely static compilation for glibc-based distributions, but I don't want to have to make extensive changes to every library that already implements the compilation command. The final impact on the compiler is best implemented only in LinuxBuilder.php.

Most of the individual lib changes are actually fixes for alpine too and not limited to glibc distros. It's about being able to compile different extensions together without running into compilation/linking failures. It just makes sense to fix them with this branch because why add half-broken c++ support like we currently have it on alpine when with minimal extra changes we can directly get other combinations working too.

The only glibc specific necessary changes are specifying -lstdc++ (which it should be anyway, bad practise to rely on it implicitly) and LD_LIBRARY_PATH in a few commands.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 21, 2023

I successfully built the C++ extension on Debian using https://dl.static-php.dev/static-php-cli/deps/musl-toolchain/ and it works just fine (using musl.cc one failed with segfault).

I will do some updates tomorrow:

  1. By default, musl-toolchain will become optional and will not be downloaded from the doctor. When the current distribution is not alpine, you can use the parameter --with-musl-cross to build C++ extensions in Debian/RHEL.
  2. The Debian series uses musl-gcc installed through package management instead of compilation. It is only compiled in environments such as RHEL that do not specify packages.
  3. Modify some environment parameters to putenv() to simplify the compilation command. Because the commands called through shell() are all child processes of the current php, env variables can be passed to the child process.
  4. Try to build aarch64 musl-cross-toolchain (but my mac is MacBook Air M2 16GB, small RAM 😢 ).

And you are right, it is unrealistic to build musl-toolchain using actions, maybe we just keep this Dockerfile and config.mak file for future modification.

@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 21, 2023

  1. By default, musl-toolchain will become optional and will not be downloaded from the doctor. When the current distribution is not alpine, you can use the parameter --with-musl-cross to build C++ extensions in Debian/RHEL.

I do not like this. It's more to maintain for us (musl-gcc wrapper AND $arch-linux-musl).

  1. The Debian series uses musl-gcc installed through package management instead of compilation. It is only compiled in environments such as RHEL that do not specify packages.

Bad idea, we have no control over which musl-gcc wrapper version it will install and if our $arch-linux-musl is compatible with it. Musl tries to be future compatible, but it's hard to guarantee it.

  1. Modify some environment parameters to putenv() to simplify the compilation command. Because the commands called through shell() are all child processes of the current php, env variables can be passed to the child process.

Good idea!

  1. Try to build aarch64 musl-cross-toolchain (but my mac is MacBook Air M2 16GB, small RAM 😢 ).

I built it no problem on a 4 core 16 gig ram aarch64 azure VM with Alma and Alpine. The RAM requirement seems to only be true for Docker compilation. Exact model: Standard D4ps v5 (4 vcpus, 16 GiB memory)
If you run into any errors, just relaunch the make -j command until it works (yes, seriously lol, I used it like 5x and then it built).

I'm not sure if this is correct, please revert if it doesn't fix the issue
@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 29, 2023

For compiling imagemagick under the macOS platform, I found a strange point: if I do not use the LIBS= variable to force the imagemagick library to be specified, a not a mach-o file error will appear; if not specified, let It looks for it automatically, it will end up with a link error of undefined reference. In the case of the latter, I found that the three libraries related to the error reported were all compiled using cmake (libjpeg, libwebp, libxml2), while the others compiled using autoconf+make did not appear here.

@DubbleClick
Copy link
Contributor Author

Hmm, I can't test MacOS compilation, sorry :(

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 30, 2023 via email

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 30, 2023

I've tested the common extensions on Linux x86_64, as well as the compilation results of icu, imagemagick, postgresql, and ldap in cross-make. At the moment, all of these seem to be working well, and it's time to merge it. 🎉

There are really a lot of changes in this pull request, and I can't possibly test all the combinations of extensions in a short period of time.

Additionally, there are two new issues and PRs will be created:

  1. A bug in xlswriter.
  2. When using --build-all, the generated cli, fpm, and micro binaries include debugging symbols.

@DubbleClick
Copy link
Contributor Author

make it rc-8, I still want to add imap support after this for 2.0 release!

@crazywhalecc crazywhalecc merged commit 9fb5173 into crazywhalecc:main Oct 31, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kind/dependency Issues related to dependencies mixed PR This PR contains multiple updates
Projects
None yet
2 participants