Brian Robert Callahan

academic, developer, with an eye towards a brighter techno-social life

2021-08-02
Reviewing my first OpenBSD port, and what I'd do differently 10 years later

By popular demand, I have unearthed my first OpenBSD port. I will review it and discuss what I would do differently now after ten years of working on ports.

I will have a YouTube video reviewing the port up in a few days. I'll update this post once it's up.

Beret: A puzzle-platformer game

The first port I ever sent was Beret, a 2D puzzle-platformer game. I guess even from the beginning I was helping out thfr@ in his #PlayOnBSD project. I cannot find the very first attempt at it, as it seems I linked a tarball from a server that has since gone to the great bitbucket in the sky. So I have the first tarball I posted instead.

`Makefile`

The most "important," or at least meaty, part of any port is usually the Makefile and that is true here as well. Let's take a look at it. This is as I posted it, mistakes and all.

# $OPENBSD$

COMMENT=	2D puzzle-platformer game
DISTNAME=	beret-1.2.1
CATEGORIES=	games x11

HOMEPAGE=	http://kiwisauce.com/beret/

MAINTAINER=	Brian Callahan <bcallah@devio.us>

# LGPLv3
PERMIT_PACKAGE_CDROM=	Yes
PERMIT_PACKAGE_FTP=	Yes
PERMIT_DISTFILES_CDROM=	Yes
PERMIT_DISTFILES_FTP=	Yes

WANTLIB += c m SDL SDL_image SDL_mixer
WANTLIB += SDL_ttf pthread

MASTER_SITES=	http://devio.us/~bcallah/

LIB_DEPENDS=	converters/libiconv \
		devel/sdl \
		devel/sdl-image \
		devel/sdl-mixer \
		devel/sdl-ttf \
		print/freetype

MAKE_FLAGS=	CC="${CC}" CFLAGS="${CFLAGS}"
CFLAGS+=	-I/usr/local/include \
		-I/usr/local/include/SDL -Wall -g

