1
\$\begingroup\$

Any suggestions on how to improve the Makefile, e.g. how to best replace the multiple uses of example1/example2, ex1_src/ex2_src, ex1_obj/ex2_obj with static pattern rules?

build := ./build
targets := $(build)/example1 $(build)/example2
src := ./src

srcfiles := $(shell find $(src) \( -name "*.cpp" -or -name "*.c" \) ! -path '*/examples/*')
objects  := $(srcfiles:%=$(build)/%.o)

ex1_src := $(shell find $(src) -name example1.cpp)
ex1_obj := $(ex1_src:%=$(build)/%.o)

ex2_src := $(shell find $(src) -name example2.cpp)
ex2_obj := $(ex2_src:%=$(build)/%.o)

all_obj := $(objects) $(ex1_obj) $(ex2_obj)
depends := $(all_obj:%.o=%.d)
incdirs := $(shell find $(src) -type d ! -path '*/examples')
includes := $(addprefix -I,$(incdirs))

CXX := g++
CXXFLAGS := -std=c++17 $(includes) -Wall -MMD -MP
LDFLAGS := -lpthread

all: $(targets)

$(build)/example1: $(objects) $(ex1_obj)
$(build)/example2: $(objects) $(ex2_obj)

$(targets):
    $(CXX) $(LDFLAGS) $^ -o $@

$(build)/%.cpp.o: %.cpp
    @mkdir -p $(dir $@)
    $(CXX) $(CXXFLAGS) -c $< -o $@

.PHONY: clean
clean:
    rm -rf $(build)

-include $(depends)
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Incorporating advice from an answer into the question violates the question-and-answer nature of this site. You could post improved code as a new question, as an answer, or as a link to an external site - as described in I improved my code based on the reviews. What next?. I have rolled back the edit, so the answers make sense again. \$\endgroup\$ Feb 7 '21 at 9:40
1
\$\begingroup\$

It's usually best to have the Makefile in the same directory as the build products, or at least have that as the working directory. Then there's no need to re-write the built-in rules for compiling and linking. Use VPATH to ensure the source files can be found.

LDFLAGS := -lpthread

That should go into LIBS (which gets expanded later in the command line), so that it gets used as needed. LDFLAGS is for flags such as -L which need expanding further to the left.

I'm not convinced that example1 and example2 are great names for your programs - surely you can think of something more descriptive and memorable for your users?

ex1_src := $(shell find $(src) -name example1.cpp)

Do you really need to invoke a find there? I would expect there to be few enough matches that you could simply list them, and update the list when you add a new one. I'd do the same for srcfiles too - or create a library, so that the resultant programs only include the objects they need.

I think generally there's too much use of shells here, despite them all being in := assignments. You want makefile parsing to be fast, so you can see all your unit test results as quickly as possible.

So I'd write

example1: example1.o object_a.o object_b.o object_c.o
example1: LINK.o = $(LINK.cc)

The target-specific LINK.o is necessary so that the C++ linker is used. It's cleaner and more portable than adding the C++ runtime library to LIBS.

You're missing some very important CXXFLAGS:

CXXFLAGS += -Wall -Wextra

I'd likely add a few more:

CXXFLAGS += -Wpedantic -Warray-bounds -Weffc++
CXXFLAGS += -Werror

Good use of .PHONY - that's often overlooked.

You should also have .DELETE_ON_ERROR: so that interrupted builds don't cause problems.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks! +1 Only one concern, I dont like to have all objects and deps in the top level build folder. Any good advice for that? \$\endgroup\$
    – linkjumper
    Feb 6 '21 at 20:22
  • 1
    \$\begingroup\$ I don't see why not - it's the way Make is designed to work. It's nice to have all the products in one place - then you can clean just by deleting the build directory. If you want to work against Make, then that's your choice of course. But I find it easier to let the built-in rules carry the load, and have a makefile that follows convention so that anyone else can follow it. \$\endgroup\$ Feb 6 '21 at 21:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.