Re: [new port] audio/d11amp
On Wed, Dec 07 2022, Thomas Dettbarn wrote: > On 12/7/22 20:09, Jeremie Courreges-Anglas wrote: >> On Wed, Dec 07 2022, Stuart Henderson wrote: >>> On 2022/12/07 00:35, Jeremie Courreges-Anglas wrote: 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. >>> Your version is OK sthen@ with or without tweaks mentioned here, >>> but it would be slightly neater to do this: >>> >>> -CFLAGS= -O3 -Os >>> +CFLAGS?= -O3 -Os >>> >>> (that could go to the upstream tree easily enough if Thomas wants), >>> and remove CFLAGS from MAKE_FLAGS; it's passed in via the environment >>> anyway and this then does the right thing. >> Even better, thanks for this! >> IMHO we could do without the licence sentence in DESCR. >>> agreed. >> Here's the updated tarball which I'm going to import later tonight. >> >> > May I humbly suggest this one instead? > > It patches the project's Makefile in a way which I would include in the > next revision. I only took the contents of the patches directory since your last version did not have some of the desirable fixes I had introduced earlier. > It also takes care of two typos in the manpage. I took that patch too but IMHO you might as well fix that in a new upstream release since you're upstream. :) Anyway, I have imported d11amp a few minutes ago. Thanks! -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [new port] audio/d11amp
On 12/7/22 20:09, Jeremie Courreges-Anglas wrote: On Wed, Dec 07 2022, Stuart Henderson wrote: On 2022/12/07 00:35, Jeremie Courreges-Anglas wrote: 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. Your version is OK sthen@ with or without tweaks mentioned here, but it would be slightly neater to do this: -CFLAGS= -O3 -Os +CFLAGS?= -O3 -Os (that could go to the upstream tree easily enough if Thomas wants), and remove CFLAGS from MAKE_FLAGS; it's passed in via the environment anyway and this then does the right thing. Even better, thanks for this! IMHO we could do without the licence sentence in DESCR. agreed. Here's the updated tarball which I'm going to import later tonight. May I humbly suggest this one instead? It patches the project's Makefile in a way which I would include in the next revision. It also takes care of two typos in the manpage. Thomas d11amp.v4.tgz Description: application/compressed-tar
Re: [new port] audio/d11amp
On Wed, Dec 07 2022, Stuart Henderson wrote: > On 2022/12/07 00:35, Jeremie Courreges-Anglas wrote: >> 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. > > Your version is OK sthen@ with or without tweaks mentioned here, > but it would be slightly neater to do this: > > -CFLAGS= -O3 -Os > +CFLAGS?= -O3 -Os > > (that could go to the upstream tree easily enough if Thomas wants), > and remove CFLAGS from MAKE_FLAGS; it's passed in via the environment > anyway and this then does the right thing. Even better, thanks for this! >> IMHO we could do without the licence sentence in DESCR. > > agreed. Here's the updated tarball which I'm going to import later tonight. d11amp.v3.tgz Description: Binary data -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [new port] audio/d11amp
On 12/7/22 11:48, Stuart Henderson wrote: On 2022/12/07 00:35, Jeremie Courreges-Anglas wrote: 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. Your version is OK sthen@ with or without tweaks mentioned here, but it would be slightly neater to do this: -CFLAGS= -O3 -Os +CFLAGS?= -O3 -Os (that could go to the upstream tree easily enough if Thomas wants), and remove CFLAGS from MAKE_FLAGS; it's passed in via the environment anyway and this then does the right thing. IMHO we could do without the licence sentence in DESCR. agreed. Thank you so much guys!! Please find my latest port attached to this Email. Consider the sentence about the license gone. I included the ports Makefile from Jeremie, and patches to the project's Makefile. While I was at it, I also patched away two typos from the manpage. I like it much better now! :) What are its chances of becoming part of the ports tree? Thomas d11amp.v3.tgz Description: application/compressed-tar
Re: [new port] audio/d11amp
On 2022/12/07 00:35, Jeremie Courreges-Anglas wrote: > 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. Your version is OK sthen@ with or without tweaks mentioned here, but it would be slightly neater to do this: -CFLAGS= -O3 -Os +CFLAGS?= -O3 -Os (that could go to the upstream tree easily enough if Thomas wants), and remove CFLAGS from MAKE_FLAGS; it's passed in via the environment anyway and this then does the right thing. > IMHO we could do without the licence sentence in DESCR. agreed.
Re: [new port] audio/d11amp
On Tue, Dec 06 2022, Thomas Dettbarn 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
+++ MakefileWed 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
# 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
d11amp.v2.tgz
Description: Binary data
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [new port] audio/d11amp
Hi, untested but I think you are getting bitten by the default
"ALL_TARGET=all"; try just setting ALL_TARGET= in the port.
On 2022/12/06 17:40, Thomas Dettbarn 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
>
> do-install:
> ...
> -x-x-x-x-x-x-x-x-x-x
