| Message ID | patch-v2-3.4-6db7dd74e52-20221129T140159Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 67b36879fc06581131fa7e57c9ee1e560ea9d1fc |
| Headers | show |
| Series | Avoid multiple patterns when recipes generate one file | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has > changed in a backward-incompatible way, as its "NEWS" file notes: > > Previously only simple (one-letter) options were added to the MAKEFLAGS > variable that was visible while parsing makefiles. Now, all options are > available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter > option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return > the set of one-letter options which can be examined via findstring, etc. Wow. That's a bold move for GNU make folks to make. > This upstream change meant that e.g.: > > make man > > Would become very noisy, because in shared.mak we rely on extracting > "s" from the $(MAKEFLAGS), which now contains long options like > "--jobserver-auth=fifo:<path>", which we'll conflate with the "-s" > option. Do our uses of $(MAKEFLAGS) for the $(PRINT_DIR) and the $(QUIET) macros that do not affect correctness? $(QUIET) thing I suspect will merely be annoyance, but $(PRINT_DIR) might affect correctness depending on how $(MAKE) output is being used. I have to wonder how many projects they have broken with this change ;-). In any case, this seems like a good thing to do. I am not sure if this is so urgent to add in the -rc period, or can safely wait post release. Thanks.
On Wed, 2022-11-30 at 13:28 +0900, Junio C Hamano wrote:
> I have to wonder how many projects they have broken with this change
Some, but not that many. Most projects don't try to investigate
MAKEFLAGS, and of those that do many were already using the recommended
method, because even prior to GNU make 4.4 it was possible for
MAKEFLAGS to have stray "s" characters, in unusual situations (for
example if MAKEFLAGS were set in the makefile).
There were various bugs filed that various options could not be
investigated from within makefiles and also that running make from
within $(shell ...) didn't work right because MAKEFLAGS was not set.
It was just a mess, trying to keep the value of MAKEFLAGS set to
different values at different points in the processing of make.
Also, ensuring this trick for searching MAKEFLAGS continues to work
would have meant strictly controlling what new options we could add to
GNU make. I haven't seen any other project use the filter-out --%
trick that the Git makefiles do, but even with that it won't help if a
new single-letter option that takes an argument is added.
On Wed, Nov 30 2022, Paul Smith wrote: > On Wed, 2022-11-30 at 13:28 +0900, Junio C Hamano wrote: >> I have to wonder how many projects they have broken with this change > > Some, but not that many. Most projects don't try to investigate > MAKEFLAGS, and of those that do many were already using the recommended > method, because even prior to GNU make 4.4 it was possible for > MAKEFLAGS to have stray "s" characters, in unusual situations (for > example if MAKEFLAGS were set in the makefile). > > There were various bugs filed that various options could not be > investigated from within makefiles and also that running make from > within $(shell ...) didn't work right because MAKEFLAGS was not set. > > It was just a mess, trying to keep the value of MAKEFLAGS set to > different values at different points in the processing of make. It was definitely a bit of a hack on our part, but to be fair before this 4.4 release doing it this way was recommended by the documentation. I see you changed that recently, but maybe this on top makes sense? diff --git a/doc/make.texi b/doc/make.texi index e3a3ade4..9e9a894e 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -5069,7 +5069,7 @@ Variable @code{MAKEFILES}}. @vindex MAKEFLAGS Flags such as @samp{-s} and @samp{-k} are passed automatically to the sub-@code{make} through the variable @code{MAKEFLAGS}. This variable is -set up automatically by @code{make} to contain the flag letters that +set up automatically by @code{make} to contain the normalized flag letters that @code{make} received. Thus, if you do @w{@samp{make -ks}} then @code{MAKEFLAGS} gets the value @samp{ks}. @@ -5085,6 +5085,10 @@ option has both single-letter and long options, the single-letter option is always preferred. If there are no single-letter options on the command line, then the value of @code{MAKEFLAGS} starts with a space. +The value of @code{MAKEFLAGS} does not correspond to the order in which +command line options are provided. Both @w{@samp{make -sk}} and @w{@samp{make -sk}} +will result in a @code{MAKEFLAGS} value of @samp{ks}. + @cindex command line variable definitions, and recursion @cindex variables, command line, and recursion @cindex recursion, and command line variable definitions @@ -12378,10 +12382,13 @@ influences such as interrupts (@code{SIGINT}), etc. You may want to install signal handlers to manage this write-back. @item -Your tool may also examine the first word of the @code{MAKEFLAGS} variable and +Your tool may also examine the first word of the @samp{-$(MAKEFLAGS)} expression and look for the character @code{n}. If this character is present then @code{make} was invoked with the @samp{-n} option and your tool may want to stop without performing any operations. + +Note that this is not equivalent to checking for the first word of +@code{MAKEFLAGS}. @end itemize @node Windows Jobserver, , POSIX Jobserver, Job Slots I.e. that "Your tool" part seems to still be assuming 4.3 semantics. > Also, ensuring this trick for searching MAKEFLAGS continues to work > would have meant strictly controlling what new options we could add to > GNU make. I haven't seen any other project use the filter-out --% > trick that the Git makefiles do, but even with that it won't help if a > new single-letter option that takes an argument is added. I'd think it would probably make sense to promise that GNU make will never add such options, so that what's currently documented continues to work. I.e. it supports: --debug=all Not these forms: -dwhy -d=why But if we're on the topic: The only reason git's Makefile uses these is because it's trying to fake up some pretty verbose-but-not-too-verbose mode. You can see this in our tree at "shared.mak", the kernel does something similar. For our Makefile this is pretty close to what we'd get from a simpler: make -B --debug=why -s | sed -E -n \ -e "s/.*: update target '(.*).o' due to.*/ CC \\1.o/" \ -e 's/due to.*//' \ -e 'p' I.e. when we have %.o" targets this is emitting " CC $@" lines, I've left matching the rest as an excercise for the reader, but it would be e.g. "GEN_PERL" or whatever for %.pm and so on. I don't know what this would look like exactly, but it would be neat if GNU make supported some way to emit such friendly output in general. Something like a sprintf format where you'd have access to the sort of input that "due to" string gets internally (and perhaps a bit more, e.g. something indicating overall progress through the graph...).
diff --git a/git-gui/Makefile b/git-gui/Makefile index 56c85a85c1e..a0d5a4b28e1 100644 --- a/git-gui/Makefile +++ b/git-gui/Makefile @@ -116,7 +116,7 @@ ifeq ($(uname_S),Darwin) TKEXECUTABLE = $(shell basename "$(TKFRAMEWORK)" .app) endif -ifeq ($(findstring $(MAKEFLAGS),s),s) +ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s) QUIET_GEN = endif diff --git a/shared.mak b/shared.mak index be1f30ff206..aeb80fc4d5a 100644 --- a/shared.mak +++ b/shared.mak @@ -37,13 +37,13 @@ space := $(empty) $(empty) QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir QUIET_SUBDIR1 = -ifneq ($(findstring w,$(MAKEFLAGS)),w) +ifneq ($(findstring w,$(firstword -$(MAKEFLAGS))),w) PRINT_DIR = --no-print-directory else # "make -w" NO_SUBDIR = : endif -ifneq ($(findstring s,$(MAKEFLAGS)),s) +ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s) ifndef V ## common QUIET_SUBDIR0 = +@subdir=
Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has changed in a backward-incompatible way, as its "NEWS" file notes: Previously only simple (one-letter) options were added to the MAKEFLAGS variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return the set of one-letter options which can be examined via findstring, etc. This upstream change meant that e.g.: make man Would become very noisy, because in shared.mak we rely on extracting "s" from the $(MAKEFLAGS), which now contains long options like "--jobserver-auth=fifo:<path>", which we'll conflate with the "-s" option. So, let's change this idiom we've been carrying since [1], [2] and [3] as the "NEWS" suggests. Note that the "-" in "-$(MAKEFLAGS)" is critical here, as the variable will always contain leading whitespace if there are no short options, but long options are present. Without it e.g. "make --debug=all" would yield "--debug=all" as the first word, but with it we'll get "-" as intended. Then "-s" for "-s", "-Bs" for "-s -B" etc. 1. 0c3b4aac8ec (git-gui: Support of "make -s" in: do not output anything of the build itself, 2007-03-07) 2. b777434383b (Support of "make -s": do not output anything of the build itself, 2007-03-07) 3. bb2300976ba (Documentation/Makefile: make most operations "quiet", 2009-03-27) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-gui/Makefile | 2 +- shared.mak | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)