bugmake - Bugs: bug #63347, make 4.4 change in behavior for...

 
 

bug #63347: make 4.4 change in behavior for sub-make invoked via $(shell)

Submitter:  None
Submitted:  Fri 11 Nov 2022 08:21:14 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  4.4 Operating System:  POSIX-Based
Fixed Release:  4.4.1 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 24 Dec 2022 04:27:08 PM UTC, comment #13: 

I will add a note to NEWS, thanks for the suggestion Dmitry.

Paul D. Smith <psmith>
Group administrator
Tue 29 Nov 2022 07:34:29 PM UTC, comment #12: 

Thanks for review, Paul.
Do you think this should be mentioned in NEWS?

Dmitry Goncharov <dgoncharov>
Mon 28 Nov 2022 10:22:52 PM UTC, comment #11: 

Pushed, with some small cleanups.  Thanks Dmitry!

Paul D. Smith <psmith>
Group administrator
Sun 27 Nov 2022 03:00:15 PM UTC, comment #10: 

The problem in the kernel build system is not related to this change; the issue can also be seen with the released GNU make 4.4 version.

For example, set MAKEFLAGS like this in your environment before invoking the kernel build:

$ export MAKEFLAGS=-I/usr/local/mk

or this:

$ export MAKEFLAGS=-Orecurse


If you use vanilla GNU make 4.4 without this patch, this will force the kernel into "silent mode".

Basically any flag that (a) uses a single-letter option to introduce it (so it won't be caught by the kernel build environment's --% filter) and (b) contains an "s", will cause this problem.

The best way to check GNU make single-letter command line flags is described in the GNU make manual: https://www.gnu.org/software/make/manual/make.html#Testing-Flags

Paul D. Smith <psmith>
Group administrator
Sun 27 Nov 2022 02:31:10 PM UTC, comment #9: 


> Differences that I could spot:
> - there's "rR" instead of "-rR"
> - "-rR" is at the beginning not at the end
> - MAKE_OPTS are part of MAKEFLAGS --


Well defined MAKEFLAGS has the following format
[shortoptions] [-option with arg]... [--long option]... [ -- cli definitions]
e.g.
rRs -Isrc -j2 -Orecurse --jobserver-auth=fifo:/tmp/GMfifo14676 --no-print-directory -- H=h C=c A=a

makefile can contain something like
MAKEFLAGS+=-k
MAKEFLAGS=r

Once parsing is over make rearranges MAKEFLAGS to conform to this format.
This patch causes make to do this rearrangement at parse time, each time
MAKEFLAGS is modified.
You observe the differences between a defined MAKEFLAGS and not defined MAKEFLAGS.


> that's what confuses kernel's build system since it merely looks for 's' in
> MAKEFLAGS after stripping long options:


Thanks for your report.
i submitted a patch here
https://lore.kernel.org/linux-kbuild/CAK7LNAQP4S0ACMkB3KtaJTaeRkpT_KjRa4CrYxNJboTdthN=Zw@mail.gmail.com/T/#m6e2681847c0b7968a713765f1df455bfae307807

Dmitry Goncharov <dgoncharov>
Sat 26 Nov 2022 11:39:17 PM UTC, comment #8: 

Looks like attached patch changes behavior in the way that makes Linux kernel build system assume silent mode.

For following input:


$ cat Makefile
MAKE_OPTS := A=a C=c H=h
all:
        $(MAKE) -C test $(MAKE_OPTS) all

$ cat test/Makefile
MAKEFLAGS += -rR

$(info MAKEFLAGS=$(MAKEFLAGS))

all:


Unpatched make 4.4:

$ make -j2
...
MAKEFLAGS= -j2 --jobserver-auth=fifo:/home/users/builder/tmp/GMfifo655627 -rR
...


Patched make 4.4:

$ make -j2
...
MAKEFLAGS=rR -j2 --jobserver-auth=fifo:/home/users/builder/tmp/GMfifo1150317 -- H=h C=c A=a
...


Differences that I could spot:
- there's "rR" instead of "-rR"
- "-rR" is at the beginning not at the end
- MAKE_OPTS are part of MAKEFLAGS -- that's what confuses kernel's build system since it merely looks for 's' in MAKEFLAGS after stripping long options: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Makefile?id=e5f3ec38c8496dd7f6ada8a5e8d4958ef46ddb3f#n97

Jan Palus <jpalus>
Wed 23 Nov 2022 12:27:07 PM UTC, comment #7: 

Tested in my cross compile scenarios. Good too.

Rudi Heitbaum <heitbaum>
Wed 23 Nov 2022 09:09:45 AM UTC, comment #6: 

