Bug 220293 - graphics/graphviz: fix building php bindings
Summary: graphics/graphviz: fix building php bindings
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Dirk Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-26 19:22 UTC by Dmitry Marakasov
Modified: 2017-07-07 13:00 UTC (History)
0 users

See Also:
dinoex: maintainer-feedback+


Attachments
Patch (4.46 KB, patch)
2017-06-26 19:22 UTC, Dmitry Marakasov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Marakasov freebsd_committer 2017-06-26 19:22:16 UTC
Created attachment 183818 [details]
Patch

graphviz with enabled PHP bindings is marked as:

IGNORE_WITH_PHP=       56 71

this was blindly and incorrectly derived from WANT_PHP_VER=70 before.

In fact, it works fine with php 7.1, so the corresponding ignore is not needed.
It doesn't build with PHP 5 however, but the cause is its incorrect use of swig: configure tries if it supports -php7 argument and uses it if it does. This, however, makes swig generate php5 incompatible code. So we have to override swig option here and use either -php5 or -php7 depending on actual php version.

While here, switch some conditions to options helpers and silence commands which should be silent.
Comment 1 Dirk Meyer freebsd_committer 2017-06-27 07:51:35 UTC
At the time of the graphviz release,
the build failed with php71 from ports.

so the setting is correct.

I object the proposed patch.

Why should commands that modify files be silent?.

Why should we use bsd.port.pre.mk ?
Comment 2 Dmitry Marakasov freebsd_committer 2017-06-27 12:12:09 UTC
(In reply to Dirk Meyer from comment #1)

> At the time of the graphviz release,
> the build failed with php71 from ports.
> 
> so the setting is correct.

No it is not, it builds fine with both php56 and php71:

https://people.freebsd.org/~amdmi3/graphviz-php56.log
https://people.freebsd.org/~amdmi3/graphviz-php71.log

> Why should commands that modify files be silent?.

It is common practice to silence post-patch, and it's also recommented by PHB:

% find /usr/ports -name Makefile -exec cat {} \;|grep '[^@]${REINPLACE_CMD}' | wc -l                                                                                                                                                                                        
    2874                                                                                                                                                                                                                                                             
% find /usr/ports -name Makefile -exec cat {} \;|grep '[@]${REINPLACE_CMD}' | wc -l                                                                                                                                                                                         
    8271                                                                                                                                                                                                                                                             

https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/slow-patch.html

> Why should we use bsd.port.pre.mk ?

Because PHP_VER is only available after it.
Comment 3 commit-hook freebsd_committer 2017-06-28 12:08:03 UTC
A commit references this bug:

Author: dinoex
Date: Wed Jun 28 12:07:47 UTC 2017
New revision: 444569
URL: https://svnweb.freebsd.org/changeset/ports/444569

Log:
  - use more OPTION_VARS

  - switch some conditions to options helpers
  - allow build with php7.1
  PR:		220293
  Submitted by:	Dmitry Marakasov

Changes:
  head/graphics/graphviz/Makefile
Comment 4 Dirk Meyer freebsd_committer 2017-06-28 12:17:21 UTC
Some parts of the patch committed.

Support for PHP5 was dropped over 6 Month ago,
when PHP5 "Active Support" had ended.

http://php.net/supported-versions.php

no complaints where received.
I see no value in resurrecting the support for the old version.
Comment 5 Dmitry Marakasov freebsd_committer 2017-06-28 12:41:59 UTC
You can not ignore PHP 5 support because it's still the default version in FreeBSD ports. Also consider this a complaint, as I'm just relaying a report of build failure for graphviz with PHP bindings enabled.
Comment 6 commit-hook freebsd_committer 2017-07-06 12:02:49 UTC
A commit references this bug:

Author: dinoex
Date: Thu Jul  6 12:02:23 UTC 2017
New revision: 445137
URL: https://svnweb.freebsd.org/changeset/ports/445137

Log:
  - support build with php5.6
  PR:		220293
  Submitted by:	Dmitry Marakasov

Changes:
  head/graphics/graphviz/Makefile
Comment 7 Dmitry Marakasov freebsd_committer 2017-07-07 13:00:26 UTC
Thank you!