treewide: reduce usage of shell in Makefiles by basilfx · Pull Request #22669 · RIOT-OS/RIOT

basilfx · 2026-09-06T22:01:31Z

Contribution description

Using $(shell ..) in Makefiles spawns a subprocess, which is less performant than built-in alternatives. Recalling #13174 from before, I let Claude analyze where this may improve on current master.

An overview:

$(shell basename $(CURDIR)) works the same as $(notdir $(CURDIR)) (unless $(CURDIR) equals /)
uppercase_and_underscore can be used instead of tr a-z- A-Z_
Use memoization for flags re-evaluated multiple times

The impact will be unnoticeable for a single build and will probably be in the noise when actual compilation takes the overhand. But (hopefully) it will be more noticeable for CI builds with warmer caches. I don't expect miracles, and it started as an experiment.

To give an idea of the improvement (I let Claude do the benchmarking) on my server (i5 14600K with 64 GiB of memory):

For make -j1 (rebuild, so warm cache):

App / board	base	head	saving	execve base → head
hello-world / nucleo-f767zi	2.30 s	0.66 s	−71%	651 → 207 (−68%)
gnrc networking / nucleo-f767zi	3.23 s	1.10 s	−66%	807 → 311 (−61%)
hello-world / samr21-xpro	1.19 s	0.65 s	−46%	314 → 209 (−33%)
hello-world / native	0.54 s	0.41 s	−23%	197 → 174 (−12%)

For make -j8 (rebuild, so warm cache):

App / board	base	head	saving
hello-world / nucleo-f767zi	2.50 s	1.72 s	−31%
gnrc networking / nucleo-f767zi	4.83 s	3.68 s	−24%
hello-world / samr21-xpro	1.52 s	1.26 s	−17%
hello-world / native	0.52 s	0.56 s	~0 (noise)

Testing procedure

This should not impact the builds. The end result should be the same.

To count subprocessed, one can use for example strace -f -qq -e trace=execve -o trace.txt make BOARD=nucleo-f767zi all. Then count the number of execve in the output file.

Issues/PRs references

None

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

Claude Code Opus 5 for coming up with alternatives (especially strings.mk) and performing benchmarks. I have read an understand every line changed, but I am not a Makefile export, and it can probably be better.

riot-ci · 2026-09-06T22:59:57Z

Murdock results

✔️ PASSED

369bf52 makefiles/toolchain: replace shell commands with functions

Success	Failures	Total	Runtime
172224	0	172224	03h:29m:03s

Artifacts

Documentation preview

basilfx · 2026-09-07T09:44:40Z

I wasn't expecting miracles, but I hoped for a bit more to be honest.

Full build took 03h 27m 11s. Its ~5 minutes faster, but I guess that is in the noise. Some older builds are equally 'fast' without the optimizations.

crasbe · 2026-09-07T12:03:11Z

I wasn't expecting miracles, but I hoped for a bit more to be honest.

I know that feeling 😅

basilfx · 2026-09-08T18:40:03Z

I also added a section on $(shell) usage in the documentation.

crasbe · 2026-09-09T08:01:05Z

+  $(error Too short to be a valid STM32 CPU_MODEL: '$(CPU_MODEL)')
+endif
+
+# 'MP' is the only type that is spelled with two characters


That is not true, there are also WB and WL.

It wasn't captured by the original regex. I'll look into it.

crasbe · 2026-09-09T08:08:48Z

+    $(info +++ FIXIT: Makefile: Add USEMODULE += unicoap_driver_dtls for CoAP over DTLS driver)
+    $(info +++ FIXIT: Makefile: Add USEMODULE += unicoap_driver_udp for CoAP over UDP driver)
+    $(info +++ FIXIT: Makefile: Add USEMODULE += unicoap_driver_rfc7252_pdu for CoAP over UDP/DTLS PDU parser only)
+    $(info +++ FIXIT: Makefile: Add USEMODULE += unicoap_driver_slipmux for CoAP over Slipmux driver)


This is not a valid change, echoing to stderr is intentional here since some scripts of the build system interpret the output on stdout.

Ideally this should use $(call echoinfo,("+++ FIXIT...")), see https://doc.riot-os.org/build-system/build_system_basics/#printing-infos-warnings-and-error-messages

These functions do use a shell call, but printing to stderr is not supported with $(info) and printing colors with $(info) is not necessarily defined, although some code does that.

crasbe · 2026-09-09T08:10:30Z

-# Check whether `which` is installed
-ifeq ($(shell command -v which 2>/dev/null),)
-  $(error 'which' is required but not found in PATH.)
-endif


But is there a big performance penalty for keeping the check? Perhaps @AnnsAnns can elaborate why he added the check?

AnnsAnns · 2026-09-09T08:22:07Z

Is the github PR review broken again, I cant directly comment for some reason?

Anyways, RE which check, if I remember correctly this check exists because we use which to check for stuff but when which is not installed it broke those checks in UB ways that caused the build system to create incorrect and misleading errors

kfessel · 2026-09-11T15:01:35Z

+Before using a `$(shell ...)` call, check whether one of the following
+alternatives already covers the case:
+
+- Use `$(notdir ...)` instead of `$(shell basename ...)` to strip the


Suggested change

- Use `$(notdir ...)` instead of `$(shell basename ...)` to strip the

- Use `$(basename $(notdir ...))` instead of `$(shell basename ...)` to strip the

$(basename) strips the suffix as shell basename does

kfessel · 2026-09-11T15:08:25Z