Thanks Dmitry. I can confirm that the patch fixes nss build that relies on same flags being passed to all $(shell)s (https://bugzilla.mozilla.org/show_bug.cgi?id=1800237).

Jan Palus <jpalus>
Wed 23 Nov 2022 02:07:10 AM UTC, comment #5: 

Thanks for your report.


This patch introduces two visible changes.
1. Have command line variable definitions in MAKEFLAGS at parse time.
2. Define makeflags at parse time, each time a makefile modifies MAKEFLAGS.

The patch contains a more detailed description of the change.


(file #53991, file #53992)

Dmitry Goncharov <dgoncharov>
Wed 16 Nov 2022 03:24:45 PM UTC, comment #4: 

Yes, that's annoying.  The problem is that MAKEFLAGS is not fully defined until after all the makefiles are parsed.  I remember I changed that as a test last year and something didn't work right, but I can't remember now what it was.  Looks like this needs to be re-investigated.

Paul D. Smith <psmith>
Group administrator
Fri 11 Nov 2022 11:01:17 PM UTC, comment #3: 

Just to be clear I don't mind the change with $(shell) now receiving those vars, what I do mind is that current behavior is selective of which $(shell)s do receive it. The one in target dependency does not. What I miss is uniform consistent behavior.

Jan Palus <jpalus>
Fri 11 Nov 2022 09:39:31 PM UTC, comment #2: 

Yes, this is changed in this release; please see the NEWS file https://git.savannah.gnu.org/cgit/make.git/tree/NEWS?h=4.4#n74 :

* WARNING: Backward-incompatibility!
  Previously makefile variables marked as export were not exported to commands
  started by the $(shell ...) function.  Now, all exported variables are
  exported to $(shell ...). [...]


If you don't want a sub-make to receive the command-line overrides, you can reset it via `MAKEOVERRIDES`; see https://www.gnu.org/software/make/manual/html_node/Options_002fRecursion.html

An option that I think does what you want might be:


get_conf_var = $(shell $(MAKE) -C lib -s get_conf_var)

all: $(get_conf_var)
        @echo $(get_conf_var)

all: private MAKEOVERRIDES :=


Result:

$ make --version | head -n1
GNU Make 4.4

$ make CONF_VAR=bar
called target foo
CONF_VAR=bar
get_conf_var=foo


Another option you can use to be more specific BUT which requires GNU Make 4.4 and won't work with earlier versions is to use the new let function https://www.gnu.org/software/make/manual/html_node/Let-Function.html :

get_conf_var = $(shell $(MAKE) -C lib -s get_conf_var)

all: $(get_conf_var)
        @echo $(let MAKEOVERRIDES,,$(get_conf_var))


Personally I think that relying on this odd quirk of shell behavior is hard for people to understand, especially in the old behavior where you got different results from running the shell function in different contexts.  It seems to me that finding a better way that made this requirement more explicit would be much cleaner.

Paul D. Smith <psmith>
Group administrator
Fri 11 Nov 2022 09:35:35 PM UTC, comment #1: 

This behavior changed to fix https://savannah.gnu.org/bugs/?10593.
See commit 98da874c43035a490cdca81331724f233a3d0c9a.

Dmitry Goncharov <dgoncharov>
Fri 11 Nov 2022 08:21:14 PM UTC, original submission:  

It appears that make 4.3 never passed environment down to sub-make if invoked via shell so ie if make was invoked `make CONF_VAR=foo` then `$(shell $(MAKE) -C subdir)` would not receive `CONF_VAR=foo`.

Can't tell whether that's expected or not but big advantage of make 4.3 was that environment was not passed consistently irrespective of where that `$(shell $(MAKE)...)` occurs.

Now that's no longer the case with make 4.4. Let's consider following example:

all: $(shell $(MAKE) -C subdir --no-print-directory get_conf_var)
        @echo $(shell $(MAKE) -C subdir --no-print-directory get_conf_var)

These two invocations will differ in environment:
- the one producing dependency will not have environment passed
- the one in target body will receive environment

more complete example:

$ cat Makefile
all: $(shell $(MAKE) -C lib --no-print-directory get_conf_var)
@echo CONF_VAR=$(CONF_VAR)
@echo get_conf_var=$(shell $(MAKE) -C lib --no-print-directory get_conf_var)

foo bar:
@echo called target $@

$ cat lib/Makefile
CONF_VAR=foo
get_conf_var:
@echo $(CONF_VAR)

$ make-4.3 CONF_VAR=bar
called target foo
CONF_VAR=bar
get_conf_var=foo

$ make-4.4 CONF_VAR=bar
called target foo
CONF_VAR=bar
get_conf_var=bar

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53991:  sv63347_fix.diff added by dgoncharov (16KiB - text/x-patch)
file #53992:  sv63347_test.diff added by dgoncharov (13KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by heitbaum (Posted a comment)
  • -email is unavailable- added by jpalus (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dgoncharov (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-28 psmith StatusNone Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4.1
        Triage StatusNone Medium Effort
    2022-11-23 dgoncharov Attached File- Added sv63347_fix.diff, #53991
        Attached File- Added sv63347_test.diff, #53992

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code