Makefile bug

[cjw85]

It looks to me like there is a typo in the Makefile. The lines:

CXX=g++
COMPILE.cpp= $(CXX) $(CPPFLAGS) $(SVNREV) $(DBDIR) $(TEST_UPDATE_DB) -c 

define the compiler and then agglomerate some flags into a variable. The COMPILE.cpp variable is not used again, which means the CPPFLAGS and others are not actually applied to any of the compilations.

[evolarjun]

Sorry for the long delay. I'm anything but a Makefile expert, and that line was added by a developer many years ago more familiar with Makefile syntax than I am.

It's my understanding that GNU Make will use COMPILE.cpp with an implicit rule for any files with the .cpp extension.

From https://www.gnu.org/software/make/manual/html_node/Catalogue-of-Rules.html I found:

make follows the convention that the rule to compile a .x source file uses the variable COMPILE.x. Similarly, the rule to produce an executable from a .x file uses LINK.x; and the rule to preprocess a .x file uses PREPROCESS.x

When I run gnu Make I do see the expected flags being passed to g++, so I think it's working:

$ make clean; make
Dectected architecture: Linux x86_64
rm -f *.o
rm -f amr_report amrfinder amrfinder_index amrfinder_update fasta_check fasta_extract fasta2parts gff_check dna_mutation
Dectected architecture: Linux x86_64
g++ -std=gnu++17 -pthread -malign-double -fno-math-errno -O3 -D'SVN_REV="3.11.23"'   -c  -o amr_report.o amr_report.cpp
...

Sometimes I get lost in all the implicit rules and shortcuts in GNU make, so please correct me if I'm wrong about something.

[cjw85]

Intriguing, I've not come across that before. I came across this issue when trying to build conda packages for aarch64 with conda build and finding that the flags weren't being applied. I patched the Makefile to work around things in that instance.

[evolarjun]

Looking back at the Makefile it is a little janky the way it's written and it could be made more explicit. It would probably be clearer to have the substitution explicitly shown e.g.:

fasta_check:    $(fasta_checkOBJS)
        $(COMPILE.cpp) -o $@ $(fasta_checkOBJS)

instead of

fasta_check:    $(fasta_checkOBJS)
        $(CXX) -o $@ $(fasta_checkOBJS)

To be honest Makefiles for GNU make are a little bit magic to me. I learned Makefile syntax on BSD and system V based unices a long time ago. I was late to the linux party. I remember them being simpler, but maybe that's just my addled memory. Since I don't know it well, all the implicit stuff that goes on makes it hard for me to predict what's going to happen so I generally leave things alone if they already work. I'll see if we can clean up the Makefile for a future release.

[evolarjun]

If it's actually causing compilation problems for you then I will definitely make it more explicit in the next release. Let me know if my suggestion to use:

fasta_check:    $(fasta_checkOBJS)
        $(COMPILE.cpp) -o $@ $(fasta_checkOBJS)

Will solve your problem.

[cjw85]

I just looked up the bioconda recipe: ncbi-amrfinderplus/build.sh

They seem to be working around flags not being applied correctly also by setting CXX to include lots of flags.

[evolarjun]

Could you attach your patch so I can see what you did?

We should be doing another release soon, so if I make any changes they may not make it into the next release, but I'd prefer to have a Makefile that is as compatible as possible.

[cjw85]

I'm on my phone; I can dig it out next week. I didn't do anything ground breaking, just explicitely included a CFLAGS in all the compilation commands in the obvious way.

[evolarjun]

I'll try some things. Thanks for your help!

[evolarjun]

This is an implicit rule for COMPILE.cpp, and making it explicit would require a fair amount of additional code or refactoring of the Makefile that I am loath to do. Unless modifying CXX is a problem as you reported is done by bioconda, I'm inclined not to do this. Please let us know if this is still causing you problems.