Makefile Makeover -- Almost Complete, Want Feedback

Question

I've been heavily refactoring my makefiles, with help from Beta, Paul R, and Sjoerd (thanks guys!).

Below is my STARTING product:

#Nice, wonderful makefile written by Jason
CC=g++
CFLAGS=-c -Wall
BASE_DIR:=.
SOURCE_DIR:=$(BASE_DIR)/source
BUILD_DIR:=$(BASE_DIR)/build
TEST_DIR:=$(BASE_DIR)/build/tests
MAKEFILE_DIR:=$(BASE_DIR)/makefiles
DATA_DIR:=$(BASE_DIR)/data
DATA_DIR_TESTS:=$(DATA_DIR)/tests
MOLECULE_UT_SOURCES :=  $(SOURCE_DIR)/molecule_test/main.cc \
    $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/molecule_manager.cpp \
    $(SOURCE_DIR)/molecule_manager_main.h \
    $(SOURCE_DIR)/molecule_manager_main.cpp \
    $(SOURCE_DIR)/molecule_reader.h \
    $(SOURCE_DIR)/molecule_reader.cpp \
    $(SOURCE_DIR)/molecule_reader_psf_pdb.h \
    $(SOURCE_DIR)/molecule_reader_psf_pdb.cpp \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.h \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.cpp \
    $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parameter_manager.cpp \
    $(SOURCE_DIR)/parser.h \
    $(SOURCE_DIR)/parser.cpp \
    $(SOURCE_DIR)/common.h
MOLECULE_UT_DATA := \
    $(DATA_DIR_TESTS)/molecule_test/par_oxalate_and_friends.inp \
    $(DATA_DIR_TESTS)/molecule_test/dicarboxy-octane_4.pdb \
    $(DATA_DIR_TESTS)/molecule_test/dicarboxy-octane_4.psf
PARAM_UT_SOURCES :=  $(SOURCE_DIR)/parameter_test/main.cc \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.h \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.cpp \
    $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parameter_manager.cpp \
    $(SOURCE_DIR)/parser.h \
    $(SOURCE_DIR)/parser.cpp \
    $(SOURCE_DIR)/common.h
PARAM_UT_DATA := $(DATA_DIR_TESTS)/molecule_test/par_oxalate_and_friends.inp

molecule_test : molecule_test_prepare_sources molecule_test_prepare_makefiles \
    molecule_test_prepare_data_files
    @$(shell cd $(TEST_DIR)/molecule_unit_test/; \
    make ./bin/molecule_test)

molecule_test_prepare_sources: molecule_test_dir
    @echo Copying sources...
    @cp --preserve $(MOLECULE_UT_SOURCES) \
    $(TEST_DIR)/molecule_unit_test/source

molecule_test_prepare_makefiles: $(MAKEFILE_DIR)/Makefile.molecule_test
    @cp  --preserve $(MAKEFILE_DIR)/Makefile.molecule_test \
    $(TEST_DIR)/molecule_unit_test/Makefile

molecule_test_prepare_data_files:
    cp --preserve $(MOLECULE_UT_DATA) $(TEST_DIR)/molecule_unit_test/bin/

molecule_test_dir:
    @if test -d $(BUILD_DIR); then \
        echo Build exists...; \
        else \
        echo Build directory does not exist, making build dir...; \
    mkdir $(BUILD_DIR); \
        fi
    @if test -d $(TEST_DIR); then \
        echo Tests exists...; \
        else \
        echo Tests directory does not exist, making tests dir...; \
    mkdir $(TEST_DIR); \
        fi
    @if test -d $(TEST_DIR)/molecule_unit_test; then \
        echo Molecule unit test directory exists...; \
        else \
        echo Molecule unit test directory does \
        not exist, making build dir...; \
        mkdir $(TEST_DIR)/molecule_unit_test; \
        fi
    @if test -d $(TEST_DIR)/molecule_unit_test/source; then \
        echo Molecule unit test source directory exists...; \
        else \
        echo Molecule unit test source directory does \
        not exist, making build dir...; \
        mkdir $(TEST_DIR)/molecule_unit_test/source; \
        fi
    @if test -d $(TEST_DIR)/molecule_unit_test/obj; then \
        echo Molecule unit test object directory exists...; \
        else \
        echo Molecule unit test object directory does \
        not exist, making object dir...; \
        mkdir $(TEST_DIR)/molecule_unit_test/obj; \
        fi
    @if test -d $(TEST_DIR)/molecule_unit_test/bin; then \
        echo Molecule unit test executable directory exists...; \
        else \
        echo Molecule unit test executable directory does \
        not exist, making executable dir...; \
        mkdir $(TEST_DIR)/molecule_unit_test/bin; \
        fi

