> On Mon, Oct 19, 2015 at 12:57:39PM +0200, Dmitrij D. Czarkoff wrote: > > Michael Seyfert said: > > > This is a port for the music on console (moc) music player. > > > I have tested it on amd64, using it as my primary music player. > > > > > > I patched out most of the strcpy and scanf. > > > Please let me know what you think of it. > > > > Several comments follow. I didn't try building this port, and didn't > > look closely into patches. > > > > In moc/Makefile: > > > > | # License: GPLv3+ > > | PERMIT_PACKAGE_CDROM = Yes > > | PERMIT_PACKAGE_FTP = Yes > > | PERMIT_DISTFILES_FTP = Yes > > > > PERMIT_PACKAGE_CDROM=Yes implies PERMIT_PACKAGE_FTP=Yes and > > PERMIT_DISTFILES_FTP=Yes.
> OK, fixed this.
> >
> > | CONFIGURE_STYLE = gnu autoconf
> >
> > autoconf implies gnu.
> >
> changed to just 'gnu'.
> > | CONFIGURE_ENV = \
> > | CFLAGS="-I${LOCALBASE}/include
> > -I${LOCALBASE}/include/db4" \
> > | LDFLAGS=-L${LOCALBASE}/lib
> >
> > You should use
> >
> > CFLAGS += -I${LOCALBASE}/include -I${LOCALBASE}/include/db4
> > LDFLAGS += -L${LOCALBASE}/lib
> >
> > The way you do it shadows user settings.
> >
> Looking at the other ports they all modify LDFLAGS the way Stuart
> said, so I changed it following his suggestion:
> CONFIGURE_ENV = \
> CPPFLAGS="-I${LOCALBASE}/include -I${LOCALBASE}/include/db4" \
> LDFLAGS="-L${LOCALBASE}/lib"
> > In moc/patches/patch-mocp_1 you should use ${LOCALBASE} here and add
> > something like this to makefile:
> >
> > post-build:
> > ${SUBST_CMD} ${WRKSRC}/mocp.1
> >
> Fixed.
> Attached is another try at this port. I patched every strcpy / sprintf
> that was warned about by the compiler regardless of triviality. Is
> this a bad idea?
> It probably doesn't hurt anything, but gives extra complexity to the
> patches/ folder. This should go upstream to the official code for the
> next moc release.
Can I get this port submitted? Is anyone interested in this?
--
Michael Seyfert <mich...@codesand.org>
moc_port_try3.tgz
Description: application/tar-gz
