Page MenuHomeFreeBSD

[NEW PORT] audio/forked-daapd
ClosedPublic

Authored by woodsb02 on May 11 2016, 6:28 PM.

Details

Summary

Add new port audio/forked-daapd, a DAAP (iTunes), MPD (Music
Player Daemon) and RSP (Roku) media server.

PR: 199743
Submitted by: takumiiinn@gmail.com, with extra work by pi
Approved by: adamw (mentor)

Test Plan

poudriere testport audio/forked-daapd

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

woodsb02 retitled this revision from to [NEW PORT] audio/forked-daapd.
woodsb02 updated this object.
woodsb02 edited the test plan for this revision. (Show Details)
woodsb02 added reviewers: adamw, mat, koobs.
audio/forked-daapd/Makefile
17 ↗(On Diff #16217)

Why all these += instead of =?

28 ↗(On Diff #16217)

USES=sqlite?

57 ↗(On Diff #16217)

Why .pre/.post.mk?

62 ↗(On Diff #16217)

Indented comments get passed to sh(1). Left-aligned comments are actual make(1) comments.

68 ↗(On Diff #16217)

What do these comments add?

audio/forked-daapd/Makefile
17 ↗(On Diff #16217)

Why USES=compiler?

As per comments from adamw:

  • Replace LIB_DEPENDS=libsqlite3.so:databases/sqlite3 with USES=sqlite
  • Change variable assignment from += to =
  • Replace bsd.port.pre.mk/.post.mk with a single bsd.port.mk
  • Left align Makefile comments
  • Keep comments that explain post-patch and post-install, but remove unhelpful makeplist comment
audio/forked-daapd/files/forked-daapd.in
3 ↗(On Diff #16218)

I believe that you'll need to set svn:keywords for this to be expanded.

As per comments from adamw:

  • Add svn:keywords on audio/forked-daapd/files/forked-daapd.in
  • Removed USES=compiler, as it was not needed

Also added tests to allow build on FreeBSD 9 to find zlib

As per comments from adamw:

  • convert CONFIGURE_ENV= to CONFIGURE_ENV+= for future-proofing
  • use CONFIGURE_ARGS+=--localstatedir=/var in place of post-patch
adamw edited edge metadata.
This revision is now accepted and ready to land.May 11 2016, 8:11 PM
This revision was automatically updated to reflect the committed changes.

(I know this has been committed, but...)

head/audio/forked-daapd/Makefile
37

I don't see any good reason to have both substitutions, it's not as if their value can change. Why not just write the files correctly and not do any of those substitutions ?

head/audio/forked-daapd/Makefile
37

Fair challenge. I'm happy to change it if you want. My reasons for doing it were:

  1. Both files could change if %%PREFIX%% is not default.
  2. joshruehlig@gmail.com provided me with an rc.d script which he used across many FreeNAS packages with no variation because of the name substitution. I used it in my multimedia/emby-server port, and the differentiation between RC_NAME and PORTNAME was an easy way to remember when to use - or _ for ports with a hyphen in their name.
head/audio/forked-daapd/Makefile
37

Of course both files should still have the %%PREFIX%% substitution, because it is a variable. The two other are constants, we don't use substitutions for constants, because, well, they don't change, so they don't need to be able to change.

Also, for new ports, the rc filename should == the rc name. So in this case, both should be forked_daapd. Really, it should be for all ports, but it's a long term task.

head/audio/forked-daapd/Makefile
37

(and the rc filename should == the package name, replacing - with _ if it contains one.)

head/audio/forked-daapd/Makefile
37

Ok, I will prepare a separate diff to remove any substitutions that are not %%PREFIX%%.
Just to be clear - are you suggesting the PORTNAME should be changed to replace - with _? Move the folder too? If so, maybe it is better to revert the commit and redo?

head/audio/forked-daapd/Makefile
37

Ok, new diff prepared for review here:
https://reviews.freebsd.org/D6329