Skip to content

Use --enable-gd instead of --with-gd for PHP 7.4#1027

Merged
jhdxr merged 1 commit into
phpbrew:masterfrom
morozov:issues/1020
Jul 25, 2019
Merged

Use --enable-gd instead of --with-gd for PHP 7.4#1027
jhdxr merged 1 commit into
phpbrew:masterfrom
morozov:issues/1020

Conversation

@morozov

@morozov morozov commented Jun 17, 2019

Copy link
Copy Markdown
Contributor

Additionally, introduced the Include and Brew prefix finders to reduce the amount of boilerplate code.

Fixes #1020.

@coveralls

coveralls commented Jun 17, 2019 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.6%) to 40.908% when pulling 542b81f on morozov:issues/1020 into 186ee90 on phpbrew:master.

@morozov morozov requested review from c9s and jhdxr June 17, 2019 03:48
@jhdxr

jhdxr commented Jun 17, 2019

Copy link
Copy Markdown
Member

I suggest to keep this PR and #1026 #1025 #1024 open until PHP-7.4 reach its first beta (should be July 25th, see https://wiki.php.net/todo/php74) since these options may change again, e.g. php/php-src@b654a07

@morozov

morozov commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for the info. Let's wait.

c9s
c9s approved these changes Jun 20, 2019
View reviewed changes

@c9s c9s left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lgtm

jhdxr
jhdxr reviewed Jun 29, 2019
View reviewed changes

@jhdxr jhdxr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The original implement has warnings for users when brew is installed but necessary library is not installed. the new implement dropped these warnings. is it intentional?

@morozov

morozov commented Jun 29, 2019

Copy link
Copy Markdown
Contributor Author

You're right, I unintentionally dropped one of the warnings. We need two of them for Homebrew:

  1. One for the situation when the expected formula is not found (this was accidentally dropped).
  2. The other when the formula is found but the prefix doesn't exist.

Additionally, I must have introduced a bug:

phpbrew/src/PhpBrew/PrefixFinder/BrewPrefixFinder.php

Line 45 in 19954a2

printf('Homebrew prefix "%s" does not exist.' . PHP_EOL, $output);

It should display the prefix reported by Homebrew if it's not empty, not just the output.

Let me fix it and thanks for the review.

@morozov
Use --enable-gd instead of --with-gd for PHP 7.4
542b81f 
Additionally, introduced the Include and Brew prefix finders to reduce the amount of boilerplate code.
@morozov

morozov commented Jul 23, 2019

Copy link
Copy Markdown
Contributor Author

https://news-web.php.net/php.internals/106249 — php7.4beta1 is tagged.

@jhdxr jhdxr merged commit bf04c50 into phpbrew:master Jul 25, 2019
@jhdxr

jhdxr commented Jul 25, 2019

Copy link
Copy Markdown
Member

merged. all green. thanks.

@morozov morozov deleted the issues/1020 branch July 25, 2019 13:30
@morozov morozov self-assigned this Nov 1, 2019
@morozov morozov added this to the Release 1.24 milestone Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhdxr jhdxr jhdxr left review comments
@c9s c9s c9s approved these changes

Assignees

@morozov morozov

Labels

Improvement PHP 7.4

Projects

None yet

Milestone

Release 1.24.0

Development

Successfully merging this pull request may close these issues.

PHP 7.4 will require --enable-gd instead of --with-gd

4 participants

@morozov @coveralls @jhdxr @c9s