Skip to content

ext/gettext: resolve underqouting that breaks with autoconf 2.72 #11427

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

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

the-eater
Copy link
Contributor

With some changes in autoconf 2.72 there is the chance that the AC_CHECK_LIB will output a comment with a comma which will be interpreted as being part of the upper AC_CHECK_LIB invocation, and thus only output half of the if statement, causing cryptic errors alike

configure: 101400: Syntax error: end of file unexpected (expecting "fi")

By qouting the AC_CHECK_LIB, this issue is resolved

(also I am very confused by the indenting in the m4 files, it seems to be mixed tabs and spaces?)

@the-eater
Copy link
Contributor Author

CI failure looks unrelated to my changes?

@nielsdos
Copy link
Member

CI failure looks unrelated to my changes?

Indeed. Sometimes tests in CI can fail due to the high I/O load, it's just CI flakiness.

@petk
Copy link
Member

petk commented Jun 17, 2023

I've used the unreleased autoconf 2.72c version and it seems to be working fine also with non-patched code.

Otherwise, I agree that this needs to be quoted. Change looks ok and we can definitely add this one. All other similar AC_CHECK_LIB calls are quoted in the php-src code, I think. And also those tabs around this code should be changed to spaces.

I'll recheck this a bit in the following days. Maybe we should even target PHP-8.1 branch here.

@the-eater
Copy link
Contributor Author

the-eater commented Jun 18, 2023

yeah for it to actually fail, it requires not having intl installed/available, it's a bit of a edge case on edge case situation right now

EDIT: after looking around a bit, it seems to be because we use gettext-tiny instead of GNU gettext, which provides the intl

@petk
Copy link
Member

petk commented Jun 24, 2023

I guess we'll need to adjust also buildconf script a bit to check for development versions of Autoconf. When Autoconf is of version 2.72c or similar:

 # Check if autoconf exists.
-ac_version=$($PHP_AUTOCONF --version 2>/dev/null|head -n 1|sed -e 's/^[^0-9]*//' -e 's/[a-z]* *$//')
+ac_version=$($PHP_AUTOCONF --version 2>/dev/null|head -n 1|sed 's/.* \([0-9.]*[0-9]\).*/\1/')

But this is for another PR and needs to be fined tuned more.

Thanks @the-eater for the patch of gettext.

@petk petk merged commit 7236dfe into php:master Jul 18, 2023
@petk
Copy link
Member

petk commented Jul 18, 2023

Merged to master. Coming up in PHP 8.3. Thank you @the-eater

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.

4 participants