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

Can't cross-compile with external libcrypt #14284

Closed
orlitzky opened this issue May 21, 2024 · 6 comments
Closed

Can't cross-compile with external libcrypt #14284

orlitzky opened this issue May 21, 2024 · 6 comments

Comments

@orlitzky
Copy link
Contributor

Description

At https://bugs.gentoo.org/931884 we have a bug report that boils down to the external libcrypt (./configure --with-external-libcrypt) checks running e.g.

  AC_CACHE_CHECK(for extended DES crypt, ac_cv_crypt_ext_des,[
    AC_RUN_IFELSE([AC_LANG_SOURCE([[
      ...                                                                          
    ]])
    ],[
      ac_cv_crypt_ext_des=yes
    ],[
      ac_cv_crypt_ext_des=no
    ],[
      ac_cv_crypt_ext_des=no
    ])
  ])

where the last case is executed for cross-compiles and forces the feature to undetected (which, in this case, is fatal). We have a few options to work around this within Gentoo, but I thought it was worth asking about upstream approach more generally. AC_RUN_IFELSE is used in a few places, and I think it would be better if the cross-compile branch were kept consistent to whatever extent that's possible.

For another example, take ext/iconv/config.m4:

AC_MSG_CHECKING([if iconv supports errno])
AC_RUN_IFELSE(...
[
  AC_MSG_RESULT(yes, cross-compiling)
])

Or ext/pcre/config0.m4:

AC_CACHE_CHECK([for JIT support in PCRE2], ac_cv_have_pcre2_jit, [
  AC_RUN_IFELSE(...
  [
    AC_CANONICAL_HOST
    case $host_cpu in
      arm*|i[[34567]]86|x86_64|mips*|powerpc*|sparc)
        ac_cv_have_pcre2_jit=yes
        ;;
      *)
	ac_cv_have_pcre2_jit=no
        ;;
    esac
  ])

This is important to me because I'm planning on making --with-external-gd the default soon, and I myself am guilty of writing a test that disables certain formats when cross-compiling because they cannot be detected with AC_RUN_IFELSE. This bug report has given me second thoughts about doing that.

ping @remicollet

PHP Version

git HEAD

Operating System

No response

@petk
Copy link
Member

petk commented May 21, 2024

Cross-compiling in PHP needs a lot of improvements indeed. Yes, all those checks should be wrapped in AC_CACHE_CHECK so the php_cv_* variables can be passed manually to help the configuration step determine the platform.

About these crypt algorithms checks I'm not sure how else could they be determined but to run the test program. Probably we can start filtering the platforms in case of cross-compiling and pass those that are certain to have these.

@orlitzky
Copy link
Contributor Author

orlitzky commented Jun 2, 2024

Thanks, I think using the cache variables is a fine approach where no better way to write the checks is apparent. I'll work on caching the format checks for the system libgd, and then afterwards solve both problems by overriding the cache variables.

@petk
Copy link
Member

petk commented Jun 2, 2024

The libgd checks can be done like this: #14443
I'm still fixing two AC_RUN_IFELSE checks in fpm and two left overs in configure.ac.

Yes, to cross-compile (for the time being) can be done like this:

./configure --host=<target-triplet> --enable-gd --with-external-gd \
  php_cv_lib_gd_gdImageCreateFromPng=yes \
  php_cv...=...

@orlitzky
Copy link
Contributor Author

orlitzky commented Jun 2, 2024

The libgd checks can be done like this: #14443 I'm still fixing two AC_RUN_IFELSE checks in fpm and two left overs in configure.ac.

Oh, you already did it, thank you!

@petk
Copy link
Member

petk commented Jul 11, 2024

Just a quick update here because changing everything takes several steps. So far many AC_RUN_IFELSE checks were wrapped in the AC_CACHE_CHECK so the cache variables can be used. The external libcrypt simplifications are done in this PR: #14791 which introduces a new single cache variable for all required crypt algos: php_cv_lib_crypt_algos=yes|no instead of setting 6 cache variables just for the external crypt library.

And what I'd suggest for the time being is to go with these. These run checks cannot be changed to link or compile checks at this point I believe. If you find anything else to simplify or if some known platform can directly override some check and can skip setting the cache variable, please send a PR or open an issue for that. This will be further checked in the future also if more simplifications are possible.

@orlitzky
Copy link
Contributor Author

I pushed 8.3.9 to our users with the cache variables set yesterday morning. I haven't heard anything yet, but I think that should fix this particular issue. I switched to the system libgd at the same time and took advantage of the cache variables there, too. Thanks for all the work you did to make this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants