Bug 237672 - print/py-fonttools: add missing deps
Summary: print/py-fonttools: add missing deps
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: Sunpoet Po-Chuan Hsieh
URL: https://reviews.freebsd.org/D20138
Keywords:
Depends on:
Blocks: 231392
  Show dependency treegraph
 
Reported: 2019-04-30 15:51 UTC by Steve Wills
Modified: 2019-09-25 20:34 UTC (History)
3 users (show)

See Also:


Attachments
patch to add devel/py-fs2 dep to print/py-fonttools (636 bytes, patch)
2019-04-30 15:51 UTC, Steve Wills
no flags Details | Diff
patch to update print/py-fonttools to 3.41.0 and add missing deps (1.61 KB, patch)
2019-04-30 16:05 UTC, Steve Wills
no flags Details | Diff
Updated patch to add missing deps (1.08 KB, patch)
2019-05-02 11:17 UTC, Steve Wills
no flags Details | Diff
Patch to add py-fonttools-lxml-ufo slave port (2.00 KB, patch)
2019-07-01 16:19 UTC, Steve Wills
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Wills freebsd_committer 2019-04-30 15:51:34 UTC
Created attachment 204122 [details]
patch to add devel/py-fs2 dep to print/py-fonttools

Please see attached patch, this fixes print/py-psautohint.
Comment 1 Steve Wills freebsd_committer 2019-04-30 15:52:10 UTC
This port may need other deps added, but it at least needs py-fs2:

https://github.com/fonttools/fonttools/blob/master/setup.py#L34
Comment 2 Steve Wills freebsd_committer 2019-04-30 16:05:55 UTC
Created attachment 204123 [details]
patch to update print/py-fonttools to 3.41.0 and add missing deps

Updated patch attached, please approve or commit.
Comment 3 Steve Wills freebsd_committer 2019-05-02 11:17:14 UTC
Created attachment 204160 [details]
Updated patch to add missing deps

Please approve or commit.
Comment 4 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-05-07 20:08:05 UTC
I think those are options with extra dependencies. And I would like to keep them off by default. I'll prepare a new patch tomorrow.
Comment 5 Steve Wills freebsd_committer 2019-05-07 20:39:45 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #4)
True, it will install and work without them, but without the ability to do certain things. Given the dependencies are small, I think it makes sense to keep them on by default. Why do you want to have them off by default?
Comment 6 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-05-09 19:47:21 UTC
It has more options. The patch for all options: https://people.FreeBSD.org/~sunpoet/patch/print-py-fonttools.txt

Regardless of the default option, it should be done in py-psautohint by adding the dependencies, [ufo] and [lxml] in this case, in the first place. I've told the submitter in the comment of another PR. I think it's the better way to ensure the port works in any case with less changes.
Comment 7 Steve Wills freebsd_committer 2019-05-10 15:20:17 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #6)
But py-psautohint doesn't require ufo, it requires a version of py-fonttools that has ufo support.

Your patch looks fine, except we need either the UFO option on by default or a slave with it on that py-psautohint can depend on. Though given the size of the added dependencies I don't see why a slave with the option would be necessary or better.

