-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Fix #79026: Allow PHP_EXTRA_VERSION overrides #11706
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
Conversation
Kind of make sense. cc @iluuu1994 @petk ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice idea. This is cool addition, yes. Thanks @athos-ribeiro
892669b
to
4080e06
Compare
Thanks! @petk , I addressed your comments and re-based this on master. I was indeed wondering if it would be OK to declare it as a precious variable and ended up not doing it for consistency with the other vars around the same script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@athos-ribeiro the dnl
at the end of the line is to reduce empty newline in configure?
I think this is the most consistent way in this situation. It is available in the help output, variable is marked as precious (meaning it shouldn't be changed during the rest of the configure script) and we don't need to pass it to other macros because there won't be any substitution happening at this point.
This is ready to merge, if you're happy with the code style. Maybe just sync the space between the arguments of the two macro calls. Your call. Thank you for the patch.
When building from sources, someone distributing PHP may want to add a vendor specific string to the PHP_VERSION so users can differentiate multiple vendor builds from the same PHP version. For instance, a vendor backporting a bug fix to a no-longer-supported PHP version could extend their PHP_EXTRA_VERSION to allow their users to identify that they carry such fix by checking their PHP_VERSION.
4080e06
to
e779563
Compare
Thanks, @petk ! Yes, the dnl in the end of the line to to avoid generating an extra empty line in the final I sync'd the style as you suggested and re-based this on master again. +1 on merging this as is. |
If it's fine with everyone, I'll merge this one also later today. I'll update the changelog... I think this is good addition and it resolves a bug. |
When building from sources, someone distributing PHP may want to add a vendor specific string to the PHP_VERSION so users can differentiate multiple vendor builds from the same PHP version. For instance, a vendor backporting a bug fix to a no-longer-supported PHP version could extend their PHP_EXTRA_VERSION to allow their users to identify that they carry such fix by checking their PHP_VERSION.