Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Sun, 26 Jun 2022 14:00:06 +0200

On Sun, Jun 26, 2022 at 11:07:52AM +0000, Tom Schwindl wrote:
> Let the Makefile be a bit more verbose and remove unnecessary extensions
> and flags in config.mk.
> ---
> Makefile | 37 +++++++++++++++++++------------------
> config.mk | 18 ++++++++----------
> 2 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 56e636734216..d73909283970 100644
> --- a/Makefile
> +++ b/Makefile
> _AT_@ -1,5 +1,6 @@
> # sent - plain text presentation tool
> # See LICENSE file for copyright and license details.
> +.POSIX:
>
> include config.mk
>
> _AT_@ -18,43 +19,43 @@ config.h:
> cp config.def.h config.h
>
> .c.o:
> - _AT_echo CC $<
> - _AT_${CC} -c ${CFLAGS} $<
> + _AT_echo ${CC} $<
> + ${CC} -c ${CFLAGS} $<
>
> ${OBJ}: config.h config.mk
>
> sent: ${OBJ}
> - _AT_echo CC -o $@
> - _AT_${CC} -o $@ ${OBJ} ${LDFLAGS}
> + _AT_echo ${CC} -o $@
> + ${CC} -o $_AT_ ${OBJ} ${LDFLAGS}
>
> cscope: ${SRC} config.h
> _AT_echo cScope
> - _AT_cscope -R -b || echo cScope not installed
> + cscope -R -b || echo cScope not installed
>
> clean:
> _AT_echo cleaning
> - _AT_rm -f sent ${OBJ} sent-${VERSION}.tar.gz
> + rm -f sent ${OBJ} sent-${VERSION}.tar.gz
>
> dist: clean
> _AT_echo creating dist tarball
> - _AT_mkdir -p sent-${VERSION}
> - _AT_cp -R LICENSE Makefile config.mk config.def.h ${SRC} sent-${VERSION}
> - _AT_tar -cf sent-${VERSION}.tar sent-${VERSION}
> - _AT_gzip sent-${VERSION}.tar
> - _AT_rm -rf sent-${VERSION}
> + mkdir -p sent-${VERSION}
> + cp -R LICENSE Makefile config.mk config.def.h ${SRC} sent-${VERSION}
> + tar -cf sent-${VERSION}.tar sent-${VERSION}
> + gzip sent-${VERSION}.tar
> + rm -rf sent-${VERSION}
>
> install: all
> _AT_echo installing executable file to ${DESTDIR}${PREFIX}/bin
> - _AT_mkdir -p ${DESTDIR}${PREFIX}/bin
> - _AT_cp -f sent ${DESTDIR}${PREFIX}/bin
> - _AT_chmod 755 ${DESTDIR}${PREFIX}/bin/sent
> + mkdir -p ${DESTDIR}${PREFIX}/bin
> + cp -f sent ${DESTDIR}${PREFIX}/bin
> + chmod 755 ${DESTDIR}${PREFIX}/bin/sent
> _AT_echo installing manual page to ${DESTDIR}${MANPREFIX}/man1
> - _AT_mkdir -p ${DESTDIR}${MANPREFIX}/man1
> - _AT_cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
> - _AT_chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
> + mkdir -p ${DESTDIR}${MANPREFIX}/man1
> + cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
> + chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
>
> uninstall:
> _AT_echo removing executable file from ${DESTDIR}${PREFIX}/bin
> - _AT_rm -f ${DESTDIR}${PREFIX}/bin/sent
> + rm -f ${DESTDIR}${PREFIX}/bin/sent
>
> .PHONY: all options clean dist install uninstall cscope
> diff --git a/config.mk b/config.mk
> index d61c55437f46..15fdcfa10dc7 100644
> --- a/config.mk
> +++ b/config.mk
> _AT_@ -11,20 +11,18 @@ X11INC = /usr/X11R6/include
> X11LIB = /usr/X11R6/lib
>
> # includes and libs
> -INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
> -LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> +INCS = -I/usr/include/freetype2 -I${X11INC}
> +LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
> # OpenBSD (uncomment)
> -#INCS = -I. -I${X11INC} -I${X11INC}/freetype2
> +#INCS = -I${X11INC} -I${X11INC}/freetype2
> # FreeBSD (uncomment)
> -#INCS = -I. -I/usr/local/include -I/usr/local/include/freetype2 -I${X11INC}
> -#LIBS = -L/usr/local/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> +#INCS = -I/usr/local/include/freetype2 -I${X11INC}
> +#LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
>
> # flags
> CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
> -CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
> -LDFLAGS += -g ${LIBS}
> -#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
> -#LDFLAGS += ${LIBS}
> +CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes -Wold-style-definition -Os ${INCS} ${CPPFLAGS}
> +LDFLAGS = ${LIBS}
>
> # compiler and linker
> -CC ?= cc
> +CC = cc
> --
> 2.36.1
>
>

I would remove the echo also for the verbose changes.

I see the CFLAGS is also changed. You should document it in the commit message.
I'm not sure I agree with adding -Wall or the many -Woptions though.

It is up to the current sent maintainer to decide.

-- 
Kind regards,
Hiltjo
Received on Sun Jun 26 2022 - 14:00:06 CEST

This archive was generated by hypermail 2.3.0 : Sun Jun 26 2022 - 14:00:36 CEST