param_test : param_test_prepare_sources param_test_prepare_makefiles \
    param_test_prepare_data_files
    @$(shell cd $(TEST_DIR)/param_unit_test/; \
    make ./bin/param_test)

param_test_prepare_sources: param_test_dir
    @echo Copying sources...
    @cp --preserve $(PARAM_UT_SOURCES) $(TEST_DIR)/param_unit_test/source

param_test_prepare_makefiles: $(MAKEFILE_DIR)/Makefile.param_test
    @cp  --preserve $(MAKEFILE_DIR)/Makefile.param_test \
    $(TEST_DIR)/param_unit_test/Makefile

param_test_prepare_data_files:
    cp --preserve $(PARAM_UT_DATA) $(TEST_DIR)/param_unit_test/bin/

param_test_dir:
    @if test -d $(BUILD_DIR); then \
        echo Build exists...; \
        else \
        echo Build directory does not exist, making build dir...; \
    mkdir $(BUILD_DIR); \
        fi
    @if test -d $(TEST_DIR); then \
        echo Tests exists...; \
        else \
        echo Tests directory does not exist, making tests dir...; \
    mkdir $(TEST_DIR); \
        fi
    @if test -d $(TEST_DIR)/param_unit_test; then \
        echo Param unit test directory exists...; \
        else \
        echo Param unit test directory does \
        not exist, making build dir...; \
        mkdir $(TEST_DIR)/param_unit_test; \
        fi
    @if test -d $(TEST_DIR)/param_unit_test/source; then \
        echo Param unit test source directory exists...; \
        else \
        echo Param unit test source directory does \
        not exist, making build dir...; \
        mkdir $(TEST_DIR)/param_unit_test/source; \
        fi
    @if test -d $(TEST_DIR)/param_unit_test/obj; then \
        echo Param unit test object directory exists...; \
        else \
        echo Param unit test object directory does \
        not exist, making object dir...; \
        mkdir $(TEST_DIR)/param_unit_test/obj; \
        fi
    @if test -d $(TEST_DIR)/param_unit_test/bin; then \
        echo Param unit test executable directory exists...; \
        else \
        echo Param unit test executable directory does \
        not exist, making executable dir...; \
        mkdir $(TEST_DIR)/param_unit_test/bin; \
        fi

...and the sub makefile:

#Nice, wonderful makefile written by Jason
CC=g++
CFLAGS=-c -Wall
SOURCE_DIR:=./source
OBJ_DIR:=./obj
EXE_DIR:=./bin

$(EXE_DIR)/molecule_test : $(OBJ_DIR)/main.o \
    $(OBJ_DIR)/parameter_manager_lj_molecule.o \
    $(OBJ_DIR)/parameter_manager.o $(OBJ_DIR)/parser.o \
    $(OBJ_DIR)/molecule_manager.o $(OBJ_DIR)/molecule_manager_main.o \
    $(OBJ_DIR)/molecule_reader.o \
    $(OBJ_DIR)/molecule_reader_psf_pdb.o
    @$(CC) $(OBJ_DIR)/main.o $(OBJ_DIR)/parameter_manager.o \
    $(OBJ_DIR)/parser.o $(OBJ_DIR)/parameter_manager_lj_molecule.o \
    $(OBJ_DIR)/molecule_manager.o $(OBJ_DIR)/molecule_manager_main.o \
    $(OBJ_DIR)/molecule_reader.o \
    $(OBJ_DIR)/molecule_reader_psf_pdb.o \
    -o molecule_test
    @mv molecule_test $(EXE_DIR)/ 

