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

Jump to bottom

php < 7.4: Fix validation of PKG_CONFIG var #90249

Merged
merged 1 commit into from Jun 14, 2020
Merged

php < 7.4: Fix validation of PKG_CONFIG var #90249

merged 1 commit into from Jun 14, 2020

Conversation

Ericson2314
Copy link
Member

Motivation for this change

They were assuming it is an absolute path, but it (conventionally) can
be something to look up on the PATH too.

Fixes #90202

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ericson2314
php < 7.4: Fix validation of PKG_CONFIG var
c1b9f86 
They were assuming it is an absolute path, but it (conventionally) can
be something to look up on the PATH too.

Fixes NixOS#90202
@ofborg ofborg bot requested review from talyz, aanderse, etu, globin and Ma27 June 13, 2020 19:15
@Ericson2314 Ericson2314 mentioned this pull request Jun 13, 2020
Closed
@Izorkin
Copy link
Contributor

Izorkin commented Jun 14, 2020

@GrahamcOfBorg build php72 php73 php74

@Izorkin
Copy link
Contributor

Izorkin commented Jun 14, 2020

Need add this path to pkgs/top-level/php-packages.nix ?

In the build log lot of warning mesages:

...
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './Zend/acinclude.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './Zend/Zend.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/libxml/config0.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/dba/config.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/xml/config.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/tidy/config.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/date/config0.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/json/config.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/pdo_sqlite/config.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/xmlrpc/libxmlrpc/acinclude.m4'
substituteStream(): WARNING: pattern 'test -x "$PKG_CONFIG"' doesn't match anything in file './ext/xmlrpc/libxmlrpc/xmlrpc.m4'
...

It is possible to replace this patch? -

