Wednesday, February 02, 2022

Re: [new] graphics/nsxiv - neo simple X image viewer

Hello,

Thim <thim@cederlund.de> writes:

> Hi,
>
> The developer for sxiv (simple X image viewer) stopped maintaining it a
> couple of months back and so here is a port of nsxiv.
>
> It is a fork that aims to be a drop-in replacement but with added
> features. Since it is essentially the same program, the makefile for
> sxiv is heavily used.
>
> As always, let me know what you think.

it works fine and handles gif better than gpicview :)

some comments on the port:

: COMMENT = neo simple X Image Viewer
:
: GH_PROJECT = nsxiv
: GH_ACCOUNT = nsxiv
: GH_TAGNAME = v28
: REVISION = 1

REVISION should start empty (no revision at all) and is incremented when
the port is modified without bumping the version (0, 1, 2, ...)

: [...]
: WANTLIB += Imlib2 X11 Xft c exif fontconfig gif lib/inotify/inotify

make port-lib-depends-check complains that webp and webpdemux are
missing.

(it also complains that inotify is not reachable, but I *think* that's a
limitation of the script, as it's devel/libinotify is in LDEPS)

: RUN_DEPENDS = devel/desktop-file-utils \
: x11/gtk+3,-guic \
: graphics/libwebp

this is wrong. libwebp provides the *library* libwebp it should be
listed under LIB_DEPENDS. portcheck complains fact that:

> the following libraries in WANTLIB look like masked by RUN_DEPENDS:
> webp webpdemux

: LIB_DEPENDS = devel/libinotify \
: graphics/imlib2 \
: graphics/libexif

^^ so graphics/libwebp should only be listed here

: MAKE_FLAGS = CC="${CC}" \
: V=1 \
: PREFIX=${PREFIX} \
: MANPREFIX=${PREFIX}/man \
: CFLAGS="${CFLAGS} -I${X11BASE}/include -I${X11BASE}/include/freetype2 \
: -I${LOCALBASE}/include -I${LOCALBASE}/include/inotify " \
: LDFLAGS="-L${X11BASE}/lib -L${LOCALBASE}/lib -L${LOCALBASE}/lib/inotify \
: -linotify -Wl,-rpath ${LOCALBASE}/lib/inotify -lwebp" \

not really a problem in itself, and sxiv makefile does the same, but
I'd modify CFLAGS/LDFLAGS on a separate line before and then inject them
here, it'd make this chunk a little bit more readable even if a little
more verbose.

: NO_TEST = Yes
:
: post-patch:
: ${SUBST_CMD} ${WRKSRC}/nsxiv.1
:
: # Git errors break version.h build; this makes it fall back to a hardcoded value
: pre-build:
: ln -sf /usr/bin/true ${WRKDIR}/bin/git
: sed -i 's/$$(CC) $$(LDFLAGS) -o $$@/$$(CC) *.o $$(LDFLAGS) -o $$@/g' \
: ${WRKSRC}/Makefile

I think it's more clear to just patch the makefile. Instead of `*.o'
you can just use $(OBJS). Such patch could also be upstreamed...

: post-install:
: ${INSTALL_DATA_DIR} ${PREFIX}/share/applications
: ${INSTALL_DATA} ${WRKSRC}/nsxiv.desktop ${PREFIX}/share/applications
:
: .include <bsd.port.mk>

No comments:

Post a Comment