$(OBJ_DIR)/main.o: $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.h \
    $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/molecule_manager_main.h \
    $(SOURCE_DIR)/molecule_reader.h \
    $(SOURCE_DIR)/molecule_reader_psf_pdb.h \
    $(SOURCE_DIR)/common.h $(SOURCE_DIR)/main.cc
    $(CC) $(CFLAGS) $(SOURCE_DIR)/main.cc
    @mv main.o $(OBJ_DIR)/

$(OBJ_DIR)/molecule_reader.o: $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.h \
    $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/molecule_manager_main.h \
    $(SOURCE_DIR)/molecule_reader.h \
    $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/molecule_reader.cpp
    @mv molecule_reader.o $(OBJ_DIR)/

$(OBJ_DIR)/molecule_reader_psf_pdb.o: $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parameter_manager_lj_molecule.h \
    $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/molecule_manager_main.h \
    $(SOURCE_DIR)/molecule_reader.h \
    $(SOURCE_DIR)/molecule_reader_psf_pdb.h \
    $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/molecule_reader_psf_pdb.cpp
    @mv molecule_reader_psf_pdb.o $(OBJ_DIR)/

$(OBJ_DIR)/molecule_manager.o: $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/molecule_manager.cpp
    @mv molecule_manager.o $(OBJ_DIR)/

$(OBJ_DIR)/molecule_manager_main.o: $(SOURCE_DIR)/molecule_manager.h \
    $(SOURCE_DIR)/molecule_manager_main.h \
    $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/molecule_manager_main.cpp
    @mv molecule_manager_main.o $(OBJ_DIR)/

$(OBJ_DIR)/parameter_manager_lj_molecule.o: $(SOURCE_DIR)/common.h \
    $(SOURCE_DIR)/parameter_manager.h \
    $(SOURCE_DIR)/parser.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/parameter_manager_lj_molecule.cpp
    @mv parameter_manager_lj_molecule.o $(OBJ_DIR)/

$(OBJ_DIR)/parameter_manager.o: $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/parameter_manager.cpp
    @mv parameter_manager.o $(OBJ_DIR)/

$(OBJ_DIR)/parser.o: $(SOURCE_DIR)/parser.h
    @$(CC) $(CFLAGS) $(SOURCE_DIR)/parser.cpp
    @mv parser.o $(OBJ_DIR)/

$(OBJ_DIR)/common.o: $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(SOURCE_DIR)/common.h
    mv common.h.gch $(OBJ_DIR)/

With some help from the above users and figuring out a few nifty tricks on my own as well (like use of wildcards), I've refactored the makefiles heavily, plus added comments for posterity.

Here's the top level result:

####################################################
##  -------------------------------
##  - Monte Carlo Source Makefile -
##  -------------------------------
##
## Author:  Jason R. Mick
## Date:    July 7, 2010
## Company: Wayne State University
##
## CHANGE LOG
## Author        Date          Description
##
##
##
####################################################

#################################
# These lines set up some basic vars
# such as compiler, flags, and dirs.
#################################
CC=g++
CFLAGS=-c -Wall
BASE_DIR:=.
SOURCE_DIR:=$(BASE_DIR)/source
BUILD_DIR:=$(BASE_DIR)/build
TEST_DIR:=$(BASE_DIR)/build/tests
MAKEFILE_DIR:=$(BASE_DIR)/makefiles
DATA_DIR:=$(BASE_DIR)/data
DATA_DIR_TESTS:=$(DATA_DIR)/tests

#################################
# Note use of wildcards to catch *.h and *.cpp files and all the sub_classes
# ... for future unit tests/classes, follow this approach, please
#################################
MOLECULE_UT_SOURCES :=  $(SOURCE_DIR)/molecule_test/main.cc \
    $(SOURCE_DIR)/molecule_manager* \
    $(SOURCE_DIR)/molecule_reader* \
    $(SOURCE_DIR)/parameter_manager* \
    $(SOURCE_DIR)/parser* \
    $(SOURCE_DIR)/common.h
MOLECULE_UT_DATA := \
    $(DATA_DIR_TESTS)/molecule_test/par_oxalate_and_friends.inp \
    $(DATA_DIR_TESTS)/molecule_test/dicarboxy-octane_4.*
