Bug 192732 - [MAINTAINER] devel/doxygen: use options for graphviz and LaTeX, use new OPTIONS helpers
Summary: [MAINTAINER] devel/doxygen: use options for graphviz and LaTeX, use new OPTIO...
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: freebsd-ports-bugs (Nobody)
URL:
Keywords:
: 192611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-08-17 05:51 UTC by Naram Qashat
Modified: 2014-08-22 17:12 UTC (History)
4 users (show)

See Also:


Attachments
doxygen-1.8.7_1,1.patch (6.03 KB, patch)
2014-08-17 05:51 UTC, Naram Qashat
no flags Details | Diff
doxygen-1.8.7_1,1.patch (6.55 KB, patch)
2014-08-18 04:53 UTC, Naram Qashat
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Naram Qashat 2014-08-17 05:51:30 UTC
Created attachment 145903 [details]
doxygen-1.8.7_1,1.patch

- Use options for graphviz and LaTeX (these are also defaulted, so hopefully the rest of the ports tree that uses doxygen for their documentation aren't adversely affected).
- Use new OPTIONS helpers.
- PDFDOCS relies on both the LATEX and HTMLDOCS options to be enabled.
- Remove a couple patches that I believe have no effect on the PDFDOCS build because it should've been setting HAVE_LATEX in MAKE_ARGS instead of HAVE_PDFDOCS (which was done by a previous commit that I was unaware of back when it happened, but never should've happened to begin with).

NOTE: I have made the port rely on the a full LaTeX system, but this may be excessive. I have not bothered to look into just how much of the LaTeX system is needed for doxygen to function properly when generating PDF docs.

NOTE 2: This will most likely need an exp-run done because of the number of ports that utilize doxygen to build their own documentation.
Comment 1 John Marino freebsd_committer freebsd_triage 2014-08-17 07:50:31 UTC
I think lines like this "cd latex ; $(MAKE)" break multijob capability (making port need MAKE_JOBS_UNSAFE=yes defined)

It should be "${MAKE} -C latex" instead, right?
Comment 2 John Marino freebsd_committer freebsd_triage 2014-08-17 11:24:49 UTC
*** Bug 192611 has been marked as a duplicate of this bug. ***
Comment 3 Naram Qashat 2014-08-17 15:41:08 UTC
I'm not familiar enough with the intricacies of make to know if that is the case or not. If someone more familiar with make knows better, then I am fine with the change.
Comment 4 John Marino freebsd_committer freebsd_triage 2014-08-17 17:56:41 UTC
Then assume I'm correct and use "make -V folder" over "cd folder ; make"
Comment 5 John Marino freebsd_committer freebsd_triage 2014-08-17 18:37:35 UTC
(In reply to John Marino from comment #4)
> Then assume I'm correct and use "make -V folder" over "cd folder ; make"

typo:
Then assume I'm correct and use "make -C folder" over "cd folder ; make"
Comment 6 Naram Qashat 2014-08-17 20:32:23 UTC
Oh, actually, I remember why MAKE_JOBS_UNSAFE was added, it had nothing to do with the use of "cd folder; make", it was because of the port-build target needing to run make in the examples folder. I've been told before that post-build causes issues with parallel building, but I was never told if there is a solution to this.
Comment 7 John Marino freebsd_committer freebsd_triage 2014-08-17 20:48:19 UTC
I didn't even know MAKE_JOBS_UNSAFE was set.  I was saying if you had cd then make, then it wouldn't be jobs safe.  Looks like I was right?  It's probably in multiple places in your makefile.
Comment 8 Naram Qashat 2014-08-18 04:53:15 UTC
Created attachment 145952 [details]
doxygen-1.8.7_1,1.patch

OK, I am replacing the patch. I am only modifying the port Makefile and not the ones included with doxygen itself. The usage of ${MAKE_CMD} in post-build now appears to work correctly with MAKE_JOBS_UNSAFE not being defined. And even with the "cd folder ; ${MAKE}" usages inside of doxygen's Makefiles, it appears to still handle parallel building correctly, at least I had no errors because of such inside of poudriere.
Comment 9 John Marino freebsd_committer freebsd_triage 2014-08-18 06:23:14 UTC
(In reply to Naram Qashat from comment #8)
> Created attachment 145952 [details]
> doxygen-1.8.7_1,1.patch
> 
> OK, I am replacing the patch. I am only modifying the port Makefile and not
> the ones included with doxygen itself.

I don't understand the logic.  If doxygen makefile is doing it wrong, it doesn't help to only fix some of them.  It's either all of them or don't bother.

>  The usage of ${MAKE_CMD} in
> post-build now appears to work correctly with MAKE_JOBS_UNSAFE not being
> defined. 

Did you test this on -j8 or -j20 machine?  Redports?


> And even with the "cd folder ; ${MAKE}" usages inside of doxygen's
> Makefiles, it appears to still handle parallel building correctly, at least
> I had no errors because of such inside of poudriere.

I'll get /danfe to comment, he's the expert in parallel building
Comment 10 John Marino freebsd_committer freebsd_triage 2014-08-18 11:42:39 UTC
danfe, your jobs-unsafe expertise is needed.
Comment 11 Alexey Dokuchaev freebsd_committer freebsd_triage 2014-08-19 02:00:41 UTC
Noted; I will review the patch and test it under -jX environment.
Comment 12 John Marino freebsd_committer freebsd_triage 2014-08-20 22:23:34 UTC
hi danfe, any news?  This patch basically extends what was done before so I don't want to hold it up for too long, especially since it looks like an important update.
Comment 13 Alexey Dokuchaev freebsd_committer freebsd_triage 2014-08-21 15:11:29 UTC
I will start -jX builds shortly.  Meanwhile, two comments on the patch in general:

1) USES is not sorted properly (should be: bison gmake iconv perl5 python:2)
2) This part:

