make all fails silently if database.h fails to build

[proppy]

assuming csh is not present:

# ./configure
...
checking for csh... no
...

make all will fails silently and exit with 0:

# make all
make all
--- errors and warnings logged in file make.log
--- making header file database/database.h
# echo $?
echo $?
0

while make.log contains the appropriate error message:

--- making header file database/database.h
./scripts/makedbh database/database.h.in database/database.h
/bin/bash: ./scripts/makedbh: /bin/csh: bad interpreter: No such file or directory
make[1]: *** [Makefile:48: database/database.h] Error 126
make[1]: Leaving directory '/tmp/1/magic'

[proppy]

this is likely caused by: https://github.com/RTimothyEdwards/magic/blob/master/Makefile#L22

which invert grep for specific keywords like Stop in the log while in that case make outputs Error.

[RTimothyEdwards]

But if the lack of "csh" now prevents "configure" from completing, then it doesn't matter, does it?

[proppy]

I'd argue that it would be best for make to always surface build error messages (and exit with the corresponding exit code) as it makes it challenging to debug in CI environment (where the capability to easily be able to access the filesystem to inspect make.log should not be taken for granted)

[RTimothyEdwards]

It's very likely that the reason that I did the grep for "Error" was because the compile was not stopping on an error, which made it hard to find the needle in the output haystack. I'd suppose that with the change to halt the build on an error, there's no longer any particular need to filter the terminal output.

[proppy]

+1, something like:

diff --git a/Makefile b/Makefile
index e1d61fb..dc0efc4 100644
--- a/Makefile
+++ b/Makefile
@@ -18,12 +18,10 @@ INSTALL_CAD_DIRS = windows doc ${TECH}
 all:        $(ALL_TARGET)
 
 standard:
-        @echo --- errors and warnings logged in file make.log
-        @${MAKE} mains 2>&1 | tee -a make.log | egrep -i "(.c:|Stop.|---)"
+        @${MAKE} mains
 
 tcl:
-        @echo --- errors and warnings logged in file make.log
-        @${MAKE} tcllibrary 2>&1 | tee -a make.log | egrep -i "(.c:|Stop.|---)"
+        @${MAKE} tcllibrary
 
 force: clean all

and

diff --git a/scripts/configure b/scripts/configure
index 549324e..5c91663 100755
--- a/scripts/configure
+++ b/scripts/configure
@@ -9504,9 +9504,6 @@ echo
 
 echo "Use 'make' to compile and 'make install' to install."
 echo
-echo "Errors may not be printed to stdout:  see files 'make.log' "
-echo "  and 'install.log' for complete error summary."
-echo
 echo "-----------------------------------------------------------"
 echo
 
diff --git a/scripts/configure.in b/scripts/configure.in
index 79b4cd8..1e5ace6 100644
--- a/scripts/configure.in
+++ b/scripts/configure.in
@@ -1910,9 +1910,6 @@ echo
 
 echo "Use 'make' to compile and 'make install' to install."
 echo
-echo "Errors may not be printed to stdout:  see files 'make.log' "
-echo "  and 'install.log' for complete error summary."
-echo
 echo "-----------------------------------------------------------"
 echo

should allow the error logs and status code to propagate nicely (in combination with #148 fixes).

[RTimothyEdwards]

I can see the need to remove the egrep statement, but is there any particular reason not to tee to a log file?

[proppy]

I think in that case you'd want to either:

• add -o pipefail to SHELL
• add || exit ${PIPESTATUS[0]}

to make sure you propagate the make exit code back to the parent make invocation; otherwise it will just propagate tee success.

[RTimothyEdwards]

Ah, right, the obscure pipefail thing.

[RTimothyEdwards]

@proppy : Given that SHELL is set to /bin/sh, which is not necessarily bash, is it better to insist that SHELL be bash, or just forget about logging the output?

[proppy]

maybe it's best to keep the Makefile simple and document in the README how to get it to log a file if needed with make 2>&1 | tee make.log, wdyt?

[RTimothyEdwards]

Works for me.

[RTimothyEdwards]

Should be all fixed now.

[RTimothyEdwards]

Note that the currently posted version on github will fail to compile with NULL graphics. Version 8.3.284 will fix this, as well as correctly declining to link "magicdnull" to any graphics libraries.