6
\$\begingroup\$

I must have re-written it from scratch: Makefile for a tiny C++ project, follow-up 1 Thanks for any further advice in advance!

This is an additional question to C++ code reading from a text file, storing value in int, and outputting properly rounded float, not a follow-up, since I ask to review my C++ Makefile, not my C++ source code.

I did not use many Makefiles over the time. Please excuse my inexperience. All recommendations will be upvoted and welcome. Thank you in advance.

PS: It should now be more or less portable across architectures, tested on Ubuntu Noble amd64 and Debian Bookworm arm64. I don't know if I have done a good job in this area, though. Also, tested with the BSD make, not only with the GNU make with plausible results.

# Use the latest version of the compiler on Debian 12 OS, arm64 platform:
CXX=g++-12

# Use the official standard;
# Optimize for speed;
# Stop on all warnings;
# Do not include debug symbols:
CXXFLAGS=-std=c++17 -O3 -Wall -Wextra -Werror -Wpedantic -pedantic-errors

# The name of the resulting executable file:
APP_NAME=fan-control

# The name of the C++ source code file:
SRC_FILE=$(APP_NAME).cpp

# Possibly not doing anything when using GNU make;
# You may see this for an explanation:
# https://www.gnu.org/software/make/manual/html_node/Special-Targets.html#index-POSIX_002dconforming-mode_002c-setting
.POSIX:

# Force rebuild work-around (--always-make)!
# You may see these for an explanation:
# https://stackoverflow.com/a/70199176/1997354
# https://stackoverflow.com/a/79300019/1997354
# (Shell alias make='make -B' has same effect.)
.force_rebuild:

.PHONY: .force_rebuild
.PHONY: distrib
.PHONY: run
.PHONY: clean
.PHONY: all

$(APP_NAME): .force_rebuild

$(APP_NAME): $(SRC_FILE)
    @printf '%s' 'Compiling program... '
    @$(CXX) $(CXXFLAGS) $(SRC_FILE) -o $(APP_NAME)
    @strip -s $(APP_NAME)
    @printf '%s\n' 'Done.'

distrib: $(SRC_FILE) Makefile
    @printf '%s' 'Compresssing source code... '
    @tar --posix -czf $(APP_NAME).tar.gz $(SRC_FILE) Makefile
    @printf '%s\n' 'Done.'

run: $(APP_NAME)
    @printf '%s\n' 'Running program... '
    @./$(APP_NAME)

clean:
    @rm -f -v $(APP_NAME)
    @rm -f -v $(APP_NAME).tar.gz

all: distrib run
\$\endgroup\$
3
  • \$\begingroup\$ My make files contain two other targets. debug test. The debug builds with -g to create a version with debug symbols. The test target builds with coverage enabled, then builds the unit tests (in the test directory), runs the tests to make sure that all work, then checks code coverage. \$\endgroup\$ Commented Dec 22, 2024 at 19:12
  • \$\begingroup\$ Also an install that copies anything needed to /usr/bin or /usr/lib etc. But you need to be root to run the install. \$\endgroup\$ Commented Dec 22, 2024 at 19:14
  • \$\begingroup\$ @LokiAstari I must have re-written it from scratch: Makefile for a tiny C++ project, follow-up 1 Thanks for any further advice in advance! \$\endgroup\$ Commented Dec 23, 2024 at 10:05

5 Answers 5

Reset to default
10
\$\begingroup\$

nit: spacing

CXX=g++-12

Bash requires squishing together FOO=value. But in a makefile we can make it slightly easier to read by adding whitespace.

CXX = g++-12

redundant comments

Elide redundant remarks, such as

# The name of the C++ source code file:

recipe errors

The .POSIX target is a little weird, and its comment is less helpful than it could be. I don't recall ever having seen it in production Makefiles. If that behavior mattered to me, I would probably define

SHELL = bash -e

to make it clear that the child is doing the familiar set -e so it will bail on first error. The OP code seems more cryptic than necessary.

I didn't notice any cmd1; cmd2 recipe statements, so I doubt it makes a difference. Plus it's probably better to author a recipe as cmd1 && cmd2 anyway.

--always-make

The .force_rebuild seems odd, and I would remove it from my copy of the makefile if I was working on this project. It makes this file more closely resemble an "always do steps one, two, & three in order" bash script, and less of a makefile check-the-timestamps DAG. But I guess it works for you, so go for it.