do-install:
	${INSTALL_PROGRAM} ${WRKSRC}/beret ${PREFIX}/bin
	${INSTALL_DATA_DIR} ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/lgpl-3.0.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/OFL.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/FONTLOG.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/README.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret
	${INSTALL_DATA} ${WRKSRC}/AveriaSans-Regular.ttf ${PREFIX}/share/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/images
	${INSTALL_DATA} ${WRKSRC}/images/* ${PREFIX}/share/beret/images
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/music
	${INSTALL_DATA} ${WRKSRC}/music/* ${PREFIX}/share/beret/music
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/rooms
	${INSTALL_DATA} ${WRKSRC}/rooms/* ${PREFIX}/share/beret/rooms
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/sfx
	${INSTALL_DATA} ${WRKSRC}/sfx/* ${PREFIX}/share/beret/sfx

.include <bsd.port.mk>

OK, so things have changed in the interim decade. I am going to fix up this Makefile to fit into today's ports infrastructure, since you cannot build the port as-is. What needs to change? Fortunately not too much. The PERMIT_PACKAGE lines have been simplified. We used to have all these permutations when we shipped CDs. But today that no longer happens and it has been streamlined to a single PERMIT_PACKAGE line. So let's do that to make our lives a little easier.

# $OPENBSD$

COMMENT=	2D puzzle-platformer game
DISTNAME=	beret-1.2.1
CATEGORIES=	games x11

HOMEPAGE=	http://kiwisauce.com/beret/

MAINTAINER=	Brian Callahan <bcallah@devio.us>

# LGPLv3
PERMIT_PACKAGE =	Yes

WANTLIB += c m SDL SDL_image SDL_mixer
WANTLIB += SDL_ttf pthread

MASTER_SITES=	http://devio.us/~bcallah/

LIB_DEPENDS=	converters/libiconv \
		devel/sdl \
		devel/sdl-image \
		devel/sdl-mixer \
		devel/sdl-ttf \
		print/freetype

MAKE_FLAGS=	CC="${CC}" CFLAGS="${CFLAGS}"
CFLAGS+=	-I/usr/local/include \
		-I/usr/local/include/SDL -Wall -g

do-install:
	${INSTALL_PROGRAM} ${WRKSRC}/beret ${PREFIX}/bin
	${INSTALL_DATA_DIR} ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/lgpl-3.0.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/OFL.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/FONTLOG.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/README.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret
	${INSTALL_DATA} ${WRKSRC}/AveriaSans-Regular.ttf ${PREFIX}/share/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/images
	${INSTALL_DATA} ${WRKSRC}/images/* ${PREFIX}/share/beret/images
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/music
	${INSTALL_DATA} ${WRKSRC}/music/* ${PREFIX}/share/beret/music
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/rooms
	${INSTALL_DATA} ${WRKSRC}/rooms/* ${PREFIX}/share/beret/rooms
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/sfx
	${INSTALL_DATA} ${WRKSRC}/sfx/* ${PREFIX}/share/beret/sfx

.include <bsd.port.mk>

OK, let's start from the top and work our way down.

Our first issue in on the very top line. The RCS ID should read $OpenBSD$ . Not a huge problem, but needs to be fixed.

One important style tweak that you may have noticed I did with the new PERMIT_PACKAGE line: putting a space after each variable name. I would highly recommend for all new ports that be done.

Moving on, I would make the license marker more exacting. The current tag, LGPLv3 is slightly ambiguous: it could mean LGPLv3 only or it could mean LGPLv3 or later. For Beret, it is LGPLv3 only, so that's what I would tweak the license marker to.

In the LIB_DEPENDS list, which informs the ports system the shared libraries also found in ports that are linked into this program, and thus are needed on the system at both build time and run time, there seem to be extra libraries. Both converters/libiconv and print/freetype do not appear in the WANTLIB list, which is the list of all shared libraries this ports needs, so that tells me that either these libraries are not needed or the WANTLIB list is incorrect. We will learn which of these is the case once we have successfully built the port. For now, let's assume they are extra and remove them. If we're wrong about that, we'll find out soon enough and the fix is easy enough: add them back.

Additionally, the devel/sdl entry is also extraneous. It will get pulled in by all of the SDL sub-libraries.

The next tweak I would make are to combine the MAKE_FLAGS and CFLAGS lines. There is a utility, sdl-config that can be used to get the correct CFLAGS for SDL programs. So I would compress these two lines into the following.

MAKE_FLAGS =	CC="${CC}" CFLAGS="${CFLAGS} `sdl-config --cflags`"

Shorter and I suppose more pedantically correct.

The last tweak I would make would be to put a comment about why we have the do-install routine. In this case, it is because upstream did not write an installation routine, so we have to do it ourselves.

By the way, in case you didn't know, there are pre-, do-, and post- routines for all the major steps of the build process. The pre- and post- run immediately before or immediately after the suffixed step, while do- replaces the step with whatever is specified. Note that post- of a step is run before pre- of the subsequent step, so post-configure will run before pre-build.

Looking over the do-install routine, I noticed that a font is being installed. It is not always the case, but it is often enough the case that fonts are licensed under a different license from the rest of the code, usually the SIL Open Font License. I checked and indeed the font is licensed under the SIL OFL. We should notate that in the license marker.

Oh, and that MASTER_SITES doesn't exist anymore. The current port uses a server that does exist. Let's go ahead and change it too to make our lives easier.

Let's rewrite our Makefile with all these changes.

# $OpenBSD$

COMMENT =	2D puzzle-platformer game
DISTNAME =	beret-1.2.1
CATEGORIES =	games x11

HOMEPAGE =	http://kiwisauce.com/beret/
MAINTAINER =	Brian Callahan <bcallah@devio.us>

# LGPLv3 only
# AveriaSans-Regular.ttf: SIL Open Font License
PERMIT_PACKAGE =	Yes

WANTLIB += c m SDL SDL_image SDL_mixer
WANTLIB += SDL_ttf pthread

MASTER_SITES =	https://mirrors.nycbug.org/pub/distfiles/

LIB_DEPENDS =	devel/sdl-image \
		devel/sdl-mixer \
		devel/sdl-ttf

MAKE_FLAGS=	CC="${CC}" CFLAGS="${CFLAGS} `sdl-config --cflags`"

# Upstream does not provide an installation routine.
do-install:
	${INSTALL_PROGRAM} ${WRKSRC}/beret ${PREFIX}/bin
	${INSTALL_DATA_DIR} ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/lgpl-3.0.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/OFL.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/FONTLOG.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/README.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret
	${INSTALL_DATA} ${WRKSRC}/AveriaSans-Regular.ttf ${PREFIX}/share/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/images
	${INSTALL_DATA} ${WRKSRC}/images/* ${PREFIX}/share/beret/images
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/music
	${INSTALL_DATA} ${WRKSRC}/music/* ${PREFIX}/share/beret/music
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/rooms
	${INSTALL_DATA} ${WRKSRC}/rooms/* ${PREFIX}/share/beret/rooms
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/sfx
	${INSTALL_DATA} ${WRKSRC}/sfx/* ${PREFIX}/share/beret/sfx

.include <bsd.port.mk>

`patches/`

Now let's take a look at the patches directory. Inside there are two patches: patch-Makefile and patch-game_c. Let's take them one at a time. Here is patch-Makefile.

$OpenBSD$
--- Makefile.orig	Tue Jan 10 01:55:02 2012
+++ Makefile	Wed Jan 11 14:58:26 2012
@@ -1,13 +1,11 @@
-CC= gcc
-CFLAGS= -Wall -g
-LFLAGS= -lSDLmain -lSDL -lSDL_image -lSDL_ttf -lSDL_mixer
+LFLAGS= -L/usr/local/lib -L/usr/X11R6/lib -lSDLmain -lSDL -lSDL_image -lSDL_ttf -lSDL_mixer
 
-default: beret
+all: beret
 
 clean:
 	rm beret *.o
 
 beret: game.o thing.o physics.o
-	$(CC) $(LFLAGS) -o $@ game.o thing.o physics.o
+	$(CC) $(LFLAGS) -pthread -o $@ game.o thing.o physics.o
 
 %.o: %.c %.h

OK, this looks less than ideal. We don't want to hardcode /usr/local or /usr/X11R6. Instead, we can again use sdl-config to get the correct linker flags. This would be added to the Makefile we just finished editing and removed from this patch. We should do the following.

MAKE_FLAGS =	CC="${CC}" CFLAGS="${CFLAGS} `sdl-config --cflags`" \
		LFLAGS="${LDFLAGS} `sdl-config --libs` -lSDL_image -lSDL_mixer -lSDL_ttf"

Additionally, we can set ALL_TARGET in the Makefile as well, which will completely eliminate the need for this patch. It looks like this.

ALL_TARGET =	default

Neat! One less patch is one less thing to worry about maintaining. I will reproduce a new Makefile after we look at all the patches, in case we can also elimate the other patch.

Here is patch-game_c.

$OpenBSD$
--- game.c.orig	Tue Jan 10 01:55:02 2012
+++ game.c	Thu Jan 12 11:33:20 2012
@@ -95,7 +95,7 @@
 #define RESOURCE_PATH ""
 #else
 #define SUPPORT_PATH ".beret/"
-#define RESOURCE_PATH ""
+#define RESOURCE_PATH "/usr/local/share/beret/"
 #endif
 
 #define QUITMOD_WIN KMOD_ALT

Same thing here, we do not want to hardcode /usr/local. Additionally, we should put a comment at the top of the patch explaining why this patch is needed. It will help out a lot to remind you why you did something years later when you need to update to port.

Here is the new patch.

$OpenBSD$

Install the game data files to ${TRUEPREFIX}/share/beret
This matches the do-install routine.

--- game.c.orig	Tue Jan 10 01:55:02 2012
+++ game.c	Thu Jan 12 11:33:20 2012
@@ -95,7 +95,7 @@
 #define RESOURCE_PATH ""
 #else
 #define SUPPORT_PATH ".beret/"
-#define RESOURCE_PATH ""
+#define RESOURCE_PATH "${TRUEPREFIX}/share/beret/"
 #endif
 
 #define QUITMOD_WIN KMOD_ALT

We will need to add a do-configure routine to expand ${TRUEPREFIX}.

Trying our revised port

Let's try to build our new port. This is the finalized Makefile and remember too we have removed patch-Makefile.

# $OpenBSD$

COMMENT =	2D puzzle-platformer game
DISTNAME =	beret-1.2.1pl1
CATEGORIES =	games x11

HOMEPAGE =	http://kiwisauce.com/beret/
MAINTAINER =	Brian Callahan <bcallah@devio.us>

# LGPLv3 only
# AveriaSans-Regular.ttf: SIL Open Font License
PERMIT_PACKAGE =	Yes

WANTLIB += c m SDL SDL_image SDL_mixer
WANTLIB += SDL_ttf pthread

MASTER_SITES =	https://mirrors.nycbug.org/pub/distfiles/

LIB_DEPENDS =	devel/sdl-image \
		devel/sdl-mixer \
		devel/sdl-ttf

MAKE_FLAGS =	CC="${CC}" CFLAGS="${CFLAGS} `sdl-config --cflags`" \
		LFLAGS="${LDFLAGS} `sdl-config --libs` -lSDL_image -lSDL_mixer -lSDL_ttf"

# Expand ${TRUEPREFIX}
do-configure:
	${SUBST_CMD} ${WRKSRC}/game.c

# Upstream does not provide an installation routine.
do-install:
	${INSTALL_PROGRAM} ${WRKSRC}/beret ${PREFIX}/bin
	${INSTALL_DATA_DIR} ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/lgpl-3.0.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/OFL.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/FONTLOG.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/README.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret
	${INSTALL_DATA} ${WRKSRC}/AveriaSans-Regular.ttf ${PREFIX}/share/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/images
	${INSTALL_DATA} ${WRKSRC}/images/* ${PREFIX}/share/beret/images
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/music
	${INSTALL_DATA} ${WRKSRC}/music/* ${PREFIX}/share/beret/music
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/rooms
	${INSTALL_DATA} ${WRKSRC}/rooms/* ${PREFIX}/share/beret/rooms
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/sfx
	${INSTALL_DATA} ${WRKSRC}/sfx/* ${PREFIX}/share/beret/sfx

.include <bsd.port.mk>

Let's start the build process. I changed DISTNAME to match the current name of the tarball the port is made from.

make makesum, which fetches the tarball and generates a distfile, worked fine. But I didn't expect any issues here. make configure, which will run make extract and make patch as well, also worked fine and I see that the variable expansion took place correctly.

But make build failed on the linker stage. It looks like the LFLAGS didn't catch. So let's revise. Let's replace our LFLAGS line within MAKE_FLAGS to be LDFLAGS="${LDFLAGS} `sdl-config --libs`" instead. We will have to recreate a patch-Makefile but it will be much more simple.

$OpenBSD$

Use the LDFLAGS we specified in the port Makefile.

Index: Makefile
--- Makefile.orig
+++ Makefile
@@ -8,6 +8,6 @@ clean:
        rm beret *.o

 beret: game.o thing.o physics.o
-       $(CC) $(LFLAGS) -o $@ game.o thing.o physics.o
+       $(CC) ${LDFLAGS} $(LFLAGS) -o $@ game.o thing.o physics.o

 %.o: %.c %.h

Let's try building again. Another linker error. This time it reports some undefined symbols that live in libm so we can just add that. We can just tack it on to our new LDFLAGS line or in the patch itself. I put it in the LDFLAGS line.

OK, now it builds. Let's run make update-plist which will run make fake first and the generate pkg/PLIST. Then we'll run make test and it complains there are no tests, so we will need to add NO_TEST=Yes to our Makefile. Finally, we will run make port-lib-depends-check to check WANTLIB. I am going to presume my WANTLIB is wrong or at least out of style, so I'll delete it entirely and copy and paste the output of make port-lib-depends-check.

Here is our finished Makefile.

# $OpenBSD$

COMMENT =	2D puzzle-platformer game
DISTNAME =	beret-1.2.1pl1
CATEGORIES =	games x11

HOMEPAGE =	https://kiwisauce.com/beret/
MAINTAINER =	Brian Callahan <bcallah@devio.us>

# LGPLv3 only
# AveriaSans-Regular.ttf: SIL Open Font License
PERMIT_PACKAGE =	Yes

WANTLIB += SDL SDL_image SDL_mixer SDL_ttf c m pthread

MASTER_SITES =	https://mirrors.nycbug.org/pub/distfiles/

LIB_DEPENDS =	devel/sdl-image \
		devel/sdl-mixer \
		devel/sdl-ttf

ALL_TARGET =	beret
MAKE_FLAGS =	CC="${CC}" CFLAGS="${CFLAGS} `pkg-config --cflags sdl`" \
		LDFLAGS="${LDFLAGS} `pkg-config --libs sdl` -lm"

NO_TEST =	Yes

do-configure:
	${SUBST_CMD} ${WRKSRC}/game.c

do-install:
	${INSTALL_PROGRAM} ${WRKSRC}/beret ${PREFIX}/bin
	${INSTALL_DATA_DIR} ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/lgpl-3.0.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/OFL.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/FONTLOG.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA} ${WRKSRC}/README.txt ${PREFIX}/share/doc/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret
	${INSTALL_DATA} ${WRKSRC}/AveriaSans-Regular.ttf ${PREFIX}/share/beret
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/images
	${INSTALL_DATA} ${WRKSRC}/images/* ${PREFIX}/share/beret/images
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/music
	${INSTALL_DATA} ${WRKSRC}/music/* ${PREFIX}/share/beret/music
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/rooms
	${INSTALL_DATA} ${WRKSRC}/rooms/* ${PREFIX}/share/beret/rooms
	${INSTALL_DATA_DIR} ${PREFIX}/share/beret/sfx
	${INSTALL_DATA} ${WRKSRC}/sfx/* ${PREFIX}/share/beret/sfx

.include <bsd.port.mk>

To complete the port, I just used the existing pkg/DESCR.

Testing to make sure it works

Now we can run make install and see if it works. It does, though that isn't too surprising. And with that, we have finished revising our port.

Conclusion

Following along with the upcoming video may be more illuminating than just the blog post. But I hope you learned something about how I approach creating OpenBSD ports. I enjoyed revising my first port, though I'll note it's basically identical to the current version of the port. That's kinda nice.

Brian Robert Callahan

[prev]

[next]

2021-08-02
Reviewing my first OpenBSD port, and what I'd do differently 10 years later

Beret: A puzzle-platformer game

`Makefile`

`patches/`

Trying our revised port

Testing to make sure it works

Conclusion

Brian Robert Callahan

[prev]

[next]

2021-08-02Reviewing my first OpenBSD port, and what I'd do differently 10 years later

Beret: A puzzle-platformer game

Makefile

patches/

Trying our revised port

Testing to make sure it works

Conclusion

2021-08-02
Reviewing my first OpenBSD port, and what I'd do differently 10 years later

`Makefile`

`patches/`