0
\$\begingroup\$

I decided to implement a Makefile for a personal project I'm working on (I've never created Makefile before) but I need to learn for an upcoming work assignment. The Makefile it's supposed to build the unit test binary(graphs_test/graphtest) for my graph API code. My directory structure for the project is as follows:

.
├── README.md
├── graphs
│   ├── DFS
│   │   ├── a.out
│   │   └── dfsDriver.cpp
│   ├── include
│   │   └── graph.h
│   └── source
│       └── graph.cpp
└── graphs_test
    ├── Makefile
    ├── graphtest
    ├── include
    │   └── graphTest.h
    ├── obj
    │   └── graphTest.o
    └── source
        └── graphTest.cpp
    

My Makefile is as follows:


CXX=g++

LDFLAGS=-pthread

LOCALIDIR=./include
_LOCALDEPS=graphTest.h 
LOCALDEPS=$(patsubst %, $(LOCALIDIR)/%,$(_LOCALDEPS))

IDIR=../graphs/include
_DEPS=graph.h
DEPS=$(patsubst %, $(IDIR)/%,$(_DEPS))

ODIR=obj
SOURCE=./source

_OBJ=graphTest.o
OBJ=$(patsubst %,$(ODIR)/%,$(_OBJ))
LIBS=/usr/local/lib/libgtest.a

CFLAGS=-I$(IDIR) -I$(LOCALIDIR) -Wall -pedantic -Werror

$(ODIR)/%.o: $(SOURCE)/%.cpp $(DEPS) $(LOCALDEPS) 
    @mkdir -p $(ODIR)
    $(CXX) -c -o $@ $< $(CFLAGS)
 
graphtest: $(OBJ)
    $(CXX) -o $@ $^ $(LIBS) $(LDFLAGS)

info: 
    @echo 'LOCALDEPS is: "$(LOCALDEPS)"'
    @echo 'DEPS is: "$(DEPS)"'
    @echo 'OBJ is: "$(OBJ)"'

clean:
    rm -rf $(ODIR)
    

This currently works. That is it builds my test binary. But I would appreciate any tips on how I can improve this Makefile or gotchas that can burn me later. Note that I haven't created the Makefile for my graph API code. So, I'm confused if I create rules to compile my graph API as well (I realize that the latter question might be out of scope for this site). I don't know if it should matter, but the graph.cpp file is empty since my API is parameterized.

\$\endgroup\$
6
  • \$\begingroup\$ ITs not clear what you are trying to build. What are the targets. Target: Libraries, Executables, Exported Header Files. \$\endgroup\$ Commented Jun 29, 2023 at 2:37
  • \$\begingroup\$ @MartinYork for now just an executable. \$\endgroup\$
    – KetDog
    Commented Jun 29, 2023 at 2:40
  • \$\begingroup\$ Do you care about debug and release builds. Not all systems (or compile flag combination) result in binaries that are compatible between release and debug (though this distinction is shrinking thankfully). Personally I actually build three versions: release / debug / coverage builds. Each build has its own set of unique flags. \$\endgroup\$ Commented Jun 29, 2023 at 2:40
  • \$\begingroup\$ What executable? The a.out in DFS? \$\endgroup\$ Commented Jun 29, 2023 at 2:40
  • \$\begingroup\$ @MartinYork graphTest in graphs_test directory. I don't care about release builds. \$\endgroup\$
    – KetDog
    Commented Jun 29, 2023 at 2:41

1 Answer 1

2
\$\begingroup\$

Traditionally.

The first target is all. This should be the first rule so that if no arguments are provided to make it builds all. Also here you are building the tests so I would expect all to simply build test.

 all:    test
 test:   graphtest
     echo "Please run the tests now they have been built"