PARAM_UT_SOURCES :=  $(SOURCE_DIR)/parameter_test/main.cc \
    $(SOURCE_DIR)/parameter_manager* \
    $(SOURCE_DIR)/parser* \
    $(SOURCE_DIR)/common.h
PARAM_UT_DATA := $(DATA_DIR_TESTS)/molecule_test/par_oxalate_and_friends.inp

#################################
# Use sub-make inside subdirectory on test target
# NOTE: @ silences output of this call...
#################################
molecule_test : molecule_test_prepare_sources molecule_test_prepare_makefiles \
    molecule_test_prepare_data_files
    @$(MAKE) -C $(TEST_DIR)/molecule_unit_test/ ./bin/molecule_test

#################################
# NOTE: this target uses --preserve to keep base source modification date
# to prevent unnecessary rebuilds
#################################
molecule_test_prepare_sources: molecule_test_dir
    @echo Copying sources...
    @cp --preserve $(MOLECULE_UT_SOURCES) \
    $(TEST_DIR)/molecule_unit_test/source

molecule_test_prepare_makefiles: $(MAKEFILE_DIR)/Makefile.molecule_test
    @cp  --preserve $(MAKEFILE_DIR)/Makefile.molecule_test \
    $(TEST_DIR)/molecule_unit_test/Makefile

molecule_test_prepare_data_files:
    @cp --preserve $(MOLECULE_UT_DATA) $(TEST_DIR)/molecule_unit_test/bin/

#################################
# NOTE: This mkdir command uses -p flag to create any missing parent dirs.
# If all dirs already exist, it also returns no error...
#################################
molecule_test_dir:
    mkdir -p $(TEST_DIR)/molecule_unit_test/source
    mkdir -p $(TEST_DIR)/molecule_unit_test/obj
    mkdir -p $(TEST_DIR)/molecule_unit_test/bin

param_test : param_test_prepare_sources param_test_prepare_makefiles \
    param_test_prepare_data_files
    @$(MAKE) -C $(TEST_DIR)/param_unit_test/ ./bin/param_test

param_test_prepare_sources: param_test_dir
    @echo Copying sources...
    @cp --preserve $(PARAM_UT_SOURCES) $(TEST_DIR)/param_unit_test/source

param_test_prepare_makefiles: $(MAKEFILE_DIR)/Makefile.param_test
    @cp  --preserve $(MAKEFILE_DIR)/Makefile.param_test \
    $(TEST_DIR)/param_unit_test/Makefile

param_test_prepare_data_files:
    @cp --preserve $(PARAM_UT_DATA) $(TEST_DIR)/param_unit_test/bin/

param_test_dir:
    mkdir -p $(TEST_DIR)/param_unit_test/source
    mkdir -p $(TEST_DIR)/param_unit_test/obj
    mkdir -p $(TEST_DIR)/param_unit_test/bin

Here's the sub-makefile result:

####################################################
##  -------------------------------
##  - Monte Carlo Source Submake  -
##  -------------------------------
##
## Author:  Jason R. Mick
## Date:    July 7, 2010
## Company: Wayne State University
##
## CHANGE LOG
## Author        Date          Description
##
##
##
####################################################

################################
# These lines set up some basic vars
# such as compiler, flags, and dirs.
################################
CC=g++
CFLAGS=-c -Wall
SOURCE_DIR:=./source
INCDIRS := -I$(SOURCE_DIR)
OBJ_DIR:=./obj
EXE_DIR:=./bin

################################
#This line tells make what directories to search in for rules...
################################
VPATH = $(SOURCE_DIR)

################################
# INFO on the "magic" here:
#$^ is all the prerequisite (.o files), $@ is target, and % is wildcard
################################
$(EXE_DIR)/molecule_test : $(OBJ_DIR)/main.o \
    $(OBJ_DIR)/parameter_manager_lj_molecule.o \
    $(OBJ_DIR)/parameter_manager.o $(OBJ_DIR)/parser.o \
    $(OBJ_DIR)/molecule_manager.o $(OBJ_DIR)/molecule_manager_main.o \
    $(OBJ_DIR)/molecule_reader.o \
    $(OBJ_DIR)/molecule_reader_psf_pdb.o
    $(CC) $^ -o $@

