sddf icon indicating copy to clipboard operation
sddf copied to clipboard

`export override` does not work as expected with GNU make

Open Courtney3141 opened this issue 11 months ago • 5 comments

While creating a multicore echo server, I stumbled across a problem with the makefiles effecting all versions of GNU make (found on version 3.81, but also occurs on version 4.41). The compound directive

export override x = a

when x is a command line provided variable only updates the value of x locally on the more recent version of make, and does not update the value of x locally or in sub-makes for the older version of make (which is the default macOS version). If instead, you split the command

override x := a export x

This updates the value of x locally for both versions of make, but not for sub-makes. The only variable we currently override is MICROKIT_SDK to ensure is holds the value of the absolute path, but for the SMP echo server I plan on introducing another path variable which I wish to override with the absolute path.

We discovered that currently the echo server makefile does successfully override the MICROKIT_SDK variable, but this is only due to the fact that the variable is set explicitly when invoking sub-make:

qemu ${IMAGE_FILE} ${REPORT_FILE} clean clobber: ${BUILD_DIR}/Makefile FORCE
	${MAKE}  -C ${BUILD_DIR} MICROKIT_SDK=${MICROKIT_SDK} $(notdir $@)

without MICROKIT_SDK=${MICROKIT_SDK}, the export override does not work.

For the time being, I will do the same for the new path variable I am creating, and perhaps this is the best solution for overriding command line variables, as after doing some research it seems that it is basically impossible to override a command line variable any other way as it has the highest precedence.

Perhaps we should remove the export override commands, as unless I am mistaken about the original intention, they do not work as expected.

Courtney3141 avatar Jan 30 '25 04:01 Courtney3141

Submakes are always invoked with the same command line variables as the parent. TO have the export command work the variable needs to be in teh environment, not in the command line.

So: X=y make is different from make X=y

In the first case, the overridden value`` of X will be passed down; in the second case it will not be.

wom-bat avatar Jan 30 '25 21:01 wom-bat

Here's a simple example: in Makefile:

export override FOO += bah

all:
	echo ${FOO}
	${MAKE} -e -f a.mk

in a.mk;

all:
	echo ${FOO}

Then:

$ make
make
echo bah
bah
make -e -f a.mk
make[1]: Entering directory '/home/peterc/src/work/make-test'
echo bah
bah
make[1]: Leaving directory '/home/peterc/src/work/make-test'

and

$ make FOO=foo
echo foo bah
foo bah
make -e -f a.mk
make[1]: Entering directory '/home/peterc/src/work/make-test'
echo foo
foo
make[1]: Leaving directory '/home/peterc/src/work/make-test'

and

$ FOO=foo make
echo foo bah
foo bah
make -e -f a.mk
make[1]: Entering directory '/home/peterc/src/work/make-test'
echo foo bah
foo bah
make[1]: Leaving directory '/home/peterc/src/work/make-test'

wom-bat avatar Jan 30 '25 21:01 wom-bat

Thanks for the explanation Peter, I didn't realise there was such a big difference between command line vs environment variables, it makes sense now.

There is still an issue here though. I mentioned that in GNU make 3.81 (which is the default version of make used for MacOS), the directive export override X=Y does not behave well with command line variables. After doing some testing with the examples you showed me, it turns out that it also does not behave as expected for environment variables.

If my makefile is:

export override FOO += bah

all:
	echo ${FOO}

then the outcome is:

$ FOO=foo make
echo foo
foo

If my makefile is:

override FOO += bah
export FOO

all:
	echo ${FOO}

or

override FOO += bah

all:
	echo ${FOO}

or even

export FOO += bah

all:
	echo ${FOO}

then the outcome is:

$ FOO=foo make
echo foo bah
foo bah

When testing with a more recent version of GNU make, it works like your example. It might be something worth changing, since it just seems like combining the two directives is not syntactically correct in the older version of make. If you instead use:

override FOO += bah
export FOO

then both makes behave in the same way.

But actually, I want to argue that the export override directive doesn't make sense for variables that we require the user to specify. If we expect variable X to be specified by the user, then it must have been specified as an environment or command line variable. If it is an environment variable, then neither export not override is necessary, and a simple assignment will change its value for make and submakes. If it is a command line variable, then override is necessary to change the value for make, and passing it's new value when submakes are invoked is necessary to propagate its new value. In neither case is export required.

If I am missing something, then please point it out, and perhaps we can just separate the commands onto two lines. Otherwise, I suggest we remove the export component of our export override commands.

Courtney3141 avatar Mar 06 '25 12:03 Courtney3141

@Courtney3141 I think what you're saying makes sense.

If it's passed in the environment, it is automatically exported to the sub-make automatically (along with any changes you make in the top-level makefile) per the make manual

Except by explicit request, make exports a variable only if it is either defined in the environment initially, or if set on the command line and its name consists only of letters, numbers, and underscores.

If it's passed by the command line and need to change the original value, you need to pass it explicitly in the args to the submake anyway, so exporting doesn't make a difference.

TLDR: All the export override is doing in our case is overwriting either an environment variable or a command-line argument. In both cases, it is not defining a NEW variable, so an export is not needed.

alwin-joshy avatar May 05 '25 22:05 alwin-joshy

Thanks. On top of that, putting export override in one command is not syntactically correct for the default version of GNU make for MacOS and has no effect, so it's worse than just being pointless.

I'll make a PR to change this.

Courtney3141 avatar May 06 '25 03:05 Courtney3141

Can we close this now?

Ivan-Velickovic avatar Sep 02 '25 04:09 Ivan-Velickovic

Yes

Courtney3141 avatar Sep 03 '25 00:09 Courtney3141