-.if ${PORT_OPTIONS:MHTMLDOCS}
-ALL_TARGET+=	docs
-BUILD_DEPENDS+=	dot:${PORTSDIR}/graphics/graphviz
+.if ${PORT_OPTIONS:MPDFDOCS} && \
+	(empty(PORT_OPTIONS:MHTMLDOCS) || empty(PORT_OPTIONS:MLATEX))
+IGNORE=		the PDFDOCS option requires the HTMLDOCS and LATEX options to be set
+.endif

Would it be possible to treat HTMLDOCS and LATEX options as set automatically for PDFDOCS?  This should simplify the logic and avoid nagging the user.
Comment 14 Alexey Dokuchaev freebsd_committer freebsd_triage 2014-08-21 16:56:26 UTC
(In reply to Naram Qashat from comment #0)
> NOTE: I have made the port rely on the a full LaTeX system, but this may be
> excessive. I have not bothered to look into just how much of the LaTeX
> system is needed for doxygen to function properly when generating PDF docs.

Oh boy, this whole LaTeX stack is huge!  I certainly vote to make is disabled by default.

That said, I also didn't test how well Doxygen works without it (but then again, I try not to poison my systems with Doxygen itself).  It can be a good idea to see how RedHat/Debian/Gentoo package it, as a reference.
Comment 15 Naram Qashat 2014-08-21 17:46:07 UTC
Yeah, I know that the whole LaTeX stack is huge. But I'd be against making it default to off, if only because I don't know how much that would break other ports in the tree. I might be able to look at what other repositories use for this packages later today, or over the weekend here. But the LaTeX option would definitely need to stay defaulted to on unless it turns out that nothing in the ports tree requires doxygen to build PDF documentation. This is why I mentioned that an exp-run would likely be needed.
Comment 16 Naram Qashat 2014-08-21 17:49:15 UTC
(In reply to Alexey Dokuchaev from comment #13)
> 2) This part:
> 
> -.if ${PORT_OPTIONS:MHTMLDOCS}
> -ALL_TARGET+=	docs
> -BUILD_DEPENDS+=	dot:${PORTSDIR}/graphics/graphviz
> +.if ${PORT_OPTIONS:MPDFDOCS} && \
> +	(empty(PORT_OPTIONS:MHTMLDOCS) || empty(PORT_OPTIONS:MLATEX))
> +IGNORE=		the PDFDOCS option requires the HTMLDOCS and LATEX options to be
> set
> +.endif
> 
> Would it be possible to treat HTMLDOCS and LATEX options as set
> automatically for PDFDOCS?  This should simplify the logic and avoid nagging
> the user.

I almost missed this comment. In any case, I have no issues with wanting to simplify the logic. I suppose that does mean that the description for PDFDOCS needs to mention it forces inclusion of HTMLDOCS and LATEX, though.
Comment 17 Adam Weinberger freebsd_committer freebsd_triage 2014-08-21 18:56:55 UTC
The best way to run make is with ${DO_MAKE_BUILD}. It contains the entire make command, with MAKE_ENV, MAKE_ARGS, etc. It just needs -C.

${DO_MAKE_BUILD} -C ${WRKSRC}/latex
Comment 18 commit-hook freebsd_committer freebsd_triage 2014-08-22 17:10:51 UTC
A commit references this bug:

Author: danfe
Date: Fri Aug 22 17:10:21 UTC 2014
New revision: 365679
URL: http://svnweb.freebsd.org/changeset/ports/365679

Log:
  - Optionize for Graphviz and LaTeX (enabled by default, so hopefully the
    rest of the ports tree that uses Doxygen for their documentation aren't
    adversely affected)
  - Use new OPTIONS framework helpers
  - PDFDOCS relies on both the LATEX and HTMLDOCS options to be enabled
  - Remove a couple patches that believed to have no effect on PDFDOCS build
    because it should've been setting HAVE_LATEX in MAKE_ARGS instead of
    HAVE_PDFDOCS (which was done by a previous commit that submitter was
    unaware of back when it happened, but never should've happened to begin
    with)
  - While here: sort USES, remove MAKE_JOBS_UNSAFE [*], and cleanup Makefile,
    use correct spelling of PostScript and PDF in port description text

  [*] Except cleaning targets, use more robust make(1) syntax of -C, albeit
      I did not find evidence that cd ... ; $(MAKE) syntax was causing build
      failures with -jX.  Still, -C is better and safer in general.

  PR:		192732
  Submitted by:	maintainer
  Reviewed by:	marino

Changes:
  head/devel/doxygen/Makefile
  head/devel/doxygen/files/patch-Makefile.in
  head/devel/doxygen/files/patch-latex
  head/devel/doxygen/files/patch-refman
  head/devel/doxygen/pkg-descr
Comment 19 Alexey Dokuchaev freebsd_committer freebsd_triage 2014-08-22 17:12:08 UTC
Committed as r365679 with minor modifications.

http://svnweb.freebsd.org/ports?view=revision&revision=365679