HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Generic Makefile handling unit testing and folder structure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
generichandlingmakefiletestingfolderstructureandunit

Problem

I'm going back to C++ and, even though I'm not a beginner in programming and OOP, I'm quite new in the C++ world and I've not mastered the art of Makefiles.

I'd like to know what you think about my third Makefile and the folder structure that I use. Don't hesitate to make any comments you think necessary.

Here is the folder structure of the "project":

out/ --- .exe and .zip
tmp/ --- *.o
lib/
| catch/ --- Unit testing framework
| | inc/
| | | catch.hpp
inc/ --- Headers
| Factorial.hpp
src/ --- Source files
| Factorial.cpp
| main.cpp
tst/ --- Tests specific source files
| FactorialTest.cpp
| MathTest.cpp
| mainTest.cpp
Makefile


It is designed to support unit testing (here using Catch) with the tst folder.

I didn't know how to use external libraries with C++, so I created a lib folder to put Catch in. I thought libraries could be downloaded from the configure.sh script in the future. Does it sound like a "professional" solution for larger projects?

The Makefile :

`CXX := g++
LD := g++

EXEC := factorial
FLAGS := -Wall
CXXFLAGS := $(FLAGS)
LDFLAGS := $(FLAGS)
INC := -I inc
SRC := $(wildcard src/*.cpp)
OBJ := $(SRC:src/%.cpp=tmp/%.o)

EXEC_TEST := test
FLAGS_TEST := $(FLAGS)
CXXFLAGS_TEST := $(CXXFLAGS)
LDFLAGS_TEST := $(LDFLAGS)
INC_TEST := $(INC) -I lib/catch/inc
SRC_TEST := $(wildcard tst/*.cpp)
OBJ_TEST := $(filter-out tmp/main.o, $(OBJ)) $(SRC_TEST:tst/%.cpp=tmp/%.o)

.SUFFIXES:

# --------------------------------------------------------------

.PHONY: all
all: out/$(EXEC)

out/$(EXEC): $(OBJ)
@$(LD) $(LDFLAGS) $^ -o $@ && echo "[OK] $@"

# --------------------------------------------------------------

.PHONY: test
test: out/$(EXEC_TEST)

out/$(EXEC_TEST): $(OBJ_TEST)
@$(LD) $(LDFLAGS_TEST) $^ -o $@ && echo "[OK] $@"

# --------------------------------------------------------------

tmp/%.o

Solution

I'll assume throughout this answer that you're targeting GNU Make. It's available on all targets that matter, and it simplifies your world no end if you can avoid pandering to the idiosyncrasies of the manifold implementations of Make supplied with operating systems.
Object files depend on headers

This is the biggest thing that I think could be improved; I think it's probably a whole question in itself. Your object files naturally depend on the corresponding C++ source files, but they also need rebuilding when their headers change. Consider including generated makefiles that tell make how the object files depend on the headers; for a suitable recipe, see the Stack Overflow question "Makefile, header dependencies", and for a longer exposition, read "Auto-Dependency Generation" by Paul D. Smith. I normally use something like

-include $(OBJS:.o=.d)

%.d: %.c
    $(CC) $(CFLAGS) -MM -MT $@ -MF $@ %%CODEBLOCK_0%%lt;


More warnings

It's good to see that you're using -Wall. I also recommend

CXXFLAGS += -Wextra -Wwrite-strings -Wno-parentheses
CXXFLAGS += -Wpedantic -Warray-bounds -Weffc++


The first line above always helps; the second does sometimes improve my code (you'll want to make use of -isystem to quieten your library includes, if you want to make the latter bearable).
Output directory

Rules of makefiles advises that

Life is simplest if the targets are built in the current working directory.

Use VPATH to locate the sources from the objects directory, not to locate the objects from the sources directory.

That's certainly something to consider, though it might already be too painful to change. I certainly prefer out-of-tree builds, so I can clean up with rm -rf build/ and be left with just my sources.
Hiding commands

I appreciate that you don't want a wall of text, but when you're debugging (or on an automated build host where you want a complete record of the build), then you might want to override this, and actually see which commands were run. You can make the behaviour adjustable, like this:

QUIET := @
ECHO := @echo
ifneq ($(QUIET),@)
ECHO := @true
endif


Use it for your commands:

clean clear:
    $(ECHO) "[CL]  out/"
    $(QUIET)$(RM) out/*
    $(ECHO) "[CL]  tmp/"
    $(QUIET)$(RM) tmp/*


I changed the order to print first, like Make does, according to the Principle of Least Surprise. It makes it more obvious what action was in progress when any error occurs.

Now, when you need to see the actual commands, you can make QUIET= to override the value and get the full output again. IIRC, the Linux kernel uses a similar system (toggled by a KConfig setting), as does Qt's qmake (toggled with CONFIG += silent in the .pro file).
Re-use Make's pre-defined commands

Instead of writing

%.o: %.cpp
    $(CXX) $(CXXFLAGS) -c %%CODEBLOCK_4%%lt; $(INC) -o $@

%: $(OBJ)
    $(LD) $(LDFLAGS_TEST) $^ -o $@


you can make the rules more like the built-in versions (that you can see with make --print-database):

%.o: %.cpp
    $(COMPILE.cpp) $(OUTPUT_OPTION) %%CODEBLOCK_5%%lt;

%: $(OBJ)
    $(LINK.cpp) $(OUTPUT_OPTION) $^ $(LOADLIBES) $(LDLIBS)


Adapted to use QUIET and ECHO, that's

%.o: %.cpp
    $(ECHO) "[OK]  $@"
    $(QUIET)$(COMPILE.cpp) $(OUTPUT_OPTION) %%CODEBLOCK_6%%lt;

%: $(OBJ)
    $(ECHO) "[OK]  $@"
    $(QUIET)$(LINK.cpp) $(OUTPUT_OPTION) $^ $(LOADLIBES) $(LDLIBS)


You'll need to ensure that your include path is in CXXFLAGS, and you might need target-dependent flags:

CXXFLAGS += $(INC)

tmp/%: CXXFLAGS=$(CXXFLAGS_TEST)


All makefiles should have...

.DELETE_ON_ERROR:


Without this, a failed command could leave a part-written output that a subsequent make believes is up-to-date.
Finally

Here's a tyni tyop:

.PHONY: clean, clear


Lose that , - you've said that clean, is a phony target, not clean as you intended.

Code Snippets

-include $(OBJS:.o=.d)

%.d: %.c
    $(CC) $(CFLAGS) -MM -MT $@ -MF $@ $<
CXXFLAGS += -Wextra -Wwrite-strings -Wno-parentheses
CXXFLAGS += -Wpedantic -Warray-bounds -Weffc++
QUIET := @
ECHO := @echo
ifneq ($(QUIET),@)
ECHO := @true
endif
clean clear:
    $(ECHO) "[CL]  out/"
    $(QUIET)$(RM) out/*
    $(ECHO) "[CL]  tmp/"
    $(QUIET)$(RM) tmp/*
%.o: %.cpp
    $(CXX) $(CXXFLAGS) -c $< $(INC) -o $@

%: $(OBJ)
    $(LD) $(LDFLAGS_TEST) $^ -o $@

Context

StackExchange Code Review Q#157780, answer score: 6

Revisions (0)

No revisions yet.