On Tue, Dec 06 2022, Thomas Dettbarn <det...@dettus.net> wrote: > Hello. > > > So, I started writing a port for my retro MP3 player d11amp. > You can find the project's homepage here: https://dettus.net/d11amp > > > The port is what I have attached to this email. I tested it by > putting it into the ports tree and running "make install". > > It would be great if it could become part of the official OpenBSD > ports tree one day, but only if it meets your standards. > > > Thus far, I am not 100% happy, since I was forced to include a > do-build target. Now, its very existence seems odd to me. May I > bother one of you more experienced porters to have a look? Is it > really necessary or did I do something wrong? > (see below) > > Thomas > > > -x-x-x-x-x-x-x-x-x-x (from the Makefile) > MAKE_FLAGS= CC="${CC}" CFLAGS="${CFLAGS}" > .... > do-build: > cd ${WRKBUILD} && pwd && make
You're setting CFLAGS on the make command line because you spotted that
its value wasn't in control of the ports framework. But passing CFLAGS
on the make command-line means that the CFLAGS assignement and
subsequent appends (pkg-config --cflags ...) in upstream's Makefile are
ignored, so the build fails. Your do-build target doesn't respect
CFLAGS as set in the port Makefile, so the build succeeds.
In this kind of situation I think it's fair to patch upstream's Makefile
in order to satisfy the needs of both the ports framework and upstream's
Makefile, introducing a new variable PORTS_CFLAGS.
Here's a list of what I attempted to fix:
- zap REVISION, this is a new port
- COMMENT starts with lowercase
- move V closer to where it's used
- usual casing for PERMIT_PACKAGE
- generate WANTLIB from make port-lib-depends-check
- reorder slightly WANTLIB/MASTER_SITES (see Makefile.template)
- use PORTS_CFLAGS and patch upstream's Makefile to respect ports CFLAGS,
zap do-build target
- override INSTALLMAN and zap do-install target
- zap empty line in PLIST - see portcheck(1)
- zap trailing whitespace in DESCR - portcheck(1).
- trim double word in DESCR.
Makefile diff below and updated tarball attached.
IMHO we could do without the licence sentence in DESCR.
The updated tarball looks ready to import, if someone wants to review
and commit it: ok jca@
--- Makefile.orig Tue Dec 6 17:26:43 2022
+++ Makefile Wed Dec 7 00:13:18 2022
@@ -1,7 +1,6 @@
-V= 0.59
-REVISION= 1
-COMMENT= Simple MP3 player
+COMMENT= simple MP3 player
+V= 0.59
DISTNAME= d11amp_${V}
PKGNAME= d11amp-${V}
EXTRACT_SUFX= .tar.bz2
@@ -11,30 +10,24 @@
MAINTAINER= Thomas Dettbarn <det...@dettus.net>
# BSD
-PERMIT_PACKAGE= YES
+PERMIT_PACKAGE= Yes
+WANTLIB += c cairo cairo-gobject gdk_pixbuf-2.0 gio-2.0 glib-2.0
+WANTLIB += gobject-2.0 graphene-1.0 gtk-4 harfbuzz intl m mpg123
+WANTLIB += pango-1.0 pangocairo-1.0 portaudio pthread zip
+MASTER_SITES= https://www.dettus.net/d11amp/
+
LIB_DEPENDS= graphics/gdk-pixbuf2 \
- x11/gtk+4 \
+ x11/gtk+4 \
archivers/libzip \
audio/mpg123 \
- audio/portaudio-svn
-
-WANTLIB = c gtk-4 gdk_pixbuf-2.0 zip mpg123 portaudio
+ audio/portaudio-svn
-MASTER_SITES= https://www.dettus.net/d11amp/
-MAKE_FLAGS= CC="${CC}" CFLAGS="${CFLAGS}"
-
+MAKE_FLAGS= CC="${CC}" PORTS_CFLAGS="${CFLAGS}"
+FAKE_FLAGS= INSTALLMAN="${WRKINST}${PREFIX}/man"
TEST_ENV= SHA256_CMD=sha256 TMP_DIR=/tmp/d11amp/
TEST_TARGET= check
-
-
-do-build:
- cd ${WRKBUILD} && pwd && make
-
-do-install:
- ${INSTALL_PROGRAM} ${WRKSRC}/d11amp ${PREFIX}/bin/d11amp
- ${INSTALL_MAN} ${WRKSRC}/d11amp.1 ${PREFIX}/man/man1
.include <bsd.port.mk>
d11amp.v2.tgz
Description: Binary data
-- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