I don't recall that I have ever issued a make --always-make command. I would typically just make clean all, or perhaps rm a single product file to force rebuilding some things:

$ rm fan-control.o; make all

all

Putting the all: target at the end is odd. Make uses first target as default target. Conventionally we put all: near the top of the file, so that make is identical to make all.

silent running

    @printf '%s' 'Compiling program... '
    @$(CXX) $(CXXFLAGS) $(SRC_FILE) -o $(APP_NAME)

Using all those @-signs is odd. One of the nicest things about make is that there's no guesswork about what magical things it might be doing in the background, because you can see exactly what compiler command it's running now. Except that you hid that. For any "important" command which might take some time and which might possibly fail, you should definitely reveal the details. Now the strip -s maybe is "minor" and would be distracting visual noise, so retaining @strip -s might make sense. Me, personally? I would probably reveal that it ran, so some maintenance engineer recently recruited to the team wouldn't be left wondering why gdb found no symbols, but YMMV.

The clean: recipe should definitely reveal the rm command it just ran.

default format

distrib: $(SRC_FILE) Makefile
    ...
    @tar --posix -czf $(APP_NAME).tar.gz $(SRC_FILE) Makefile

That --format=pax (posix) option is the default. I wouldn't bother to specify it. I have never seen interoperability issues between e.g. my macOS bsdtar 3.5.1 and various linux revs such as GNU tar 1.34.

Folks maintaining this source file are manually keeping the deps in sync. We can DRY it up a bit. Prefer this command:

    tar -czf $(APP_NAME).tar.gz $^

Then adding a file to the prerequisites will automatically add it to the tar command.

Alternatively you might invent a new symbol:

SOURCES = $(SRC_FILE) Makefile
distrib: $(SOURCES)
    tar -czf $(APP_NAME).tar.gz $(SOURCES)

object files

It's a bit odd that your clean: recipe doesn't mention *.o. Most projects wind up with multiple source files that contribute to the final a.out. So if you have foo.cpp and bar.cpp, we would expect to see foo.o and bar.o lying around in your project directory. And then if we edit just one of those, there's no need to recompile the other one.

Given that you only have a single C++ source file so far, perhaps this detail has not yet arisen for you.

Anyway I do like that you have a run: target, and that all: invokes it. That makes the project's expectations obvious to any new engineer that joins the project team.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ $^ is not portable, and works only with GNU make, so I'm not using that point. (Tested with BSD make, it errored out.) \$\endgroup\$ Commented Dec 23, 2024 at 2:05
  • \$\begingroup\$ I have done a fair bit of work on {Open,Net}BSD and especially FreeBSD systems. People seldom write Bourne scripts (sh) any more, as bash is readily available and significantly more powerful. Similarly an app targeted for running on a BSD host is likely to have a gmake Makefile. My macOS /usr/bin/make is "GNU Make 3.81". The typical reason for sticking to the ancient Berkeley subset would be that we need to integrate with the OS make world system of *.mk files. \$\endgroup\$ Commented Dec 23, 2024 at 2:41
  • \$\begingroup\$ Well, "portability" is about minimizing development effort, so you can write once, test in one place, and reliably run in many places. For shell scripts, it tends to be much cheaper to ensure that e.g. /usr/local/bin/bash is a modern shell interpreter, and then use a shebang of #! /usr/bin/env bash. In the particular case of make, where posix just kind of stagnated more than a decade ago, often the most cost effective approach is to insist that gmake is available, or that make runs GNU make, and then that will let you simplify how you author and test any given Makefile. \$\endgroup\$ Commented Dec 23, 2024 at 2:50
  • \$\begingroup\$ I like most of that rm. Consider just putting *.o in the middle. An alternative is that, similar to defining SOURCES, many production makefiles will define OBJS which is essentially *.o, derived from a bunch of *.cpp files we know about. So $(OBJS) might also go in the middle. You have a small number of source files ATM (one file!), but over the lifetime of a project that number inevitably ratchets upward, so uniformly applying a common pattern like OBJS from the get-go tends to make sense. \$\endgroup\$ Commented Dec 23, 2024 at 5:00
  • \$\begingroup\$ I must have re-written it from scratch: Makefile for a tiny C++ project, follow-up 1 Thanks for any further advice in advance! \$\endgroup\$ Commented Dec 23, 2024 at 10:06
