Skip to content

Conversation

@remicollet
Copy link
Member

@remicollet remicollet commented May 19, 2025

Using PHP_LIBDIR for lib64 case
Fix AS_CASE for libdir and datadir broken by quotes

Only to master to avoid file placement in stable release
in some build, FPM status page will move from /usr/share/fpm to /usr/share/php/fpm

P.S. it seems the quote are there since always (at least in 5.6.0)

Using PHP_LIBDIR for lib64 case
Fix AS_CASE for libdir and datadir broken by quotes
@remicollet
Copy link
Member Author

Notice: applied in Fedora, it allows to build embed SAPI from the same tree than fpm (so reducing global build time)
=> https://src.fedoraproject.org/rpms/php/c/6710dea4031d586ce479cda8a518c132715cf9a8?branch=rawhide

@remicollet
Copy link
Member Author

@petk as you are one of the few playing with build system, can you please review this one ?

@remicollet remicollet requested a review from petk May 26, 2025 09:48
@petk
Copy link
Member

petk commented May 26, 2025

I think that's good, yes. I'll have to retest this a bit to refresh myself about this. Yes, I think it's about time that these installation directories get refactored, even if it is a BC break. So, for master (upcoming PHP 8.5) that's ideal timing.

Copy link
Member

@petk petk left a comment

Choose a reason for hiding this comment

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

I haven't forgotten about this PR. :D I was testing this a bit and I honestly don't fully understand it why it doesn't work ok. Because, before this change the php string gets appended to the libdir variable, and after this change it actually doesn't. That case condition doesn't seem to work ok, yet. But I think it won't cause any major issues, if this is added.

To not hold this PR waiting for too long, I think it's fine. What could be improved here in the future is to split that php subdirectory into a separate configuration option/variable and leave the libdir be changed accordingly (either with --with-libdir option or to the default by Autotools).

Yes, for the embed SAPI, it currently could go with program prefix/suffix for multiple versions of PHP installed...

So, yes, best to fix it for your build case and we'll go from there forward for any possible issues in other builds, such as Debian etc.

@remicollet
Copy link
Member Author

Because, before this change the php string gets appended to the libdir variable, and after this change it actually doesn't

What are your configure options ?

I can run more tests

such as Debian etc.

probably @oerdnj may help here

@petk
Copy link
Member

petk commented Jun 3, 2025

Try running default configuration and installation:

./configure
make
INSTALL_ROOT=... make install

and (depending on the system if that works) there are two different installation directory structures before and after this change. For example, notice, where the "build" directory gets installed. Currently, I can't confirm this exactly but I think that was the issue. I was trying even some of that $(echo $libdir) but I think it won't be enough for what you're trying to fix here. What should be probably done is to completely re-concatinate that libdir from scratch, so it has $PHP_LIBDIR at the end.

@remicollet
Copy link
Member Author

remicollet commented Jun 3, 2025

OK, I see, so we have to test both cases, unexpanded and expanded path

Please check with new commit

Debug output,
with no option

Before
configure: WARNING: libdir:     ${exec_prefix}/lib
configure: WARNING: libdireval: ${prefix}/lib
configure: WARNING: phplib:     lib
configure: WARNING: prefix:     /usr/local
configure: WARNING: exec:       ${prefix}
configure: WARNING: datadir:    ${datarootdir}
configure: WARNING: datadireval:${prefix}/share
configure: WARNING: rootdir:    ${prefix}/share
After
configure: WARNING: libdir:     ${exec_prefix}/lib/php
configure: WARNING: datadir:    ${datarootdir}/php

Debug output,
using --prefix=/usr --exec-prefix=/usr --datadir=/usr/share --libdir=/usr/lib64 --with-libdir=lib64

Before
configure: WARNING: libdir:     /usr/lib64
configure: WARNING: libdireval: /usr/lib64
configure: WARNING: phplib:     lib64
configure: WARNING: prefix:     /usr
configure: WARNING: exec:       /usr
configure: WARNING: datadir:    /usr/share
configure: WARNING: datadireval:/usr/share
configure: WARNING: rootdir:    ${prefix}/share
After
configure: WARNING: libdir:     /usr/lib64/php
configure: WARNING: datadir:    /usr/share/php

Copy link
Member

@petk petk left a comment

Choose a reason for hiding this comment

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

This works indeed.

@oerdnj
Copy link
Contributor

oerdnj commented Jun 3, 2025

Debian packages uses this (Makefile) syntax:

                --libdir=\$${prefix}/lib/php \
                --libexecdir=\$${prefix}/lib/php \
                --datadir=\$${prefix}/share/php/$(PHP_NAME_VERSION) \
                --sysconfdir=/etc \
                --localstatedir=/var \
                --mandir=/usr/share/man \

So, my guess is that the Debian packaging will be unaffected by this change.

@petk
Copy link
Member

petk commented Jun 3, 2025

So, my guess is that the Debian packaging will be unaffected by this change.

Yes. It doesn't affect it.

@remicollet
Copy link
Member Author

Thanks for help on this one

Squashed and merged as 6820a4d

@remicollet remicollet closed this Jun 4, 2025
@remicollet remicollet deleted the issue-libdir branch June 4, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants