Bug 217222 - graphics/php70-gd and relatives should not build bundled libgd
Summary: graphics/php70-gd and relatives should not build bundled libgd
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Torsten Zuehlsdorff
URL:
Keywords: patch
Depends on: 229707
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-19 07:01 UTC by Mikhail Teterin
Modified: 2018-07-31 12:56 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (tz)


Attachments
Patch php70-gd to build against graphics/gd (1.22 KB, patch)
2017-02-19 07:01 UTC, Mikhail Teterin
no flags Details | Diff
180132: Patch php70-gd to build against graphics/gd (3.41 KB, patch)
2017-04-14 17:27 UTC, Mikhail Teterin
no flags Details | Diff
Rebased patch (4.02 KB, patch)
2018-07-11 13:43 UTC, Torsten Zuehlsdorff
no flags Details | Diff
Reorder include paths so upgrades succeed (670 bytes, patch)
2018-07-11 19:29 UTC, Mikhail Teterin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Teterin freebsd_committer freebsd_triage 2017-02-19 07:01:40 UTC
Created attachment 180132 [details]
Patch php70-gd to build against graphics/gd

Although PHP bundles a version of libgd, it supported building against the already-installed library.

To ease code-maintenance and reduce diskspace and run-time memory requirements, the php-extensions should build against the existing library instead of the bundled version.

The attached patch does this for php-7.0 -- something very similar (if not outright identical) -- should work for 5.x and 7.1.

For an immediate benefit, this would obviate Bug 216654...
Comment 1 Mikhail Teterin freebsd_committer freebsd_triage 2017-04-14 17:27:44 UTC
Created attachment 181795 [details]
180132: Patch php70-gd to build against graphics/gd

This version removes the options which aren't really options for gd (any more).

It also adds a patch to ensure smooth upgrading of the port, when an earlier version (using bundled libgd) is already installed. That patch has been submitted upstream:

https://bugs.php.net/bug.php?id=74443

If you commit this together with upgrading lang/php70 to 7.0.18, no PORTREVISION bump will be necessary.
Comment 2 Walter Schwarzenfeld 2018-02-07 23:28:48 UTC
We have PORTVERSION=    7.0.27. Feedback please!
Comment 3 Torsten Zuehlsdorff freebsd_committer freebsd_triage 2018-05-18 11:20:10 UTC
Can you please rebase the patch? It doesn't work anymore. Also i think it is needed for php71 and php72 too.
Comment 4 Torsten Zuehlsdorff freebsd_committer freebsd_triage 2018-07-11 13:43:28 UTC
Created attachment 195051 [details]
Rebased patch

I rebased the patch now myself. I discovered some issues with the old one:
- the patch file is not recognized
- the LIB_DEPENDS to gd is wrongly defined
- LIB_DEPENDS is placed wrongly and did not work at all
- after fixing LIB_DEPENDS it wants USE_XORG+=x11 which does not seem too good to me as default

This makes me question, if it worked before. 

Since you added a test-target i tried to execute it. But it stuck around test 95 while failing various tests before. I'm not sure about the failure rate without the patch. Maybe it is this high, too - i did not check it.

Please review my patch and your purpose. With this many issues the solution is not compelling to me. At least i need a clear path to make sure everything is fine.

Greetings,
Torsten
Comment 5 Mikhail Teterin freebsd_committer freebsd_triage 2018-07-11 19:29:27 UTC
Created attachment 195058 [details]
Reorder include paths so upgrades succeed