7
\$\begingroup\$

Generally, I would use as much of the make built-ins as possible. Specifically:

  • I would normally not specify the C++ (or C) compiler. The default for gnu make is g++; the system should be set up to use the most adequate one — for example, through a symbolic link from g++ to the g++ du jour. Changing that compiler, which may happen at any point, should not make it necessary to touch the Makefile. An explicit compiler specification would (only) make sense for projects which have special needs like depending on a specific compiler version, or a cross compiler.

  • There is no need to tell gnu make how to compile and link C++ files to produce an executable. This case is so common that it is built-in: The line $(CXX) $(CXXFLAGS) $(SRC_FILE) -o $(APP_NAME) is exactly the built-in rule. Making it explicit adds nothing except trouble for a reviewer, possibly yourself later on: "Since the author was explicit, they probably wanted to do something differently. Where is the intended deviation from the built-in rules?" It also introduces an opportunity for hard-to-find mistakes: Misspelling a built-in variable silently results in an empty string.

    The core of your Makefile, therefore, is a simple $(APP_NAME): $(SRC_FILE).

\$\endgroup\$
0
7
\$\begingroup\$

The biggest problem I see is that it doesn't handle dependency files.

You can ask gcc (and MSVC!) to output an auxiliary file containing a Make-style dependency specification listing all input files, including headers, that were processed.

During the first build, this is not yet relevant, because nothing has been built yet, but if you later on change a header file, you want to recompile only those objects that use that header. For this, you want to tell Make to include these auxiliary files; the dependencies listed then get added to the command from the pattern rule.

Including these files causes Make to want to build them as targets if they don't exist yet, so you need a rule to create them. The usual approach is to either create them as empty, or if you are generating headers as part of the build, list all of them as dependencies for every object, so the headers are initially generated before any objects are built.

\$\endgroup\$
4
  • \$\begingroup\$ Thank you, sorry I do not have time right today to study your comments, could you refactor the Makefile directly to address issues you spotted, please include this code into your answer, that would be very helpful, upvoted and merry christmas. Cheers \$\endgroup\$ Commented Dec 24, 2024 at 7:04
  • \$\begingroup\$ Also, this is dated, please see the follow-up instead. Have fun \$\endgroup\$ Commented Dec 24, 2024 at 7:06
  • \$\begingroup\$ Do you know MSVC command for this? \$\endgroup\$ Commented Dec 24, 2024 at 8:20
  • \$\begingroup\$ MSVC has a gcc compatibility mode, and I dimly remember being able to use -MM & co. \$\endgroup\$ Commented Dec 24, 2024 at 14:42
6
\$\begingroup\$

In addition to all the things that the other contributorors have mentioned: the first target of a Makefile is its default target, i.e. the one that gets built if you just say make on the command line. Therefore, I always structure my Makefiles like this:

# Copyright blabla and other legal blurbs.

default: all

all: distrib run

# ...

Rationale: It makes the default target explicit and prevents you from accidentally changing it by placing another rule above it.

I also think that it is more common to have just one .PHONY instruction in a Makefile:

.PHONY: all clean run \
    distrib .force_rebuild

I have also re-ordered the phony targets to what I am used to.

But there is nothing wrong with your version. Keep it, if you prefer it.

The .PHONY also tends to appear at the end of the Makefile, after the targets, not at the beginning.

But what is the purpose of the run target? If the executable image you are building is "whoa", how helpful is it to be able to say "make run" and see a message "Running program..." before your latest masterpiece breaks loose? What about just ./whoa? ;)

