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

reference pr that shall not be merged #214

Closed
wants to merge 67 commits into from

Conversation

DubbleClick
Copy link
Contributor

@DubbleClick DubbleClick commented Oct 1, 2023

relies on imap to be merged first

closes #212
closes #148

Summary

The following changes have been made to this PR, checked is tested.

  • Add ext-zip dependency.
  • Simplify cpp-extension detection.
  • Remove redundant LibraryBase::tryBuild() method and add patching status.
  • macOS library libiconv use .a directly for linking. (Why?)
  • Add imap extension support.
  • Use cross-make toolchain for non-alpine Linux.
  • After building embed for Linux, replace php-config wrong order.
  • Use -fPIE for embed SAPI. (Improve libphp security?)
  • Adjust Linux distro detection.
  • Fix icu C++ support for Linux (Here I would rather not hard-code the path to libstdc++.a).
  • Fix libxml2 with icu errors #94 (libxml2 with icu support, please write some comments).
  • Add library imap support for Linux. (It is recommended to separate the mac and Linux build methods)
  • Add LIBS for library ldap. (Maybe we could use pkg-config to generate it in the future)
  • Add library libwebp patches for pkg-config.
  • Fix library libzip pkg-config libs order.
  • Fix C++ support for library postgresql.
  • Add suggested library ldap, libpam for postgresql. (Bug: getLib('pam') -> getLib('libpam'))
  • Make --with-clean working for build command.
  • Add patchFile method.

src/SPC/builder/linux/library/libxml2.php Outdated Show resolved Hide resolved
src/SPC/builder/unix/library/snappy.php Outdated Show resolved Hide resolved
src/SPC/command/DownloadCommand.php Outdated Show resolved Hide resolved
src/SPC/doctor/item/LinuxMuslCheck.php Outdated Show resolved Hide resolved
src/SPC/builder/linux/LinuxBuilder.php Outdated Show resolved Hide resolved
src/SPC/builder/linux/library/icu.php Outdated Show resolved Hide resolved
@DubbleClick DubbleClick marked this pull request as draft October 1, 2023 21:11
@DubbleClick DubbleClick marked this pull request as ready for review October 1, 2023 21:46
@crazywhalecc crazywhalecc added wip Work In Process kind/dependency Issues related to dependencies os/linux Things only for Linux OS labels Oct 2, 2023
@crazywhalecc
Copy link
Owner

I haven't gotten home yet. It seems that it is feasible to implement C++ using cross-compilation, but I am not very clear about it. It seems that just replacing CXX is enough?

@DubbleClick
Copy link
Contributor Author

It's not really cross compilation (we could do that too, but no need). It essentially compiles gcc/g++ targeting musl libc/libstdc++. Then we use these compilers to compile libs and php. It's essentially what happens on Alpine, except that the pre-installed ones there already target musl.

@DubbleClick
Copy link
Contributor Author

last commit closes #215 #216

@crazywhalecc
Copy link
Owner

It seems that this PR is based on the PR of imap. I will check the imap part first and then merge this.

@crazywhalecc crazywhalecc marked this pull request as draft October 3, 2023 01:03
composer.json Outdated Show resolved Hide resolved
@crazywhalecc
Copy link
Owner

I feel like testing all of these features at once in a PR would be disastrous, but taking them apart would also be a huge amount of work.

My idea is that this PR can be kept first. For some pre-functions that can work stably (such as doctor, cross-make-gcc), start from the main branch and make specific modifications. Then I will quickly review and merge it into main branch, and then merge the main branch into the current branch to reduce the number of files modified by this branch.

@crazywhalecc
Copy link
Owner

I merged some general small fixes into the main branch first to reduce the amount of file changes in this branch.

@DubbleClick
Copy link
Contributor Author

I feel like testing all of these features at once in a PR would be disastrous, but taking them apart would also be a huge amount of work.

My idea is that this PR can be kept first. For some pre-functions that can work stably (such as doctor, cross-make-gcc), start from the main branch and make specific modifications. Then I will quickly review and merge it into main branch, and then merge the main branch into the current branch to reduce the number of files modified by this branch.

I agree, I will pick it apart into a bunch of separate branches as well as possible once everything is working. I'm not yet sure how to fix the musl-compiler issue. Worst case we can't support intl on Debian yet.

@DubbleClick
Copy link
Contributor Author

@crazywhalecc what do you think is the best course of action for the musl compilers - alpine always stays the same and directly uses gcc / g++.

