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

Hi Omar,

On 2022-02-02 20:40:17, Omar Polo wrote:
> Hello,
>
> : 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, ...)

I see, thanks.

> : [...]
> : WANTLIB += Imlib2 X11 Xft c exif fontconfig gif lib/inotify/inotify
>
> make port-lib-depends-check complains that webp and webpdemux are
> missing.

Darn it, missed this step entirely. Thanks


> : 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.

I could do that yeah, but the issue here is making the include line
without exceeding 80 characters. In any case the \ split is necessary
for formatting reasons, like so.

: _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"
:
: MAKE_FLAGS = CC="${CC}" \
: V=1 \
: PREFIX=${PREFIX} \
: MANPREFIX=${PREFIX}/man \
: CFLAGS="${CFLAGS} ${_CFLAGS}" \
: LDFLAGS="${_LDFLAGS}"
:

It is like comparing apples to apples to me. Suggestions?


> : # 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...

Agreed, I will make a patch for the Makefile instead


Best Regards,

Thim Cederlund