################################
# These are extra includes for the general
# rule at the end....
################################
$(OBJ_DIR)/main.o $(OBJ_DIR)/molecule_reader.o \
  $(OBJ_DIR)/molecule_reader_psf_pdb.o: \
  molecule_manager.h \
  molecule_manager_main.h \
  parameter_manager.h \
  parameter_manager_lj_molecule.h

$(OBJ_DIR)/molecule_manager_main.o: molecule_manager.h

$(OBJ_DIR)/parameter_manager_lj_molecule.o: parser.h

$(OBJ_DIR)/molecule_reader_psf_pdb.o: molecule_reader.h

################################
# Special rule for main object
################################
$(OBJ_DIR)/main.o: $(SOURCE_DIR)/main.cc \
  molecule_reader.h \
  molecule_reader_psf_pdb.h common.h
    $(CC) $(CFLAGS) $(INCDIRS) $< -o $@

################################
# The GENERAL RULE for objects...
# INFO on the "magic" here:
#$< is the first prerequisite (.cpp file), $@ is target, and % is wildcard
################################
$(OBJ_DIR)/%.o: $(SOURCE_DIR)/%.cpp $(SOURCE_DIR)/%.h $(SOURCE_DIR)/common.h
    $(CC) $(CFLAGS) $(INCDIRS) $< -o $@

... basically I'm pretty satisfied, everything is working, clean, and well documented, but I wanted to see if anyone else has suggestions of things I should change for "best practice" etc. I'm trying to learn as much as I can!! Thanks in advance!!

Cheers, Jason

Answer 1

I would rethink calling make recursively, and instead @include the submakefiles in the main makefile. It is much easier to get the dependancies to work right, and it allows you to utilize multiple cores for building (using -j). Take a look at Recursive Make Considered Harmful for all the gory details.

Also, take a look at these questions:
What is your experience with non recursive make?
Recursive make friend or foe

Answer 2

(I won't enter into the recursive/non-recursive debate again. 'Nuff said.)

Why copy all of these files around? I understand that you don't want to hardcode a map of your directory structure in the submake, but you can save a lot of time and trouble by giving the submake access to the original files, either by symbolic link:

molecule_test_prepare_sources: molecule_test_dir
    @echo linking to sources...
    @ln -s $(SOURCE_DIR) $(TEST_DIR)/molecule_unit_test/source  

molecule_test_prepare_makefiles: $(MAKEFILE_DIR)/Makefile.molecule_test
    @ln -s $< $(TEST_DIR)/molecule_unit_test/Makefile

Or by passing a parameter to the submake:

param_test : param_test_prepare_sources param_test_prepare_makefiles \
  param_test_prepare_data_files
    @$(MAKE) -C $(TEST_DIR)/param_unit_test/ SOURCE_DIR=$(SOURCE_DIR) \
      ./bin/param_test # and then use SOURCE_DIR in the submake

The same goes for param_test. I think this actually lets you do away with MOLECULE_UT_SOURCES and PARAM_UT_SOURCES, and good riddance. (I left out the data directories, because I don't know what this code actually does-- maybe it needs a restriced diet, or modifies its input files or something.)

Finally as a matter of style, almost anywhere you see redundancy you can remove it and make the makefile easier to read. For instance,

$(EXE_DIR)/molecule_test : $(OBJ_DIR)/main.o \
  $(OBJ_DIR)/parameter_manager_lj_molecule.o \
  $(OBJ_DIR)/parameter_manager.o $(OBJ_DIR)/parser.o \
  $(OBJ_DIR)/molecule_manager.o $(OBJ_DIR)/molecule_manager_main.o \
  $(OBJ_DIR)/molecule_reader.o \
  $(OBJ_DIR)/molecule_reader_psf_pdb.o
  $(CC) $^ -o $@

can become

OBJECTS := main \
  parameter_manager_lj_molecule \
  parameter_manager parser \
  molecule_manager molecule_manager_main \
  molecule_reader \
  molecule_reader_psf_pdb

OBJECTS := $(patsubst %,$(OBJ_DIR)/%.o, $(OBJECTS)

$(EXE_DIR)/molecule_test : $(OBJECTS)
  $(CC) $^ -o $@