Re: [UPDATE] math/lrs -> 071a

On 2020/11/18 02:39, Dimitri Karamazov wrote:
> On Tue, November 17, 2020 21:01, Edd Barrett wrote:
> > I think the comment should say 1.0, since before you patched it the
> > makefile was like:
> >
> Attached below with given changes
>
> >
> > You might want to try to feed parts of that makefile patch upstream btw.
> >
> Will do
>
> >
> > If another dev OKs this (and specifically checks the 'a' suffix on the
> > version number), I'd be happy to commit it.
> >
> Parts can also be numbers with an optional letter appended (packages-specs)
> Also I don't see any reservation for letter 'a'. Only thing which comes
> close to it is 'alpha' which has to be in long-form.

lrs-042 -> lrs-071a is fine and treated as expected. If in doubt you can
tweak /usr/src/regress/usr.sbin/pkg_add/check-name (see check_order right
at the bottom) to test.

quick comments on the rest, I haven't looked closely

> ++++ makefile Sun Nov 15 22:58:52 2020
> +@@ -35,7 +35,7 @@ default: lrs
> + INCLUDEDIR = /usr/local/include
> + LIBDIR = /usr/local/lib
> +
> +-CFLAGS = -O3 -Wall
> ++CFLAGS ?= -O3 -Wall

usually there's no need to patch to change = to ?= like this, just pass
CFLAGS in via MAKE_FLAGS - these are passed on the command line rather
than environment, and override things in the makefile. Sometimes you do
need to patch for ?= but usually only when one makefile is explicitly
calling a child make process and you need to pass things through the
environment instead.

> + lrs: ${LRSOBJ}
> +- $(CC) ${CFLAGS} -DMA -DB128 -L${LIBDIR} -o lrs ${LRSOBJ} -lgmp
> +- $(CC) -O3 -DGMP -I${INCLUDEDIR} -o lrsgmp lrs.c lrslib.c lrsgmp.c lrsdriver.c -L${LIBDIR} -lgmp
> +- $(CC) -O3 hvref.c -o hvref
> ++ $(CC) -Wall ${CFLAGS} -DMA -DB128 ${LDFLAGS} -o lrs ${LRSOBJ} -lgmp
> ++ $(CC) ${CFLAGS} -DGMP ${CPPFLAGS} -o lrsgmp lrs.c lrslib.c lrsgmp.c lrsdriver.c ${LDFLAGS} -lgmp
> ++ $(CC) ${CFLAGS} hvref.c -o hvref
> + ln -s -f lrs redund
> + ln -s -f lrsgmp redundgmp
> +
> + lrs64: ${LRSOBJ64}
> +- $(CC) ${CFLAGS} -DMA -L${LIBDIR} -o lrs ${LRSOBJ64} -lgmp
> ++ $(CC) -Wall ${CFLAGS} -DMA ${LDFLAGS} -o lrs ${LRSOBJ64} -lgmp
> +
> + lrs.o: lrs.c
> +- $(CC) ${CFLAGS} -DMA -DB128 -c -o lrs.o lrs.c
> ++ $(CC) -Wall ${CFLAGS} -DMA -DB128 -c -o lrs.o lrs.c
> +
> + lrs64.o: lrs.c
> +- $(CC) ${CFLAGS} -DMA -c -o lrs64.o lrs.c
> ++ $(CC) -Wall ${CFLAGS} -DMA -c -o lrs64.o lrs.c

Removing -O3 is correct for ports, but these many additions of -Wall
seem unnecessary churn - apart from being a pain for updates, they also
make it hard to see what the rest of the patch is doing.

If you want extra warnings just add -Wall to CFLAGS when passed in.

> + ######################################################################
> + # From here on the author is David Bremner <bremner@unb.ca> to whom you should turn for help
> + #
> + # Shared library variables
> +-SONAME ?=liblrs.so.1
> +-SOMINOR ?=.0.0
> ++SONAME ?=liblrs.so.${LMAJ}
> ++SOMINOR ?=.${LMIN}
> + SHLIB ?=$(SONAME)$(SOMINOR)
> + SHLINK ?=liblrs.so

You can probably pass this in via MAKE_FLAGS too.