Bug 205883
| Summary: | databases/rrdtool: DEJAVU option logic broken | ||
|---|---|---|---|
| Product: | Ports & Packages | Reporter: | John Marino <marino> |
| Component: | Individual Port(s) | Assignee: | Niclas Zeising <zeising> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | Keywords: | needs-patch, needs-qa |
| Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(zeising) koobs: merge-quarterly? |
| Version: | Latest | ||
| Hardware: | Any | ||
| OS: | Any | ||
okay, this PR timed out today. I'm probably going to remove DEJAVU option and make dejavu an unconditional RUN_DEPENDency. Give me time to work on this, please. Or at least give me a chance to review the patch, I'm working on my backlog as quick as possible, but it's only been two weeks. okay, I'll give you more time, but I have to object to "but it's only been two weeks." The timeout period is two weeks. Usually commenting on the PR will give a lot more time so the submitter knows the maintainer is working on it, but you can't complain that two weeks passes when you don't comment on the PR. That's exactly why the period was documented! Plus I seem to recall that you said you would be MIA indefinitely and I didn't know you actually came back. So, I think I know what's going on here. It is possible to build cairo and pango without X11 support, meaning no fonts are installed. Therefore, it might make sense to depend on the fonts explicitly if this is the case. It might also be the case that the fonts are pulled in by cario and pango, but we want to depend on them explicitly, hence the check if the port is installed. I have to think a bit about the best course of action. It might be to remove the option altogether, or remove the check if the port is installed, and just keep the option. I'm also contemplating reworking the option to depend on xorg-fonts instead. If you have any suggestions, please let me know. I'm sorry about the delay, I've been working hard on ports-mgmt/synth towards the first release.
Okay, so I'm hearing that there is a way where x11-fonts/dejavu is not guaranteed to be installed when the user selects no default options.
In that case, I'd recommend that DEJAVU be set to "on" by default and just use the standard options framework, e.g. DEJAVU_RUN_DEPENDS= dejavu>0:${PORTSDIR}/x11-fonts/dejavu
that means:
1) it's correct for default options
2) The person that already customized pango/cairo will have to also customize rrdtool to not use dejavu. I don't think that's unreasonable. #1 would cover 99%+ of the use cases I think, so it should be biased to that direction.
A commit references this bug: Author: zeising Date: Wed Jan 27 11:48:10 UTC 2016 New revision: 407355 URL: https://svnweb.freebsd.org/changeset/ports/407355 Log: Update to 1.5.5 Rework the DEJAVU option to actually work. [1] Before this commit rrdtool depends on x11-fonts/dejavu if that port is already installed, but not otherwise, and this sort of implicit dependencies are bad. Make the DEJAVU option default to on. If cairo and pango are built without X11 support, please consider disabling this option to avoid pulling in large parts of X11 Remove the JSON option. This is a third party patch that doesn't apply any more. Feel free to port it to this version of RRDTool. Discussed with: marino [1] PR: 205883 [1] Submitted by: marino [1] Changes: head/databases/rrdtool/Makefile head/databases/rrdtool/distinfo head/databases/rrdtool/files/patch-bindings-Makefile.in head/databases/rrdtool/files/patch-bindings_perl-shared_Makefile.PL head/databases/rrdtool/files/patch-configure head/databases/rrdtool/files/patch-doc-Makefile.in head/databases/rrdtool/files/patch-examples-Makefile.in head/databases/rrdtool/files/patch-examples_rrdcached_Makefile.in head/databases/rrdtool/files/patch-src-rrd_open.c head/databases/rrdtool/files/patch-src__rrd_graph.c head/databases/rrdtool/files/patch-src__rrdupdate.c head/databases/rrdtool/files/thirdparty-json.diff head/databases/rrdtool/pkg-plist Committed a fix as noted. Please let me know if it's not enough, and thanks for giving me some extra time to fix this. Thanks! If I wasn't so busy these last two weeks, I would have provided a patch to make it easier. It was on my to-do list. This fix should be perfect from my POV, but I'll let you know if it isn't. |
rrdtool makefile has these lines in it: .if exists(${LOCALBASE}/share/fonts/dejavu) || ${PORT_OPTIONS:MDEJAVU} RUN_DEPENDS+= dejavu>0:${PORTSDIR}/x11-fonts/dejavu .endif Besides being a questionable technique, it doesn't work as intended. The dejavu dependency will be packaged 100% of the time regardless of value of the option or if the rrdtool is built in a clean jail (where share/fonts/dejavu doesn't exist in theory) Why? because xorg-fonts-truetype (from pango) pulls it in, and by the time it's packaged, share/fonts/dejavu exists. `-- Installing xorg-fonts-truetype-7.7_1... | `-- Installing font-misc-meltho-1.0.3_3... | | `-- Installing mkfontdir-1.0.7... | | `-- Installing mkfontscale-1.1.2... | | | `-- Installing libfontenc-1.1.3... | | | `-- Extracting libfontenc-1.1.3: ...... done | | `-- Extracting mkfontscale-1.1.2: .. done | | `-- Extracting mkfontdir-1.0.7: .. done | `-- Extracting font-misc-meltho-1.0.3_3: .......... done | `-- Installing font-bh-ttf-1.0.3_3... | `-- Extracting font-bh-ttf-1.0.3_3: .......... done | `-- Installing font-misc-ethiopic-1.0.3_3... | `-- Extracting font-misc-ethiopic-1.0.3_3: .. done | `-- Installing dejavu-2.35... | `-- Extracting dejavu-2.35: .......... done I think the best approach is remove the override of the dejavu option. Either that, or force it as an unconditional RUN depends. The current method is broken.