1801182 - Allow overriding OS_ARCH, OS_TEST and OS_RELEASE in Makefile

Status: UNCONFIRMED

(NSS :: Build, defect, P3)

Description

[Giulio Benetti]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0

Steps to reproduce:

I've built with Make-4.3.91 using Buildroot, here is the log:
http://autobuild.buildroot.net/results/1074143dbea60567cd83be0a23f7c0214d470de9/build-end.log

Actual results:

Note:

Linux5.15_x86_mips-buildroot-linux-musl-gcc.br_real_glibc_PTH_DBG.OBJ

compared to:

Linux2.6_mips_mips-buildroot-linux-musl-gcc.br_real_glibc_PTH_DBG.OBJ

The last should be the correct one, but starting from Make-4.3.91 the := variables are not overriden via command line correctly.
The problem is that Makefile variables OS_ARCH, OS_TEST and OS_RELEASE that we override in Buildroot get a strange value due to the fact that at the moment those variable are Simple Expanded:
OS_TEST := $(shell uname -m)
but here we need conditional expanded:
OS_TEST ?= $(shell uname -m)
So up to make 4.3 it worked with := and starting from make 4.3.91 it doesn't work anymore.

Comment 1

[Giulio Benetti]

Attachment: 0001-Bug-1801182-Allow-overriding-OS_ARCH-OS_TEST-and-OS_.patch

Comment 2

[rudi]

See also - https://bugzilla.mozilla.org/show_bug.cgi?id=1800237

Comment 3

[Giulio Benetti]

(In reply to rudi from comment #2)

See also - https://bugzilla.mozilla.org/show_bug.cgi?id=1800237

Hi Rudi,

my patch has been accepted in Buildroot:
https://github.com/buildroot/buildroot/commit/5852fee868c318f0864bed3238b46348156422b6

And Autobuilders stopped to have build failures due to that bug:(Note date 2022-11-22 VS 2022-11-21):
http://autobuild.buildroot.net/?reason=libnss-%

Comment 4

[Jan Palus]

Not sure I follow what this patch fixes exactly:

$ cat Makefile 
OS_TEST := $(shell uname -m)
OS_TEST2 ?= $(shell uname -m)

test:
	echo $(OS_TEST)
	echo $(OS_TEST2)

$ make OS_TEST=customarch OS_TEST2=customarch 
echo customarch
customarch
echo customarch
customarch

$ OS_TEST=customarch OS_TEST2=customarch make
echo aarch64
aarch64
echo customarch
customarch

Works the same way both with make 4.3 and make 4.4.

Comment 5

[Jan Palus]

I guess the idea is to workaround make new inconsistency by taking OS_TEST from process environment since MAKEFLAGS are applied selectively now.

Comment 6

[Giulio Benetti]

(In reply to Jan Palus from comment #5)

I guess the idea is to workaround make new inconsistency by taking OS_TEST from process environment since MAKEFLAGS are applied selectively now.

Exactly, I’ve given an explanation that is not so clear, but what happens is that after a lot of recursive inclusion of Linux.mk, variables passed via command line can’t override the local simple expanded variables and that makes build to fail. And this happens in Make 4.3.91 and 4.4 and both cases are fixed with this patch, that yes, it’s a work around to be precise.

Comment 7

[Giulio Benetti]

Hello,

any news for this patch? It's applied to Buildroot since 2 months and Autobuilders show that the bug is fixed.
Can you please commit it?

Best regards

Comment 8

[Jan Palus]

I'm not really sure if Mozilla will be willing to apply it now that it's known the root cause is a bug in make which is already fixed upstream. Admittedly not officially released yet though. This state might not last long as 4.4.0.90 was tagged recently suggesting that 4.4.1 will be out soon.

Comment 9

[Giulio Benetti]

I'm not really sure if Mozilla will be willing to apply it now that it's known the root cause is a bug in make which is already fixed upstream. Admittedly not officially released yet though. This state might not last long as 4.4.0.90 was tagged recently suggesting that 4.4.1 will be out soon.

Hi Jan,

the problem is that Make 4.4.0.90 will be there outside forever and soon or later someone will experience the same problem.
It will take time for everybody to update to Make 4.4.1 and also the go-to for Makefile to override a variable is typically using the
form ?= instead of :=
This is my 2 cents, but it would make sense to apply IMHO. It is pretty well tested by Buildroot Autobuilders and Users because
NSS is used as dependency by a lot of other packages we provide in Buildroot.

Anyway this is up to you.

Best regards(In reply to Jan Palus from comment #8)