(In reply to Torsten Zuehlsdorff from comment #4)

Thanks for doing this yourself, sorry, I got sidetracked once again.

> - the patch file is not recognized

I presume, you are referring to my new patch-inc-dir -- I'm attaching an updated version of it verbatim. It is necessary for people trying to build the port with an earlier version of it already installed.

See https://bugs.php.net/bug.php?id=74443

> after fixing LIB_DEPENDS it wants USE_XORG+=x11

Without that, gd will be unable to process .xpm files. You may be right in that it should not be the default...

> But it stuck around test 95 while failing various tests before

Sadly, it is not unusual for the tests to fail :( As for the test 95, it refers to a bug they discovered last November -- long after I submitted this PR

See https://bugs.php.net/bug.php?id=75571

The patch referenced in the above bug fixes libgd -- unfortunately, our graphics/gd does not include it...

Are all of the tests passing successfully, when the port is built with the bundled gd?

> Please review my patch and your purpose

My purpose (in this case) is to eliminate the use of bundled 3rd-party software, when a separate port of same already exists:

See https://www.freebsd.org/doc/en/books/porters-handbook/bundled-libs.html

That the exercise unmasked deficiencies elsewhere (such as in graphics/gd) is a good thing -- we can fix gd for _all_ users, not just gd...
Comment 6 Mikhail Teterin freebsd_committer freebsd_triage 2018-07-11 19:56:22 UTC
If you upgrade to 2.2.5 as suggested in PR 229707, the test 95 passes and the rest of the test-suit runs to completion -- albeit with failures.

At least some of the failures aren't meaningful, however... Thankfully, the test-suit exits with code 0 even when it finds some errors.
Comment 7 commit-hook freebsd_committer freebsd_triage 2018-07-31 08:43:11 UTC
A commit references this bug:

Author: tz
Date: Tue Jul 31 08:42:20 UTC 2018
New revision: 475982
URL: https://svnweb.freebsd.org/changeset/ports/475982

Log:
  graphics/php70-gd: Switch from bundled libgd to graphics/gd

  Currently the gd-module uses a bundled libgd, while most systems
  already provide the same library via graphics/gd.
  Therefore instead of adding the bundled library we use the
  port instead.

  PR:		217222
  Submitted by:	Mikhail Teterin  <mi@FreeBSD.org>

Changes:
  head/graphics/php70-gd/Makefile
  head/graphics/php70-gd/files/
  head/graphics/php70-gd/files/patch-config.m4
  head/lang/php70/Makefile.ext
Comment 8 commit-hook freebsd_committer freebsd_triage 2018-07-31 08:45:18 UTC
A commit references this bug:

Author: tz
Date: Tue Jul 31 08:44:57 UTC 2018
New revision: 475983
URL: https://svnweb.freebsd.org/changeset/ports/475983

Log:
  graphics/php71-gd: Switch from bundled libgd to graphics/gd

  Currently the gd-module uses a bundled libgd, while most systems
  already provide the same library via graphics/gd.
  Therefore instead of adding the bundled library we use the
  port instead.

  PR:		217222
  Submitted by:	Mikhail Teterin  <mi@FreeBSD.org>

Changes:
  head/graphics/php71-gd/Makefile
  head/graphics/php71-gd/files/
  head/graphics/php71-gd/files/patch-config.m4
  head/lang/php71/Makefile.ext
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-07-31 08:48:23 UTC
A commit references this bug:

Author: tz
Date: Tue Jul 31 08:47:34 UTC 2018
New revision: 475984
URL: https://svnweb.freebsd.org/changeset/ports/475984

Log:
  graphics/php72-gd: Switch from bundled libgd to graphics/gd

  Currently the gd-module uses a bundled libgd, while most systems
  already provide the same library via graphics/gd.
  Therefore instead of adding the bundled library we use the
  port instead.

  PR:		217222
  Submitted by:	Mikhail Teterin  <mi@FreeBSD.org>

Changes:
  head/graphics/php72-gd/Makefile
  head/graphics/php72-gd/files/
  head/graphics/php72-gd/files/patch-config.m4
  head/lang/php72/Makefile.ext
Comment 10 Torsten Zuehlsdorff freebsd_committer freebsd_triage 2018-07-31 09:01:08 UTC
Its now committed and also ported to PHP 7.1 and PHP 7.2

Thanks for your work and patience! :)

Greetings,
Torsten
Comment 11 Dani I. 2018-07-31 12:25:57 UTC
This commit breaks php-gd for every php-version here..

 cc -I../.. -I/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd/libgd -I. -I/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd -DPHP_ATOM_INC -I/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd/include -I/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd/main -I/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd -I/usr/local/php71/include/php -I/usr/local/php71/include/php/main -I/usr/local/php71/include/php/TSRM -I/usr/local/php71/include/php/Zend -I/usr/local/php71/include/php/ext -I/usr/local/php71/include/php/ext/date/lib -I/usr/local/include -I/usr/local/include/freetype2 -DHAVE_CONFIG_H -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -c /wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd/gd.c  -fPIC -DPIC -o .libs/gd.o
/wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd/gd.c:57:11: fatal error: 'X11/xpm.h' file not found
# include <X11/xpm.h>
          ^
1 error generated.
*** Error code 1

Stop.
make[1]: stopped in /wrkdirs/usr/ports/graphics/php71-gd/work/php-7.1.20/ext/gd
*** Error code 1

=======================
Options for php71-gd:
===> The following configuration options are available for php71-gd-7.1.20_1:
     TRUETYPE=on: Enable TrueType string function
     JIS=off: Enable JIS-mapped Japanese font support
     WEBP=on: Enable WebP image format support
     X11=off: Enable XPM support

Options for gd:
===> The following configuration options are available for libgd-2.2.5,1:
     FONTCONFIG=on: X11 font configuration support
     ICONV=off: Encoding conversion support via iconv
     XPM=off: XPM pixmap image format support
     WEBP=on: WebP image format support

Looks like the XPM=off is beeing ignored.. Please take a look at this asap.
Comment 12 commit-hook freebsd_committer freebsd_triage 2018-07-31 12:56:28 UTC
A commit references this bug:

Author: tz
Date: Tue Jul 31 12:55:44 UTC 2018
New revision: 475998
URL: https://svnweb.freebsd.org/changeset/ports/475998

Log:
  graphics/php7*-gd: fix broken build when XPM option is disabled

  The switch from bundled gd to graphics/gd broke the port
  if a user disabled the XPM option.

  PR:		230234 217222
  Submitted by:	Guido Falsi <madpilot@FreeBSD.org>
  Reported by:	Guido Falsi <madpilot@FreeBSD.org>,  Dani <i.dani@outlook.com>

Changes:
  head/graphics/php70-gd/files/patch-config.m4
  head/graphics/php70-gd/files/patch-gd.c
  head/graphics/php71-gd/files/patch-config.m4
  head/graphics/php71-gd/files/patch-gd.c
  head/graphics/php72-gd/files/patch-config.m4
  head/graphics/php72-gd/files/patch-gd.c