I don't understand "I think it's the better way to ensure the port works in any case with less changes." at all. Please explain.
Comment 8 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-05-27 20:38:03 UTC
(In reply to Steve Wills from comment #7)

But other dependent ports do not need UFO.

I suggested to add the dependency to psautohint because:
- They are extra dependencies of fonttools.
- It does not affect fonttools and its dependent ports.
- The package-depends-list of psautohint is equivalent in our patches.

It's the better way which works in any case with less changes.
Comment 9 Steve Wills freebsd_committer 2019-05-27 21:08:43 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #8)

True, but there may be some in the future and then it may be confusing. And it doesn't hurt to have the extra deps. I tend to think packages should provide as many features as possible, unless there's some reason not to, such as large dependency download/install. I don't understand why "less change" is important in this case.
Comment 10 Ting-Wei Lan 2019-06-18 17:28:48 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #8)
Do you think it will be better if each option of fonttools becomes a separate package which installs no file but pulls in dependencies? I don't think we should put indirect dependencies to psautohint or any other ports because it is both misleading and harder to maintain. It also implies that the maintainer of fonttools will update dependencies for all ports depending on fonttools if an update changes dependencies of any feature, which is likely to increase the work of the maintainer.

(In reply to Steve Wills from comment #9)
I agree that we should enable extra features by default if it is not going to cause problems. Personally I don't care the number of dependencies as long as they don't take disk space unnecessarily.
Comment 11 Steve Wills freebsd_committer 2019-07-01 16:19:19 UTC
Created attachment 205471 [details]
Patch to add py-fonttools-lxml-ufo slave port

How about this patch instead, which creates a slave port of py-fonttools with the lxml and ufo options set and sets psautohint to use that.
Comment 12 Ting-Wei Lan 2019-07-01 16:53:09 UTC
(In reply to Steve Wills from comment #11)
Isn't it going to introduce the trouble of conflicting ports?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203457

===>  Installing for py36-fonttools-lxml-ufo-3.43.1
===>  Checking if py36-fonttools-lxml-ufo is already installed
===>   Registering installation for py36-fonttools-lxml-ufo-3.43.1
Installing py36-fonttools-lxml-ufo-3.43.1...
pkg-static: py36-fonttools-lxml-ufo-3.43.1 conflicts with py36-fonttools-3.43.1 (installs files into the same place).  Problematic file: /usr/local/bin/fonttools-3.6
*** Error code 70

Stop.
make[1]: stopped in /usr/ports/print/py-fonttools-lxml-ufo
*** Error code 1

Stop.
make: stopped in /usr/ports/print/py-fonttools-lxml-ufo
Comment 13 Steve Wills freebsd_committer 2019-07-01 17:12:17 UTC
(In reply to Ting-Wei Lan from comment #12)
Yes, but either this or adding the extra deps to py-psautohint seems to be the only way. Adding them to py-fonttools doesn't seem to be an option.
Comment 14 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-07-02 18:54:02 UTC
(In reply to Ting-Wei Lan from comment #10)

I could give you an example, requests[socks]. I would not ask py-requests maintainer to add it unconditionally which affects all dependent ports. I just add it to the port which requires requests[socks]. And I see other did either.

IMHO, a comment would be enough for such case. I could take this port if it's too hard for you to maintain it and the dependencies.

(In reply to Steve Wills from comment #11)

I think it works. Adding a slave port would avoid bring unnecessary dependencies to dependent ports of py-fonttools.
Comment 15 commit-hook freebsd_committer 2019-07-03 18:53:26 UTC
A commit references this bug:

Author: sunpoet
Date: Wed Jul  3 18:52:20 UTC 2019
New revision: 505780
URL: https://svnweb.freebsd.org/changeset/ports/505780

Log:
  Add py-fonttools-lxml-ufo (slave port of py-fonttools)

  PR:		237672
  Submitted by:	swills

Changes:
  head/print/Makefile
  head/print/py-fonttools/Makefile
  head/print/py-fonttools-lxml-ufo/
  head/print/py-fonttools-lxml-ufo/Makefile
Comment 16 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-07-03 18:53:48 UTC
Committed. Thanks!
Comment 17 commit-hook freebsd_committer 2019-09-25 20:34:12 UTC
A commit references this bug:

Author: sunpoet
Date: Wed Sep 25 20:33:24 UTC 2019
New revision: 512811
URL: https://svnweb.freebsd.org/changeset/ports/512811

Log:
  Remove print/py-fonttools-lxml-ufo

  PR:		237672

Changes:
  head/MOVED
  head/print/Makefile
  head/print/py-fonttools-lxml-ufo/