diff --git a/acinclude.m4 b/acinclude.m4
index 0690ec7d..3eb8b7c9 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -2208,7 +2208,7 @@ AC_DEFUN([PHP_SETUP_ICU],[
   fi

   dnl If pkg-config is found try using it
-  if test "$PHP_ICU_DIR" = "DEFAULT" && test -x "$PKG_CONFIG" && $PKG_CONFIG --exists icu-uc icu-io icu-i18n; then
+  if test "$PHP_ICU_DIR" = "DEFAULT" && type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists icu-uc icu-io icu-i18n; then
     if $PKG_CONFIG --atleast-version=4.0 icu-uc; then
       found_icu=yes
       icu_version_full=`$PKG_CONFIG --modversion icu-uc`
@@ -2361,7 +2361,7 @@ AC_DEFUN([PHP_SETUP_OPENSSL],[
   fi

   dnl If pkg-config is found try using it
-  if test "$PHP_OPENSSL_DIR" = "yes" && test -x "$PKG_CONFIG" && $PKG_CONFIG --exists openssl; then
+  if test "$PHP_OPENSSL_DIR" = "yes" && type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists openssl; then
     if $PKG_CONFIG --atleast-version=1.0.1 openssl; then
       found_openssl=yes
       OPENSSL_LIBS=`$PKG_CONFIG --libs openssl`
@@ -2596,7 +2596,7 @@ AC_DEFUN([PHP_SETUP_LIBXML], [
     fi

     dnl If pkg-config is found try using it
-    if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libxml-2.0; then
+    if type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libxml-2.0; then
       if $PKG_CONFIG --atleast-version=2.6.11 libxml-2.0; then
         found_libxml=yes
         LIBXML_LIBS=`$PKG_CONFIG --libs libxml-2.0`
diff --git a/ext/curl/config.m4 b/ext/curl/config.m4
index 10055837..c13fc413 100644
--- a/ext/curl/config.m4
+++ b/ext/curl/config.m4
@@ -10,7 +10,7 @@ if test "$PHP_CURL" != "no"; then
     AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
   fi

-  if test -x "$PKG_CONFIG"; then
+  if type -P "$PKG_CONFIG" >/dev/null; then
     dnl using pkg-config output

     AC_MSG_CHECKING(for libcurl.pc)
diff --git a/ext/odbc/config.m4 b/ext/odbc/config.m4
index 21fd94c8..fb77ffe5 100644
--- a/ext/odbc/config.m4
+++ b/ext/odbc/config.m4
@@ -395,7 +395,7 @@ PHP_ARG_WITH(iodbc,,
     if test -z "$PKG_CONFIG"; then
       AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
     fi
-    if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libiodbc ; then
+    if type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libiodbc ; then
       PHP_ADD_LIBRARY_WITH_PATH(iodbc, $PHP_IODBC/$PHP_LIBDIR)
       ODBC_TYPE=iodbc
       ODBC_INCLUDE=`$PKG_CONFIG --cflags-only-I libiodbc`
diff --git a/ext/pdo_pgsql/config.m4 b/ext/pdo_pgsql/config.m4
index 1b6dd0e1..2c75063d 100644
--- a/ext/pdo_pgsql/config.m4
+++ b/ext/pdo_pgsql/config.m4
@@ -74,7 +74,7 @@ if test "$PHP_PDO_PGSQL" != "no"; then
     AC_MSG_RESULT([yes])
     dnl First try to find pkg-config
     AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
-    if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists openssl; then
+    if type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists openssl; then
       PDO_PGSQL_CFLAGS=`$PKG_CONFIG openssl --cflags`
     fi
   else
diff --git a/ext/sodium/config.m4 b/ext/sodium/config.m4
index 9388fc21..225f7e2f 100644
--- a/ext/sodium/config.m4
+++ b/ext/sodium/config.m4
@@ -17,7 +17,7 @@ if test "$PHP_SODIUM" != "no"; then
     AC_MSG_RESULT([found in $PHP_SODIUM])

   dnl pkg-config output
-  elif test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libsodium; then
+  elif type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libsodium; then
     LIBSODIUM_VERSION=`$PKG_CONFIG libsodium --modversion`
     if $PKG_CONFIG libsodium --atleast-version=1.0.8; then
       LIBSODIUM_CFLAGS=`$PKG_CONFIG libsodium --cflags`
diff --git a/ext/zip/config.m4 b/ext/zip/config.m4
index 58c78538..7f68011d 100644
--- a/ext/zip/config.m4
+++ b/ext/zip/config.m4
@@ -62,7 +62,7 @@ if test "$PHP_ZIP" != "no"; then
       LIBZIP_LIBDIR="$PHP_LIBZIP/$PHP_LIBDIR"
       AC_MSG_RESULT(from option: found in $PHP_LIBZIP)

-    elif test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libzip; then
+    elif type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libzip; then
       if $PKG_CONFIG libzip --atleast-version 0.11; then
         LIBZIP_CFLAGS=`$PKG_CONFIG libzip --cflags`
         LIBZIP_LIBDIR=`$PKG_CONFIG libzip --variable=libdir`
diff --git a/sapi/fpm/config.m4 b/sapi/fpm/config.m4
index 4cf4ba49..864c707d 100644
--- a/sapi/fpm/config.m4
+++ b/sapi/fpm/config.m4
@@ -593,7 +593,7 @@ if test "$PHP_FPM" != "no"; then
     unset SYSTEMD_LIBS
     unset SYSTEMD_INCS

-    if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libsystemd; then
+    if type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libsystemd; then
       dnl systemd version >= 209 provides libsystemd
       AC_MSG_CHECKING([for libsystemd])
       SYSTEMD_LIBS=`$PKG_CONFIG --libs libsystemd`
@@ -601,7 +601,7 @@ if test "$PHP_FPM" != "no"; then
       SYSTEMD_VERS=`$PKG_CONFIG --modversion libsystemd`
       AC_MSG_RESULT([version $SYSTEMD_VERS])

-    elif test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libsystemd-daemon; then
+    elif type -P "$PKG_CONFIG" >/dev/null && $PKG_CONFIG --exists libsystemd-daemon; then
       dnl systemd version < 209 provides libsystemd-daemon
       AC_MSG_CHECKING([for libsystemd-daemon])
       SYSTEMD_LIBS=`$PKG_CONFIG --libs libsystemd-daemon`

timokau
timokau approved these changes Jun 14, 2020
View reviewed changes
Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

I propose we prioritize fixing the breakage asap and merge this now. The warnings could be taken care of in a follow up PR.

pkgs/development/interpreters/php/default.nix
preConfigure = ''
# Don't record the configure flags since this causes unnecessary
# runtime dependencies
preConfigure =
Copy link
Member

Choose a reason for hiding this comment

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

It would probably have been better to skip the whitespace changes here, since they will cause rebuilds for pho >= 7.4. ofBorg says its in the <=500 range though, which should be okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, as this is a rush fix.

I should put in some docs somewhere that comments outside of string literals is preferred for sake of rebuilds, so as I am not tempted to do this on a case-by-case basis so much :).

@timokau
Copy link
Member

timokau commented Jun 14, 2020

Merging, since only already broken derivations are affected (plus whitespace changes).

@timokau timokau merged commit 20e62aa into NixOS:master Jun 14, 2020
@Ericson2314 Ericson2314 deleted the fix-old-php branch June 14, 2020 19:06
@pbogdan pbogdan mentioned this pull request Aug 16, 2020
Closed
@jtojnar jtojnar mentioned this pull request May 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timokau timokau timokau approved these changes

@talyz talyz Awaiting requested review from talyz talyz is a code owner

@aanderse aanderse Awaiting requested review from aanderse aanderse is a code owner

@etu etu Awaiting requested review from etu

@globin globin Awaiting requested review from globin globin is a code owner

@Ma27 Ma27 Awaiting requested review from Ma27 Ma27 is a code owner

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

php: configure error - your system does not support systemd
3 participants
@Ericson2314 @Izorkin @timokau