mptre / pick

A fuzzy search tool for the command-line
MIT License
814 stars 42 forks source link

Replace autoconf #291

Closed mptre closed 5 years ago

mptre commented 5 years ago

This has been on my list for a long while by now. Still some dependency issue with make test using GNU make, tried using order-only-prerequisites without success.

Please give it a try on your platform.

/cc @calleerlandsson @mike-burns @dbotw @teoljungberg

ghost commented 5 years ago

Quoting https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html#Suffix-Rules:

Suffix rules cannot have any prerequisites of their own.
diff --git a/tests/Makefile b/tests/Makefile
index cfa3acb..4fd4978 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -42,11 +42,11 @@ TESTS+=     opt-s.t
 TESTS+=        opt-unknown.t
 TESTS+=        opt-x.t

-all: ${TESTS:.t=.out}
+all: ${PROG} ${TESTS:.t=.out}

 .SUFFIXES: .t .out

-.t.out: ${PROG}
+.t.out:
        sh ${.CURDIR}/pick-test.sh -b ${.OBJDIR}/${PROG} $<
mptre commented 5 years ago

On Thu, Feb 21, 2019 at 03:45:47AM -0800, Jens Guenther wrote:

Quoting https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html#Suffix-Rules:

Suffix rules cannot have any prerequisites of their own.

Thanks for the pointer.


diff --git a/tests/Makefile b/tests/Makefile
index cfa3acb..4fd4978 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -42,11 +42,11 @@ TESTS+=     opt-s.t
 TESTS+=        opt-unknown.t
 TESTS+=        opt-x.t

-all: ${TESTS:.t=.out}
+all: ${PROG} ${TESTS:.t=.out}

But there's no guarantee that PROG gets resolved before TESTS?

teoljungberg commented 5 years ago

The deprecation of -v doesn't look like a deprecation, just a deletion. Semantically to me it's not the same to delete an option and to deprecate it, the latter displays a warning that it will be removed in the next major version bump.

teoljungberg commented 5 years ago

I checked out the branch, ran the commands in CONTRIBUTING.md to build the project, with no avail:

pick autoremove % ./configure
pick autoremove % make
/Users/teo/src/pick/Makefile:1: /Users/teo/src/pick/Makefile.inc: No such file or directory
make: *** No rule to make target `/Users/teo/src/pick/Makefile.inc'.  Stop.
pick autoremove % make test
/Users/teo/src/pick/Makefile:1: /Users/teo/src/pick/Makefile.inc: No such file or directory
make: *** No rule to make target `/Users/teo/src/pick/Makefile.inc'.  Stop.

How do I generate Makefile.inc? That wasn't documented.

ghost commented 5 years ago

The deprecation of -v doesn't look like a deprecation, just a deletion.

... and IMHO there is no good reason for this ...

ghost commented 5 years ago

How do I generate Makefile.inc? That wasn't documented.

./configure should create the Makefile.Inc, but there is a problem with sh (bash 3.2) on Mac OS X: the config.log file shows ./configure: line 19: @: unbound variable ...

Changing line 19 in ./configure to

$CC -Werror -o /dev/null -x c - "$@"

fixes this and Makefile.Inc should look like

CC=     cc
CFLAGS=     -Wall -Wextra -MD -MP
CPPFLAGS=   
DEBUG=      
INSTALL=    install
LDFLAGS=    -lcurses
MALLOC_OPTIONS=
ghost commented 5 years ago
diff --git a/tests/Makefile b/tests/Makefile
index cfa3acb..113a39a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,6 +3,7 @@ include ${.CURDIR}/../Makefile.inc
 PROG=  pick-test

 SRCS+= pick-test.c
+SRCS+= ../compat-reallocarray.c

 OBJS=  ${SRCS:.c=.o}
 DEPS=  ${SRCS:.c=.d}
@@ -42,7 +43,7 @@ TESTS+=       opt-s.t
 TESTS+=        opt-unknown.t
 TESTS+=        opt-x.t

-all: ${TESTS:.t=.out}
+all: ${PROG} ${TESTS:.t=.out}

 .SUFFIXES: .t .out

@@ -51,7 +52,7 @@ all: ${TESTS:.t=.out}

 ${PROG}: ${OBJS}
-       ${CC} ${DEBUG} -o ${PROG} ${OBJS} ${LDFLAGS}
+       ${CC} ${DEBUG} -o ${PROG} ${OBJS}

 clean:
        rm -f ${DEPS} ${OBJS} ${PROG}

... so make test works on Debian 9.7, Mac OS X and NetBSD 7.1.2 ... (LDFLAGS removed: no need to link pick-test with curses)

teoljungberg commented 5 years ago

I'd look into using https://github.com/koalaman/shellcheck or something similar to enforce /bin/sh compatibility.

mptre commented 5 years ago

Thanks for the feedback, just pushed a few fixes:

ghost commented 5 years ago

Tested and works ...

mptre commented 5 years ago

Tested and works ...

Thanks for testing, merged now.

mptre commented 5 years ago

For a small project like pick dependencies in the Makefile would IMHO be enough ...

It might look a bit excessive. But, it's a matter of correctness. If a system header changes, although highly unlikely, we want to rebuild pick.