Page MenuHomeFreeBSD
Differential D16662

graphics/partio: update to 1.5.5
AbandonedPublic
Actions

Authored by fernape on Aug 10 2018, 7:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 11, 6:50 AM
Unknown Object (File)
Wed, Sep 9, 10:32 AM
Unknown Object (File)
Wed, Sep 9, 12:24 AM
Unknown Object (File)
Tue, Sep 8, 1:08 PM
Unknown Object (File)
Tue, Sep 8, 10:01 AM
Unknown Object (File)
Tue, Sep 8, 6:25 AM
Unknown Object (File)
Tue, Sep 8, 5:07 AM
Unknown Object (File)
Mon, Sep 7, 10:26 AM
View All Files
Subscribers
danfe
mat

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 18706
Build 18387: arc lint + arc unit

Event Timeline

fernape created this revision.Aug 10 2018, 7:10 PM
Herald added a subscriber: mat. · View Herald TranscriptAug 10 2018, 7:10 PM
Harbormaster completed remote builds in B18706: Diff 46524.Aug 10 2018, 7:10 PM
tcberner added inline comments.Aug 11 2018, 7:00 AM
graphics/partio/Makefile
32

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

mat added inline comments.Aug 11 2018, 8:42 AM
graphics/partio/Makefile
24

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

28

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

fernape updated this revision to Diff 46568.Aug 11 2018, 4:17 PM
  • Remove ${WRKSRC} from PYTHON_BINARY_ALIAS
  • Remove python_CMD
  • Update to 1.5.4
Harbormaster completed remote builds in B18727: Diff 46568.Aug 11 2018, 4:17 PM
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
fernape updated this revision to Diff 46786.Aug 16 2018, 4:45 PM

catch up with upstream: update to 1.5.5

Harbormaster completed remote builds in B18860: Diff 46786.Aug 16 2018, 4:45 PM
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)
tcberner added a comment.Aug 23 2018, 6:10 PM

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

fernape marked 2 inline comments as done.Aug 23 2018, 8:39 PM
In D16662#359378, @tcberner wrote:

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

I completely missed that comment. Thanks!

fernape updated this revision to Diff 47245.Aug 24 2018, 5:16 PM

Add DOXYGEN option instead of DOCS.

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

Harbormaster completed remote builds in B19140: Diff 47245.Aug 24 2018, 5:16 PM
fernape marked an inline comment as done.Aug 24 2018, 5:16 PM
mat added inline comments.Aug 25 2018, 6:19 AM
graphics/partio/Makefile
21

You are missing the DOCS option here.

fernape added inline comments.Aug 25 2018, 4:18 PM
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 :-)

mat added inline comments.Aug 27 2018, 9:58 AM
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.

fernape updated this revision to Diff 47384.Aug 28 2018, 4:26 PM

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

Harbormaster completed remote builds in B19238: Diff 47384.Aug 28 2018, 4:27 PM
mat added a comment.Aug 28 2018, 7:43 PM
In D16662#361023, @fernape 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.

fernape updated this revision to Diff 47402.Aug 28 2018, 8:45 PM

Remove post-patch-DOXYGEN-off.

Harbormaster completed remote builds in B19248: Diff 47402.Aug 28 2018, 8:45 PM
fernape added a comment.Aug 28 2018, 8:46 PM
In D16662#361092, @mat wrote:
In D16662#361023, @fernape 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

mat added a comment.Aug 29 2018, 7:44 AM
In D16662#361128, @fernape wrote:
In D16662#361092, @mat wrote:
In D16662#361023, @fernape 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.

mat added inline comments.Aug 29 2018, 7:49 AM
graphics/partio/Makefile
53–54

This could probably be remplaced with:

DOXYGEN_ALL_TARGET= all doc
fernape updated this revision to Diff 47443.Aug 29 2018, 3:40 PM

Simplify doxygen files generation.

Harbormaster completed remote builds in B19263: Diff 47443.Aug 29 2018, 3:40 PM
tcberner added a comment.Sep 6 2018, 8:03 PM

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

fernape abandoned this revision.Sep 11 2018, 3:17 PM
fernape added a subscriber: danfe.

Updated by @danfe in r479514

danfe added a comment.Sep 11 2018, 6:07 PM

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

fernape added a comment.Sep 11 2018, 6:28 PM
In D16662#365037, @danfe wrote:

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 :-)

Revision Contents
Changeset List

PathSize
graphics/
partio/
19 lines
5 lines
11 lines

Diff 46524

View Options

graphics/partio/Makefile

Loading...
View Options

graphics/partio/distinfo

Loading...
View Options

graphics/partio/pkg-plist

Loading...