+split_chars = $(strip $(subst 0, 0,$(subst 1, 1,$(subst 2, 2,$(subst 3, 3,$(subst 4, 4,$(subst 5, 5,$(subst 6, 6,$(subst 7, 7,$(subst 8, 8,$(subst 9, 9,$(subst a, a,$(subst b, b,$(subst c, c,$(subst d, d,$(subst e, e,$(subst f, f,$(subst g, g,$(subst h, h,$(subst i, i,$(subst j, j,$(subst k, k,$(subst l, l,$(subst m, m,$(subst n, n,$(subst o, o,$(subst p, p,$(subst q, q,$(subst r, r,$(subst s, s,$(subst t, t,$(subst u, u,$(subst v, v,$(subst w, w,$(subst x, x,$(subst y, y,$(subst z, z,$(subst A, A,$(subst B, B,$(subst C, C,$(subst D, D,$(subst E, E,$(subst F, F,$(subst G, G,$(subst H, H,$(subst I, I,$(subst J, J,$(subst K, K,$(subst L, L,$(subst M, M,$(subst N, N,$(subst O, O,$(subst P, P,$(subst Q, Q,$(subst R, R,$(subst S, S,$(subst T, T,$(subst U, U,$(subst V, V,$(subst W, W,$(subst X, X,$(subst Y, Y,$(subst Z, Z,$(subst _, _,$(subst -, -,$1)))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))
+
+# Returns the first $1 characters of $2, like 'cut -c -$1' does.
+first_chars = $(subst $(space),,$(wordlist 1,$1,$(call split_chars,$2)))


chars_upto instead of first_chars

might fir better with chars_from

Suggested change

first_chars = $(subst $(space),,$(wordlist 1,$1,$(call split_chars,$2)))

chars_upto = $(subst $(space),,$(wordlist 1,$1,$(call split_chars,$2)))

or first_chars and last_chars

basilfx added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 6, 2026

basilfx requested review from MichelRottleuthner, aabadie, benpicco, dylad, keestux, leandrolanzieri, nandojve and vincent-d as code owners September 6, 2026 22:01

basilfx added the AI: Co-Authored PR/Issue relies on AI. Documentation / Code was partly generated by AI. label Sep 6, 2026

crasbe reviewed Sep 7, 2026

View reviewed changes

Comment thread makefiles/toolchain/gnu.inc.mk Outdated

Comment thread makefiles/toolchain/gnu.inc.mk Outdated

basilfx removed the CI: full build disable CI build filter label Sep 7, 2026

basilfx force-pushed the feature/remove_makefile_shell branch from ccc1ea2 to 6fe1bce Compare September 7, 2026 21:19

basilfx requested a review from maribu as a code owner September 7, 2026 21:19

basilfx added 9 commits September 8, 2026 20:14

boards/e180-zg120b-tb: replace which with command -v

929d17c

pkg: replace which with command -v

d08f976

fixup! cpu: replace shell commands with functions

93bd123

sys: replace shell commands with functions

c3b37a2

fixup! makefiles: replace shell commands with functions

e59597f

tests/net/l2util: replace shell commands with functions

be5d27e

dist/tools/doxygen: replace shell commands with functions

48eed03

docs/guides: document the use of shell in Makefiles

f2470cf

makefiles/toolchain: replace shell commands with functions

369bf52

basilfx force-pushed the feature/remove_makefile_shell branch from 2e62b30 to 369bf52 Compare September 8, 2026 18:36

basilfx requested review from AnnsAnns, gschorcht and miri64 as code owners September 8, 2026 18:36

crasbe reviewed Sep 9, 2026

View reviewed changes

crasbe mentioned this pull request Sep 9, 2026

makefiles/tests: remove unused helper in tests.inc.mk #22680

Merged

kfessel reviewed Sep 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

treewide: reduce usage of shell in Makefiles - #22669

treewide: reduce usage of shell in Makefiles#22669
basilfx wants to merge 20 commits into
RIOT-OS:masterfrom
basilfx:feature/remove_makefile_shell

basilfx commented Sep 6, 2026 •

edited

Loading

Uh oh!

riot-ci commented Sep 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

basilfx commented Sep 7, 2026

Uh oh!

crasbe commented Sep 7, 2026

Uh oh!

basilfx commented Sep 8, 2026

Uh oh!

crasbe Sep 9, 2026

Uh oh!

basilfx Sep 9, 2026

Uh oh!

crasbe Sep 9, 2026

Uh oh!

crasbe Sep 9, 2026

Uh oh!

AnnsAnns commented Sep 9, 2026

Uh oh!

kfessel Sep 11, 2026

Uh oh!

kfessel Sep 11, 2026

Uh oh!

kfessel Sep 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	- Use `$(notdir ...)` instead of `$(shell basename ...)` to strip the
	- Use `$(basename $(notdir ...))` instead of `$(shell basename ...)` to strip the

	first_chars = $(subst $(space),,$(wordlist 1,$1,$(call split_chars,$2)))
	chars_upto = $(subst $(space),,$(wordlist 1,$1,$(call split_chars,$2)))

Uh oh!

Conversation

basilfx commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Declaration of AI-Tools / LLMs usage:

Uh oh!

riot-ci commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

Uh oh!

Uh oh!

basilfx commented Sep 7, 2026

Uh oh!

crasbe commented Sep 7, 2026

Uh oh!

basilfx commented Sep 8, 2026

Uh oh!

crasbe Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

basilfx Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

crasbe Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

crasbe Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

AnnsAnns commented Sep 9, 2026

Uh oh!

kfessel Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

kfessel Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

kfessel Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

basilfx commented Sep 6, 2026 •

edited

Loading

riot-ci commented Sep 6, 2026 •

edited

Loading

kfessel Sep 11, 2026 •

edited

Loading