I like that you are defining graphtest in terms of $(OBJ). But personally I would define $(OBJ) in terms of $(SRC). You can then use $(wildcard ...) to select the source files. This way you don't need to modify your Makefile when you add more source files.

 SDIR = source
 ODIR = obj

 SRC  = $(wildcard $(SDIR)/*.cpp)
 OBJ  = $(patsubst $(SDIR)/%.cpp, $(ODIR)/%.o, $(SRC)) 

This is the default definition:

CXX=g++    

I would not define it at all. By doing this you prevent the user of your makefile from overriding the default.

Note: running make can look like this:

CXX=clang make

The user defined CXX will now not take effect because you explicitly overwrite it.


This is fine for simple files:

LDFLAGS=-pthread

But as your files get more complex this can hide add libraries. So you should prefer to use +=

LDFLAGS  +=   -pthread

The builtin rule for compiling C++ is:

n.o is made automatically from n.cc, n.cpp, or n.C with a recipe of the form ‘$(CXX) $(CPPFLAGS) $(CXXFLAGS) -c’. We encourage you to use the suffix ‘.cc’ or ‘.cpp’ for C++ source files instead of ‘.C’ to better support case-insensitive file systems.

ie. You can think of the default rule as:

%.o: %.cpp
    $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $* $^

Even if you need to define your custom rule because of the source/object directory rules I would encourage you to use the same pattern:

$(ODIR)/%.cpp: $(SDIR)/%.cpp $(DEPS) $(LOCALDEPS)
    $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $* $<

Similarly for linking I would use:

executable: $(OBJ)
    $(CXX) $(LDFLAGS) $(OBJ) $(LOADLIBES) $(LDLIBS)

Personally I don't like you having a specific command in the build to build the object directory. I would make this a dependency:

$(ODIR)/%.o: $(SDIR)/%.cpp $(DEPS) $(LOCALDEPS) 
    @mkdir -p $(ODIR)              # Don't like this line.
    $(CXX) -c -o $@ $< $(CFLAGS)

I would have written it like this:

DIR_%:
     mkdir -p $*

$(ODIR)/%.o: DIR_$(ODIR) $(SDIR)/%.cpp $(DEPS) $(LOCALDEPS) 
    $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $* $<

But rather than do this for each file. I would do one other change and move this to the executable:

DIR_%:
     mkdir -p $*

graphtest:  DIR_$(ODIR) $(OBJ)
    $(CXX) -o $@ $(LDFLAGS) $(OBJ) $(LOADLIBES) $(LDLIBS)

$(ODIR)/%.o:  $(SDIR)/%.cpp $(DEPS) $(LOCALDEPS) 
    $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $* $<

You are manually building your dependencies in the makefile. This is hard work and error prone. You can get g++ to build the make depedencies fo you.

DDIR = depend
DEPS = $(patsubst $(SDIR)/%.cpp, $(DDIR)/%.d, $(SRC))

deps: DIR_$(DDIR) $(DEPS)

$(DDIR)/%.d: $(SDIR)/%.cpp
    # I may not have got this line exactly write.
    # I had to strip it out of my Makefile
    # Lookup documentation in g++ what -MF -MP -MM -MT do
    $(CXX) -MF "$@" -MM -MP -MT "$@" -MT"$(ODIR)/$(<:.cpp=.o)" $(CPPFLAGS) $(CXXFLAGS) $<

This will create a Makefile that you can include into your make file.

# Add this to the top of your make file:
-include $(DDIR)/*

You can now auto rebuild your dependencies with make dep.


This is untested. But what I would have as my Makefile (If I wrote them by hand).

LDFLAGS    +=  -pthread
CXXFLAGS   +=  -I include -I ../graphs/include
CXXFLAGS   +=  -Wall -Wextra -pedantic -Werror

SDIR        = source
ODIR        = objs
DDIR        = deps
SRC         = $(wildcard $(SDIR)/*.cpp)
OBJS        = $(patsubst $(SDIR)/%.cpp, $(ODIR)/%.o, $(SRC))
DEPS        = $(patsubst $(SDIR)/%.cpp, $(DDIR)/%.d, $(SRC))

-include $(DDIR)/*

TARGET      = graphtest

all:     test
test:    $(TARGET)
    @echo "Test exutable built. Please run it"
deps:    DIR_$(DDIR) $(DEPS)
    @echo "Dependencies updates"

graphtest:  DIR_$(ODIR) $(OBJS)
    $(CXX) -o $@ $(LDFLAGS) $(OBJS) $(LOADLIBES) $(LDLIBS)

$(ODIR)/%.o:  $(SDIR)/%.cpp $(DEPS) $(LOCALDEPS) 
    $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c $* $<

$(DDIR)/%.d: $(SDIR)/%.cpp
    $(CXX) -MF "$@" -MM -MP -MT "$(ODIR)/$(<:.cpp=.o)" $(CPPFLAGS) $(CXXFLAGS) $<

DIR_%:
    mkdir -p $*

info: 
    @echo 'LOCALDEPS is: "$(LOCALDEPS)"'
    @echo 'DEPS is: "$(DEPS)"'
    @echo 'OBJs is: "$(OBJS)"'

clean:
    rm -rf $(ODIR)
    

Though I personally put the object files in a separate directory (I do normally put the source and header files in the same directory as the Makefile.

The make file becomes a lot simpler if you have the source, header and object files all in the same directory as the Makefile as all the default rules hold.


Nowadays My makefile looks like this:

TARGET                      = graphtest.app
LDLIBS                      = -lpthread

include <Directory Where Master Make Lives>/Makefile

Then I have all the logic in the included Makefile. My master makefile will not work for you as it assumes things I do with my directory structure. But you can start putting common things into a master makefile so you don't need to type them out every time.

For some hints you can look at the one I have build over the years:

https://github.com/Loki-Astari/ThorMaker/blob/master/tools/Makefile

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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