You can also try to have a look at the Makefiles generated by the GNU autotools. Download the latest version of the GNU coreutils (https://ftp.gnu.org/gnu/coreutils/), run ./configure to generate all Makefiles of the project.

Things to have a look at:

  1. See how they execute the Makefiles recursively.
  2. Silent vs. verbose: Run make. That is the default, "silent" version. Then start over with make clean and then make V=1 to see the verbose version. Look into the Makefile to see how it is done.
  3. Automatic dependency generation: The compiler option -MD automatically creates complete dependencies for all C/C++ files as Makefile snippets and -MP also creates phony targets to suppress warnings if headers are moved or deleted. You can see how the dependencies in src/.deps are created and updated in src/Makefile.

The Makefiles generated by the GNU autotools are pretty complex but a good reference. The documentation of GNU make will help you understand them.

\$\endgroup\$
4
  • \$\begingroup\$ @VlastimilBurián, I have added a section about GNU autotools that may help you to get more ideas for improvements. \$\endgroup\$ Commented Dec 23, 2024 at 7:37
  • \$\begingroup\$ I must have re-written it from scratch: Makefile for a tiny C++ project, follow-up 1 Thanks for any further advice in advance! \$\endgroup\$ Commented Dec 23, 2024 at 10:07
  • 1
    \$\begingroup\$ I personally prefer separate .PHONY targets, because it makes for easier diffs and easier conflict resolution when merging commits in source control. \$\endgroup\$ Commented Dec 23, 2024 at 11:00
  • 1
    \$\begingroup\$ @TobySpeight, fair point. My perception of phony targets predates git. Maybe it's time to change that convention. :) That's why I told the OP there's nothing wrong with their version. \$\endgroup\$ Commented Dec 23, 2024 at 22:49
1
\$\begingroup\$

This is better:

CXXFLAGS += -std=c++17 -O3 -Wall -Wextra -Werror -Wpedantic -pedantic-errors

.DEFAULT_GOAL := fan-control
all: run distrib

distrib: Makefile $(wildcard *.cpp *.c)
    tar -czf fan-control.tar.gz $^

run: fan-control
    ./fan-control

clean:
    rm -f fan-control *.o fan-control.tar.gz

.PHONY: distrib run clean all

make already knows how to build C++ programs, and will correctly use all the configuration in the environment (your original only uses CXXFLAGS which is incorrect, see manual: https://www.gnu.org/software/make/manual/html_node/Catalogue-of-Rules.html).

Consequently, CXXFLAGS becomes a += so your configuration gets appended to the environment instead of replacing it.

I'd reconsider if you actually want -Werror, since you'll most likely be chasing warnings for new compiler versions (or different compilers) for no reason. This is a useful debugging tool, and with += you can get it with CXXFLAGS=-Werror make.

This also fixes your Makefile's stripping of debug information. If you're doing it (which you shouldn't be unless you're also saving that debug information elsewhere), then you should do it correctly, in a separate installation step.

If your program grows to include more files (say, fan-control.cpp additiona.c additiona2.cpp), the .DEFAULT_GOAL line can be replaced with

fan-control: fan-control.o additiona.o additiona2.o

Consequently, CXXFLAGS becomes a += so your configuration gets appended to the environment instead of replacing it.

The CXX assignment is removed, because it's incorrect: the environment is already configured with the right C++ compiler.

I'd remove the distrib target outright and use your VCS's archiving functionality to reflect reality better; other answers provide comments on how to configure the tar invocation if you want to keep it. Notice the dependency list turning into the arguments in my invocation; this also fixes over-zealous rebuilding.

I don't really see a reason to have a run target. Except for if this were a test, but it isn't, so.

Whatever triggered -B is removed: this is counter-productive, make already tracks dependencies correctly. If nothing is to be done, then it should do nothing, and the user can request a full re-build with make -B.

This yields

$ make
c++ -g -std=c++17 -O3 -Wall -Wextra -Werror -Wpedantic -pedantic-errors  -fuse-ld=lld  fan-control.cpp   -o fan-control

or

$ make
c++ -g -std=c++17 -O3 -Wall -Wextra -Werror -Wpedantic -pedantic-errors   -c -o fan-control.o fan-control.cpp
cc -g   -c -o additiona.o additiona.c
c++ -g -std=c++17 -O3 -Wall -Wextra -Werror -Wpedantic -pedantic-errors   -c -o additiona2.o additiona2.cpp
cc -fuse-ld=lld  fan-control.o additiona.o additiona2.o   -o fan-control

and

$ make all
./fan-control
tar -czf fan-control.tar.gz Makefile additiona2.cpp fan-control.cpp additiona.c

For a bigger project you may want to also append -MD to CFLAGS/CXXFLAGS and add include $(wildcard *.d) to the end (and *.d to rm) to track dependencies across headers as well. But this is extraneous for single-file programs.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.