A) Debian + RHEL: Use prebuilt musl-native toolchains from https://musl.cc - problem: intl extension causes a segfault that I do not understand.
B) RHEL: Built musl-cross-make from https://github.com/richfelker/musl-cross-make - everything works. Debian: Use prebuilt musl-native toolchain from musl.cc - intl doesn't work. - Cannot manage to build musl-cross-make on Debian.
C) Upload and use a gcc 11.2.0 version of musl-cross-make that we compiled ourselves - works on both RHEL and Debian.
D) Do not support c++ extensions on debian.
E) Drop Debian support entirely (also simplifies imap since rhel/alpine builds do not require libpam).
F) Only support c++ extensions build on alpine (what we have now).

@crazywhalecc
Copy link
Owner

I also want to introduce C++ compilation support in Debian and RHEL, but if it is difficult or complicated, I think it is better to "only support compilation of C++ extensions under Alpine".

Because the original bash-version is somewhat similar to the current version, which is to use the system's compilation tool chain as directly as possible. Compilation of these system-level libraries will first produce certain changes in the system itself. I certainly hope that future spc binaries can run on as many distributions as possible, but the current situation is that the distributions are very different, and it is impossible to make every distribution support 100% of all compilation features.

But don't give up, I think if cross-make can compile most extensions under RHEL and Debian, and the cross-make package can be easily installed, we can document it as an optional feature, or adding an additional compile command.

@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 15, 2023

C++ support under RHEL and Debian is working with the musl toolchains. Just intl extension segfaults when using the pre-built versions from musl.cc - maybe I can compile a version that we host somewhere that spc can download and use?

RHEL can also download musl-cross-make from source and compile it, just Debian can't and I don't know why.

A-F are all options. I favour uploading our own prebuilt musl toolchain and have spc download it.

@crazywhalecc
Copy link
Owner

Is your idea to pre-compile the Linux musl binary and related libraries, and then let spc download them directly for use?

I don't know much about your local progress. If they can be compiled successfully on a certain Linux distribution and they can be transplanted to other distributions to run normally, it may be a good choice. If the musl related source code is mirrored on GitHub, we can fork it and build it in Actions, and then use spc's Downloader to download it from the Release file.

These are the solutions I can think of so far.

@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 15, 2023

Yes, we can compile musl-cross-make, tar the produced output and have spc download it. Compiling musl-cross-make from source doesn't work on Debian (I do not know why, but after manually fixing like 10 different errors I gave up). But Debian can use the downloaded musl toolchain.

I don't think there is much point in compiling musl-cross-make in GitHub actions, though. Just compile it once, host it somewhere and let spc download it. It currently already does this (see MuslCheck), downloading the prebuilt version on musl.cc. Just intl doesn't work with it, so we could compile a version ourselves instead. This is recommended by the author of musl.cc anyway.

@DubbleClick DubbleClick reopened this Oct 15, 2023
@DubbleClick
Copy link
Contributor Author

DubbleClick commented Oct 15, 2023

Found the issue, intl doesn't work when gcc was configured with --enable-default-pie. I created a bug report in the php repo.

In the meantime, I've compiled a x86 toolchain without it that we can use. Will compile one for arm too.

@DubbleClick
Copy link
Contributor Author

had to introduce an option to disable opcache-jit on alpine-x86 to compile frankenphp (dunglas disabled it in general before)

/usr/local/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/lib/gcc/x86_64-alpine-linux-musl/12.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/static-php-cli/buildroot/lib/pkgconfig/../../lib/libphp.a(zend_jit.o): TLS transition from R_X86_64_TLSGD to R_X86_64_GOTTPOFF against `_tsrm_ls_cache' at 0x7465d in section `.text' failed
/usr/lib/gcc/x86_64-alpine-linux-musl/12.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status

@DubbleClick
Copy link
Contributor Author

I will pick this abomination of a branch into different pr's now

@DubbleClick
Copy link
Contributor Author

first pr with only the c++ support is done

I took a different approach to fixing the lib64 problem this time, so we don't have to copy or check in every individual extension that may build to lib64 rather than lib.

@crazywhalecc
Copy link
Owner

If a new imap branch is opened separately, this branch should be closed, right?

@DubbleClick
Copy link
Contributor Author

I kept it up for libpam reference, if imap is merged without libpam we can close this

@DubbleClick DubbleClick changed the title imap and C++ support on all linux (aarch64 & x86_64) systems reference pr that shall not be merged Oct 31, 2023
@DubbleClick DubbleClick deleted the cplusplus branch November 13, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/dependency Issues related to dependencies mixed PR This PR contains multiple updates os/linux Things only for Linux OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C++ extension incompatibility with linux imagick not working on Linux libxml2 with icu errors
3 participants