Makefile enhancement

[outpaddling]

The Makefile patch below adds an install target and allows the caller to control compiler flags.

This will make it easier to add minimap2 to various package managers.

If you prefer a pull request, I can do that too.

Regards,

Jason

--- Makefile.orig	2019-02-28 20:49:24 UTC
+++ Makefile
@@ -1,11 +1,19 @@
-CFLAGS=		-g -Wall -O2 -Wc++-compat #-Wextra
-CPPFLAGS=	-DHAVE_KALLOC
+CFLAGS?=	-g -Wall -O2
+CFLAGS+=	-Wc++-compat #-Wextra
+CPPFLAGS+=	-DHAVE_KALLOC
 INCLUDES=
 OBJS=		kthread.o kalloc.o misc.o bseq.o sketch.o sdust.o options.o index.o chain.o align.o hit.o map.o format.o pe.o esterr.o splitidx.o ksw2_ll_sse.o
 PROG=		minimap2
 PROG_EXTRA=	sdust minimap2-lite
 LIBS=		-lm -lz -lpthread
 
+PREFIX?=	/usr/local
+DESTDIR?=	.
+MAN1DIR?=	${PREFIX}/man/man1
+MKDIR?=		mkdir
+INSTALL?=	install
+STRIP?=		strip
+
 ifeq ($(arm_neon),) # if arm_neon is not defined
 ifeq ($(sse2only),) # if sse2only is not defined
 	OBJS+=ksw2_extz2_sse41.o ksw2_extd2_sse41.o ksw2_exts2_sse41.o ksw2_extz2_sse2.o ksw2_extd2_sse2.o ksw2_exts2_sse2.o ksw2_dispatch.o
@@ -84,6 +92,15 @@ ksw2_exts2_neon.o:ksw2_exts2_sse.c ksw2.h kalloc.h
 		$(CC) -c $(CFLAGS) $(CPPFLAGS) -DKSW_SSE2_ONLY -D__SSE2__ $(INCLUDES) $< -o $@
 
 # other non-file targets
+
+install: all
+	${MKDIR} -p ${DESTDIR}${PREFIX}/bin
+	${MKDIR} -p ${DESTDIR}${MAN1DIR}
+	${INSTALL} -c minimap2 ${DESTDIR}${PREFIX}/bin
+	${INSTALL} -c minimap2.1 ${DESTDIR}${MAN1DIR}
+
+install-strip: install
+	${STRIP} ${DESTDIR}${PREFIX}/bin/minimap2
 
 clean:
 		rm -fr gmon.out *.o a.out $(PROG) $(PROG_EXTRA) *~ *.a *.dSYM build dist mappy*.so mappy.c python/mappy.c mappy.egg*

[lh3]

With CFLAGS?=, make CFLAGS+=-Wextra doesn't work as intended. Also, different systems have different preferences for the location of the manpages and whether it should be gzip'd. As such, I will let the install script deal with this. Thanks for adding minimap2 to freebsd and for the suggestions anyway. I appreciate.

[jmarshall]

Agreed that there is no need for ?= when the caller can use make -e or make CFLAGS=… to control compiler flags.

But having an install target is a way for for minimap2 to give instructions to distributors about what parts its author thinks should be installed. Doing it via variables like MAN1DIR lets distributors set the man pages location (and they can compress or strip things themselves in their own scripts after using your make install if they wish).

[outpaddling]

Yes, one can override CFLAGS with command-line arguments, but with = instead of ?=, make will not respect CFLAGS in the environment (or in m[ak]e.conf on some systems), which is how many package building systems provide them. Hence, I always set non-essential flags with ?= so they are only default values that the user or package manager can override, and append project-specific essential flags with +=.

I'm not sure what the issue is with ?= anyway. For me, CFLAGS+=-Wextra has the same effect with or without the patch I provided:

Original Makefile:

