Page MenuHomeFreeBSD

audio/pithos: update from 1.0.0 to 1.4.1
ClosedPublic

Authored by jhixson on Jan 18 2019, 7:46 PM.

Details

Summary

audio/pithos: update from 1.0.0 to 1.4.1

Test Plan

Tested with poudriere for 11.2 and 12.0

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mat requested changes to this revision.Jan 23 2019, 8:40 AM

Remove both pkg-install and pkg-deinstall and use:

GLIB_SCHEMAS=   io.github.Pithos.gschema.xml

(and remove the entry from the pkg-plist file)

Also, all occurrences of 36 in the plist are wrong, because we have Python 3.4, 3.5, 3.6 and 3.7 in the tree.

audio/pithos/Makefile
13 ↗(On Diff #53012)

Why +=?

19 ↗(On Diff #53012)

Why +=?

35–42 ↗(On Diff #53012)

Use ${REINPLACE_CMD} and not ${SED}. Also, are you aware that you can pass multiple arguments to sed so that it processes many files at once?

This revision now requires changes to proceed.Jan 23 2019, 8:40 AM

Fixes / suggestions thanks to mat

audio/pithos/Makefile
13 ↗(On Diff #53012)

I apologize, this was just copied over from the prior version. I have fixed this.

19 ↗(On Diff #53012)

Same as above.

35–42 ↗(On Diff #53012)

Thank you. I was not aware of ${REINPLACE_CMD}. I have rearranged the sed to do multiple files at once.

35–42 ↗(On Diff #53012)

I was also not aware of the GLIB_SCHEMAS variable. Thanks for the tip. I have fixed everything you've mentioned. I am having an issue now since the pyc files are in the stage directory, but not in the pkg-plist.

miwi added inline comments.
audio/pithos/Makefile
42 ↗(On Diff #53147)

Would it not easier to use SHEBANG?

audio/pithos/Makefile
19 ↗(On Diff #53147)

You set already USES= gnome, I think USE_GNOME=glib20 will work here too.

audio/pithos/Makefile
19 ↗(On Diff #53147)

Correct. I did have that at one point. According to the handbook, it appears that using USES is the preferred method.

42 ↗(On Diff #53147)

Please tell me more about this mythical creature. I am not familiar with it.

Was the plist generated with make makeplist?

audio/pithos/Makefile
48–50 ↗(On Diff #53147)

This could probably be replaced by setting:

BINARY_ALIAS=  sphinx-build-3=sphinx-build-${PYTHON_VER}
52–55 ↗(On Diff #53147)

STAGEDIR can only be accessed during the install phase, so this should be either post-install or pre-install.

Regarding the pkg-plist, yes, I use make makeplist to generate it.

audio/pithos/Makefile
42 ↗(On Diff #53147)

Thanks. Miwi already pointed me to this. Much appreciated .

48–50 ↗(On Diff #53147)

I will give this a try.

52–55 ↗(On Diff #53147)

Will do. I appreciate the feedback. You guys have taught me a lot just in this single review ;-)

audio/pithos/Makefile
37–47 ↗(On Diff #53147)

sed can do more than one thing.

${REINPLACE_CMD} -i '' -e \
	​                's|@@PYTHONPREFIX_SITELIBDIR@@|${PYTHONPREFIX_SITELIBDIR}|g; \
	​                 s|@@PYTHON_PATH@@|${PYTHONBASE}/bin/python${PYTHON_VER}|g' \
	​                ${WRKSRC}/meson.build \
	​                ${WRKSRC}/bin/pithos.in \
	​                ${WRKSRC}/meson_post_install.py \
	​                ${WRKSRC}/docs/conf.py

Port fixes as suggested by araujo, miwi and mat

miwi requested changes to this revision.Jan 25 2019, 2:19 AM

other then that looks good to me

audio/pithos/Makefile
15 ↗(On Diff #53169)

USE_GNOME= glib20

19 ↗(On Diff #53169)

can be removed in favor of USE_GNOME= glib20

This revision now requires changes to proceed.Jan 25 2019, 2:19 AM
audio/pithos/Makefile
14 ↗(On Diff #53169)

I don't think I ever saw a port depend on this directly, usually it is done with USES=desktop-file-utils.

17 ↗(On Diff #53169)

The port does not seem to install any documentation, what is sphinx used for?

  • Remove glib20 from dependencies in favor of USE_GNOME
  • Remove patch files in favor of sed
jhixson added inline comments.
audio/pithos/Makefile
14 ↗(On Diff #53169)

This port uses it to verify desktop files.

17 ↗(On Diff #53169)

Good question, I will find the answer!

  • Remove sphinx here, no need for it
  • sed the extra 'pithos' out existence
jhixson added inline comments.
audio/pithos/Makefile
17 ↗(On Diff #53169)

Sphinx is in fact not used. I'm unsure why it is in the build, other than perhaps future use. I have removed the need for it.

Thanks, this looks good now.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 28 2019, 6:12 PM
This revision was automatically updated to reflect the committed changes.