Created attachment 160805 [details] failed build log Defining NO_FOO together with do-* targets is risky but then we can't leave do-*-${opt}-on/off by themselves. See bug 202949 for more details.
Created attachment 160806 [details] v0
Created attachment 160807 [details] QA: poudriere bulk -t -j 101i386
Please, don't do that.
Waiting for confirmation from maintainer in case the bug remains closed and no fix lands. I don't use the port, so have no opinion if someone proposes to remove DOXYGEN option as an alternative. (In reply to Mathieu Arnold from comment #3) > Please, don't do that. > Resolution: --- → Rejected > Status: New → Closed Are you trying to sweep the regression under the... portmgr blanket? It still exists even if you don't like my patch (v0 kinda expected). So, propose a better fix, land it with the blanket or, ideally, do not regress ports without documented rationale.
I'm not sure I get what you're saying. You have a port that uses NO_BUILD, so that there is no do-build target that goes and do stuff, which is the *right* way to do it. And you want to replace it with a port that has an empty do-build target, which is not the right way to do it, if you don't want to have a do-build target, you ask for the framework to not create one for you, with NO_BUILD.
If the problem is that the do-build-opt-o{n,ff} target is not run with NO_BUILD, then use post-build-opt-o{n,ff}, but don't create an empty do-build target.
Ok, after a bit of debugging, I get what the problem is. Sorry for not understanding wtf you were doing sooner :-)
Now that I get the problem, I think, a better solution would be: DOXYGEN_VARS_OFF= NO_BUILD=yes and: do-build: do the doxygen stuff Unless it feels even more kludgy.
Created attachment 160839 [details] v1 + commit message (In reply to Mathieu Arnold from comment #8) More confusing but less kludgy. Maybe drop NO_BUILD unconditionally since vendor has "all" target that does nothing.
Looks good, yes, you do know you do not need approval for fixing stuff, right ?
Created attachment 160840 [details] QA with DOXYGEN=on: poudriere bulk -t -j 101i386 poudriere logs aren't verbose enough to have any noise from running ALL_TARGET.
(In reply to Mathieu Arnold from comment #10) Our bugzilla doesn't support review flag, so I'm using the underused maintainer-approval. And phabricator often adds unnecessary indirection. So, I'll take "looks good" as r+.
Also, please, if you can prevent it, do not clutter bugzilla with poudriere logs, I will trust you that it builds, and that you tested. Providing a link to the logs is good enough, if you feel like it's that important to show them.
And we have a code review tool called phabricator, at reviews.freebsd.org :-)
Created attachment 160841 [details] QA with DOXYGEN=off: poudriere bulk -t -j 101i386
(In reply to Jan Beich from comment #15) > Created attachment 160841 [details] > QA with DOXYGEN=off: poudriere bulk -t -j 101i386 Please, STOP adding poudriere logs to bugzilla. A simple "tested with and without DOXYGEN" is fine.
A commit references this bug: Author: jbeich Date: Tue Sep 8 16:13:58 UTC 2015 New revision: 396400 URL: https://svnweb.freebsd.org/changeset/ports/396400 Log: astro/osmium: unbreak build with DOXYGEN=on after r394778 NO_BUILD does more than dummy do-build. It creates BUILD_COOKIE that marks all targets in _BUILD_SEQ as done, including do-build-${opt}-on/off and post-build-${opt}-on/off. Since the targets haven't been run yet do-install-DOXYGEN-on fails due to missing files. Fix by leaking a call to nop "all" target in vendor Makefile. It should be less kludgy than defining our own dummy do-build and less confusing than treating do-build itself as an option helper. In case "all" starts doing something useful the port would automatically take advantage of it. PR: 202951 Reviewed by: mat Approved by: portmgr blanket Changes: head/astro/osmium/Makefile
Hopefully, done.