<<<ROOT@toshiba.acadix>>> /usr/ports/wip/minimap2/work/minimap2-2.16 1062 # head Makefile 
CFLAGS=		-g -Wall -O2 -Wc++-compat #-Wextra
CPPFLAGS=	-DHAVE_KALLOC
INCLUDES=
OBJS=		kthread.o kalloc.o misc.o bseq.o sketch.o sdust.o options.o index.o chain.o align.o hit.o map.o format.o pe.o esterr.o splitidx.o ksw2_ll_sse.o
PROG=		minimap2
PROG_EXTRA=	sdust minimap2-lite
LIBS=		-lm -lz -lpthread

ifeq ($(arm_neon),) # if arm_neon is not defined
ifeq ($(sse2only),) # if sse2only is not defined
<<<ROOT@toshiba.acadix>>> /usr/ports/wip/minimap2/work/minimap2-2.16 1063 # gmake CFLAGS+=-Wextra | head -5
cc -c -Wextra -DHAVE_KALLOC  main.c -o main.o
cc -c -Wextra -DHAVE_KALLOC  kthread.c -o kthread.o
cc -c -Wextra -DHAVE_KALLOC  kalloc.c -o kalloc.o
cc -c -Wextra -DHAVE_KALLOC  misc.c -o misc.o
cc -c -Wextra -DHAVE_KALLOC  bseq.c -o bseq.o

Patched Makefile:

<<<ROOT@toshiba.acadix>>> /usr/ports/wip/minimap2/work/minimap2-2.16 1068 # head Makefile
CFLAGS?=	-g -Wall -O2
CFLAGS+=	-Wc++-compat #-Wextra
CPPFLAGS+=	-DHAVE_KALLOC
INCLUDES=
OBJS=		kthread.o kalloc.o misc.o bseq.o sketch.o sdust.o options.o index.o chain.o align.o hit.o map.o format.o pe.o esterr.o splitidx.o ksw2_ll_sse.o
PROG=		minimap2
PROG_EXTRA=	sdust minimap2-lite
LIBS=		-lm -lz -lpthread

PREFIX?=	/usr/local
<<<ROOT@toshiba.acadix>>> /usr/ports/wip/minimap2/work/minimap2-2.16 1069 # gmake CFLAGS+=-Wextra | head -5
cc -c -Wextra -DHAVE_KALLOC  main.c -o main.o
cc -c -Wextra -DHAVE_KALLOC  kthread.c -o kthread.o
cc -c -Wextra -DHAVE_KALLOC  kalloc.c -o kalloc.o
cc -c -Wextra -DHAVE_KALLOC  misc.c -o misc.o
cc -c -Wextra -DHAVE_KALLOC  bseq.c -o bseq.o

[jmarshall]

Yes, that's why I said make -e. Why are you ?= advocates always so resistant to using -e instead?

[outpaddling]

Sorry, hadn't had my morning tea yet and forgot to address the -e comment.

To answer your question, because -e is also a command-line argument, which would have to be explicitly inserted into each package building framework, whereas ?= achieves the same goal without any additional flags. There's a strong tradition among package developers to make the build system as clean as possible, largely by pushing patches upstream to eliminate unnecessary cruft in the next release. When you're part of a team maintaining 30,000+ packages, every little bit helps.

Can you provide an example where ?= causes a problem that's avoided by using make -e?

[jmarshall]

The commands and their flags invoked by a Makefile are clear and predictable, because they are written down right there in the Makefile. If I want my repeatable build commands to be influenced by external sources like environment variables, I will explicitly ask for that via make -e. Using ?= removes Make's default deterministic self-contained behaviour.

(I'm not interested in you trying to change my mind on this, thanks. I understand your argument.)

[outpaddling]

I understand what you're saying. That's why only non-essential flags like "-g -Wall -O2" should be set with ?= and all essential arguments added with +=. This gives the caller control over local preferences without risk of breaking the build.