Bruno Haible [2025-11-15 13:01 +0100] wrote: > I don't understand the problem in 0003: "recursive $(shell) expansion". > What do you mean? The result of the $(shell ...) expression is a file name; > neither GNULIB_SRCDIR nor srcdir normally contains a '$' character or > parentheses.
Sorry if this wasn't described clearly enough. I'm using GNU Make's definition[0] of recursively expanded variables assigned via =, as opposed to simply expanded variables assigned via :=. [0]: (info "(make) Flavors") Simon Josefsson [2025-11-15 13:21 +0100] wrote: > "Basil L. Contovounesios" <[email protected]> writes: >> Subject: [PATCH 3/4] maintainer-makefile: Avoid recursive $(shell) expansion. > ... >> -gnulib_dir ?= $(shell if test -n "$(GNULIB_SRCDIR)" && test -f >> "$(GNULIB_SRCDIR)/gnulib-tool"; then \ >> - echo "$(GNULIB_SRCDIR)"; \ >> - else \ >> - echo $(srcdir)/gnulib; \ >> - fi) >> +ifeq ($(origin gnulib_dir),undefined) >> + gnulib_dir := $(shell if test -n "$(GNULIB_SRCDIR)" \ >> + && test -f "$(GNULIB_SRCDIR)/gnulib-tool"; >> then \ >> + echo "$(GNULIB_SRCDIR)"; \ >> + else \ >> + echo $(srcdir)/gnulib; \ >> + fi) >> +endif > > What problem are you solving here? The problem is arguably insignificant, but I what I was trying to avoid is the following. In (info "(make) Conditional Assignment"), FOO ?= bar is defined as equivalent to ifeq ($(origin FOO),undefined) FOO = bar endif But 'recursively expanded' variables assigned via = are reexpanded on every occurrence, so in foo ?= $(shell mktemp) $(info $(foo) $(foo)) mktemp will run twice, whereas in foo := $(shell mktemp) $(info $(foo) $(foo)) mktemp will run only once, and its result saved. > I know this part of cfg.mk is really horrible, and I often ran into bugs > caused by this guessing. So this is fairly fragile code, and I worry > that you may be breaking existing assumptions about this code. Not > saying we shouldn't fix this, but it has to be done carefully. I think > you are on to something here by using the 'undefined' comparison though. The only thing I intended to change is the default flavour of the variable gnulib_dir, so that every use within maint.mk consistently expands to the same value, and so that the shell is not repeatedly invoked on each expansion. Any existing cfg.mk overrides should still maintain the same semantics as before, thanks to the $(origin) guard. In particular, cfg.mk can still assign gnulib_dir via = if desired. Note that this was purely a 'housekeeping/prevention' patch, and not addressing any real-world bug. Thanks, -- Basil
