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

rhel support & ldap support #193

Merged
merged 13 commits into from
Sep 30, 2023
Merged

Conversation

DubbleClick
Copy link
Collaborator

@DubbleClick DubbleClick commented Sep 18, 2023

I wasn't quite able to get ldap to work when building static-php-cli, but it's trivial on rhel/debian - just run

dnf install sqlite-devel openldap-devel
ln -s /usr/lib64/libldap.so /usr/lib/libldap.so && ln -s /usr/lib64/liblber.so /usr/lib/liblber.so
php-src-folder/configure --with-ldap=/usr

openldap-devel is called libldap2-dev on debian.

Edit: If you want to add a pipeline for rhel (not required imo) this is the full install script to get things running on rhel.
epel-release is required for most of the things to install, remi only for php 8.2. The default still downloads 8.0 by default, which doesn't work.

dnf install epel-release
dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-9.noarch.rpm 
dnf install -y https://rpms.remirepo.net/enterprise/remi-release-9.rpm  
dnf module enable php:remi-8.2
dnf install tar git zip php php-sodium php-zip libsodium wget

cd /opt
git clone https://github.com/crazywhalecc/static-php-cli.git
wget https://musl.libc.org/releases/musl-1.2.4.tar.gz

tar -xzf musl-1.2.4.tar.gz
cd musl-1.2.4.tar.gz
./configure
make -j
make install
cd ../static-php-cli

dnf install bison flex bzip2 cmake patch
dnf install sqlite-devel openldap-devel
ln -s /usr/lib64/libldap.so /usr/lib/libldap.so && ln -s /usr/lib64/liblber.so /usr/lib/liblber.so

php-src-dir/configure --with-ldap=/usr

@jingjingxyk
Copy link
Contributor

@crazywhalecc
Copy link
Owner

@DubbleClick Only libraries supported by static compilation can be added to static-php-cli. Currently, all packages installed by dnf are dynamically linked. So the core point is to write a method that can statically compile the openldap library.

@crazywhalecc crazywhalecc added new feature New feature or request wip Work In Process kind/extension Issues related to extensions labels Sep 18, 2023
@mhpcc mhpcc force-pushed the main branch 3 times, most recently from 6335252 to 6a9ec14 Compare September 19, 2023 09:39
@DubbleClick DubbleClick changed the title WIP: ldap support ldap support Sep 20, 2023
@DubbleClick
Copy link
Collaborator Author

ready for review now, I think everything is working as it should

@DubbleClick DubbleClick changed the title ldap support rhel support & ldap support Sep 20, 2023
@DubbleClick
Copy link
Collaborator Author

For what it's worth, I've only required perl for RHEL distros now, but I think it should be a requirement for debian and alpine too. Doesn't seem like a good idea to rely on perl being installed by default.

@DubbleClick DubbleClick force-pushed the main branch 2 times, most recently from 2933374 to 92942ea Compare September 20, 2023 15:50
@DubbleClick
Copy link
Collaborator Author

Cleaned up a few last things: enabled ldap for MacOS and moved the source to /php-src/ext/ldap.

@crazywhalecc crazywhalecc added need test This PR has not been tested yet, cannot merge now and removed wip Work In Process labels Sep 21, 2023
only enable openssl when zlib ext is also enabled (missing 'deflate' otherwise)
move back from source/php-src/ext/ldap to source/ldap (fix "LICENSE not found")
@DubbleClick
Copy link
Collaborator Author

When compiling ldap with openssl support, the zlib extension needs to be enabled. If compiled without openssl support, zlib is not required. I didn't put zlib in ext-requires because of that and instead check for openssl lib & zlib extension before enabling --with-tls

@crazywhalecc
Copy link
Owner

crazywhalecc commented Sep 23, 2023

For what it's worth, I've only required perl for RHEL distros now, but I think it should be a requirement for debian and alpine too. Doesn't seem like a good idea to rely on perl being installed by default.

Debian and Alpine has installed perl I guess (with current dependencies).

@crazywhalecc
Copy link
Owner

When compiling ldap with openssl support, the zlib extension needs to be enabled. If compiled without openssl support, zlib is not required. I didn't put zlib in ext-requires because of that and instead check for openssl lib & zlib extension before enabling --with-tls

My understanding: If you want ldap to support openssl, then it must include the zlib library. This can actually be solved like this: openssl extension adds ext-depends: zlib.

@DubbleClick
Copy link
Collaborator Author

DubbleClick commented Sep 23, 2023

Correct, but Openssl extension itself doesn't require zlib extension.

Ldap without tls needs nothing.
Ldap with tls needs openssl lib, zlib lib, zlib ext.
Openssl needs zlib lib.

@crazywhalecc
Copy link
Owner

Although the openssl extension does not require the zlib extension, according to the current extension inclusion logic, the zlib extension should be enabled when you include the zlib library, so this is reasonable.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Sep 23, 2023

Documentation Task

  • extension.md
  • CliGenerator.vue
  • doctor for RHEL part

@crazywhalecc crazywhalecc mentioned this pull request Sep 23, 2023
36 tasks
@DubbleClick
Copy link
Collaborator Author

DubbleClick commented Sep 30, 2023

The doctor command doesn't have notes about specific operating systems. Rhel would just count under linux, I think we have nothing to document there.
I've added the ldap extension to extensions.md (en an zh)

Copy link
Owner

@crazywhalecc crazywhalecc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested on my local AlmaLinux VM, and I found that the compilation part worked perfectly, but the doctor part needed some changes: The path was not added correctly when compiling musl. We need to manually write ~/.bashrc or execute export once in the current terminal.

@crazywhalecc crazywhalecc removed the need test This PR has not been tested yet, cannot merge now label Sep 30, 2023
@crazywhalecc crazywhalecc merged commit 8f43a09 into crazywhalecc:main Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/extension Issues related to extensions new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants