Page MenuHomeFreeBSD

graphics/partio: update to 1.5.5
AbandonedPublic

Authored by fernape on Aug 10 2018, 7:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 12, 1:26 AM
Unknown Object (File)
Thu, Apr 11, 10:25 PM
Unknown Object (File)
Thu, Apr 11, 9:40 PM
Unknown Object (File)
Thu, Apr 11, 1:32 PM
Unknown Object (File)
Wed, Apr 10, 2:31 AM
Unknown Object (File)
Sun, Mar 31, 1:41 PM
Unknown Object (File)
Mar 9 2024, 4:46 PM
Unknown Object (File)
Mar 3 2024, 8:04 AM
Subscribers

Details

Reviewers
tcberner
tz
Summary

Update port to 1.5.5 (reported by portscout).

  • Fix BINARY_ALIAS (only needed if python is selected)

I don't know how to deal with the shebangfix problem. I fixed it by using python2.7 and then using python_CMD pointing to that interpreter but I don't know if this is the best thing to do. How to know which interpreter is installed?

Test Plan
  • portlint -AC OK
  • poudriere builds for {10.4,11.1}{amd64,i386}, 11.2amd64, 12i386 OK

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 19263
Build 18876: arc lint + arc unit

Event Timeline

graphics/partio/Makefile
35

^the list of files is relative to ${WRKSRC}, so you can make the lines shorter

graphics/partio/Makefile
23–24

Per policy, DOCS cannot bring doxygen in, you should add a DOXYGEN option with a DOXYGEN_IMPLIES=DOCS.

32

This is already set by USES=shebangfix, not needed.

  • Remove ${WRKSRC} from PYTHON_BINARY_ALIAS
  • Remove python_CMD
  • Update to 1.5.4
fernape retitled this revision from graphics/partio: update to 1.5.3 to graphics/partio: update to 1.5.4.Aug 11 2018, 4:18 PM

catch up with upstream: update to 1.5.5

fernape retitled this revision from graphics/partio: update to 1.5.4 to graphics/partio: update to 1.5.5.Aug 16 2018, 4:45 PM
fernape edited the summary of this revision. (Show Details)

You still need to address the DOCS changes requested by mat.

You still need to address the DOCS changes requested by mat.

I completely missed that comment. Thanks!

Add DOXYGEN option instead of DOCS.

This avoids breaking policy of not pulling doxygen as a dependency
directly from DOCS.

graphics/partio/Makefile
21

You are missing the DOCS option here.

graphics/partio/Makefile
21

Hi mat!

I did it mimicking audio/chromaprint.

The doc target in partio's src/doc CMakeLists.txt only runs doxygen, and nothing else. What is the reason to have both DOCS and DOXYGEN as options? If DOCS is always on by default, and the user doesn't select DOXYGEN, which is the only one pulling devel/doxygen, it won't work.

Sorry if I'm missing something here :-)

graphics/partio/Makefile
21

You are using PORTDOCS. PORTDOCS is only used when there is a DOCS option is BOTH defined, AND enabled.
Right now, setting the DOXYGEN option does only one thing, bring in all of doxygen dependencies, build the doc, and trash them after the package is created.

Fixing DOCS option handling.

  • Added a post-install-DOCS-on target that copies tutorial and manual
  • Added a post-patch-{DOCS,DOXYGEN}-off target that disables the doc subdirectory addition in the CMakeLists.txt

I don't like duplicating the logic of post-patch-{DOCS,DOXYGEN}-off. Is it preferred to add a normal .if?

Testing:

make package and then pkg info -l -F partio-1.5.5.txz  | less

DOCS: off
DOXYGEN: off

No tutorial, manual or html present in the package

DOCS: on
DOXYGEN: off

tutorial and manual in the package. No html files from doxygen generated

DOCS: off
DOXYGEN: on

tutorial and manual in the package. html files from doxygen present in the package

DOCS: on
DOXYGEN: on

tutorial and manual in the package. html files from doxygen present in the package

I don't like duplicating the logic of post-patch-{DOCS,DOXYGEN}-off. Is it preferred to add a normal .if?

You don't need to have a post-patch-DOXYGEN-off, only the post-patch-DOCS-off one is required. Because DOXYGEN_IMPLIES=DOCS.

DOCS: off
DOXYGEN: on

You cannot have DOCS disabled if DOXYGEN is enabled. Because DOXYGEN_IMPLIES=DOCS.

Remove post-patch-DOXYGEN-off.

In D16662#361092, @mat wrote:

I don't like duplicating the logic of post-patch-{DOCS,DOXYGEN}-off. Is it preferred to add a normal .if?

You don't need to have a post-patch-DOXYGEN-off, only the post-patch-DOCS-off one is required. Because DOXYGEN_IMPLIES=DOCS.

DOCS: off
DOXYGEN: on

You cannot have DOCS disabled if DOXYGEN is enabled. Because DOXYGEN_IMPLIES=DOCS.

Sorry, I mean during configure I can select DOXYGEN and not DOCS, but yes, the built package is DOCS:on and DOXYGEN:on

In D16662#361092, @mat wrote:

DOCS: off
DOXYGEN: on

You cannot have DOCS disabled if DOXYGEN is enabled. Because DOXYGEN_IMPLIES=DOCS.

Sorry, I mean during configure I can select DOXYGEN and not DOCS, but yes, the built package is DOCS:on and DOXYGEN:on

Yes, you can, but it does not gets disabled because DOXYGEN is enabled.

graphics/partio/Makefile
55–56

This could probably be remplaced with:

DOXYGEN_ALL_TARGET= all doc

Simplify doxygen files generation.

Looks good to me, as long as @mat is happy with the option handling now :)

fernape added a subscriber: danfe.

Updated by @danfe in r479514

Whoops, sorry, I didn't know you've been working on an update. DOXYGEN option is still missing though.

Whoops, sorry, I didn't know you've been working on an update. DOXYGEN option is still missing though.

No worries! I might create a new review to add that at some point :-)