610290 - `TEST_PATH="foo/reftest.list" make reftest` doesn't run reftests

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mounir Lamouri (:mounir)]

Having it work would be nice for consistency with the most common way to run mochitests:
TEST_PATH="dir/" make mochitest-plain

1. foo/reftest.list is a valid reftest.list path
2. `make TEST_PATH="foo/reftest.list" reftest` runs the tests
3. `TEST_PATH="foo/reftest.list` make reftest` doesn't run the tests

Expected: 3. should work like 2.

Comment 1

[(not currently active) Ted Mielczarek]

This has to do with how GNU Make treats variables from the environment:
http://www.gnu.org/software/make/manual/make.html#Values

When you run:
TEST_PATH=foo make, you're passing TEST_PATH as an environment variable.
When you run:
make TEST_PATH=foo, you're passing TEST_PATH as a make variable on the commandline.

Because we set TEST_PATH as a target-specific variable:
http://mxr.mozilla.org/mozilla-central/source/testing/testsuite-targets.mk#111

it overrides any variable from the environment, but not from the commandline. You could try changing that TEST_PATH= to TEST_PATH?= , that will probably have the desired effect:
http://www.gnu.org/software/make/manual/make.html#Target_002dspecific

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Thanks for the explanation ted :) changing = to ?= fixes it.

Comment 3

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/343ca3179e98

Comment 4

[(no longer active)]

Is this possible that this patch has broken TEST_PATH for crashtest?  I used to be able to do:

make -C objdir crashtest TEST_PATH=path/to/crashtest.list

Now, neither of these two work, and they both result in the entire crashtest suite to be run:

make -C objdir crashtest TEST_PATH=path/to/crashtest.list
TEST_PATH=path/to/crashtest.list make -C objdir crashtest

Comment 5

[(no longer active)]

I filed bug 612677 about the issue in comment 4.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This seems to have broken passing TEST_PATH as a make variable for me; it now only works as an environment variable.

Comment 7

[(no longer active)]

(In reply to comment #6)
> This seems to have broken passing TEST_PATH as a make variable for me; it now
> only works as an environment variable.

Mounir fixed that in bug 612677.  If you see more issues on this, can you please comment in that bug and/or file a new one?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That bug looks like it only applies to crashtest and jsreftest.  Does it magically somehow apply to reftest as well?

Comment 9

[Mounir Lamouri (:mounir)]

This bug is changing TEST_PATH for reftest and bug 612677 for the other ones.

Comment 10

[Mounir Lamouri (:mounir)]

(In reply to comment #6)
> This seems to have broken passing TEST_PATH as a make variable for me; it now
> only works as an environment variable.

According to dholbert (on IRC), this is a bug with Ubuntu's package. That would explain why it works well for me (I'm using Gentoo).
Daniel, what would be the workaround?

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to comment #10)
> (In reply to comment #6)
> According to dholbert (on IRC), this is a bug with Ubuntu's package.

Sort of -- there is indeed a bug in Ubuntu's make 3.81 package that causes the make variable to end up with random value if it's assigned via ?= in the Makefile and also given a value on the command line.

Sadly, GNU make version 3.82 (unmodified) seems to have a similar bug -- in that case, the make variable just ends up being empty, instead of having a random value.

e.g. with this Makefile:
> test: FOO?=defaultval
> test:
> 	echo $(FOO)
...I get these results:
> [dholbert@desk:~/tmp]$ make FOO=bar
> echo h)
> /bin/sh: Syntax error: ")" unexpected
> make: *** [test] Error 2
>
> [dholbert@desk:~/tmp]$ ./make-3.81 FOO=bar
> echo bar
> bar
>
> [dholbert@desk:~/tmp]$ ./make-3.82 FOO=bar
> echo 

(The later two commands are using locally-built stock GNU make 3.81 & 3.82 from http://ftp.gnu.org/gnu/make/ .)

So apparently the "?=" operator works with stock GNU make 3.81, but is busted in make 3.82 -- though I don't know who's running that -- as well as in Ubuntu's package of make 3.81.

> Daniel, what would be the workaround?

I don't know of one, aside from "use an environmental variable instead" or "install unmodified GNU make 3.81 on your system". :)

Or perhaps we could just expand this ?= one-liner into the equivalent multi-line logic?

Comment 12

[Mounir Lamouri (:mounir)]

I've open a bug upstream:
http://savannah.gnu.org/bugs/?31743

Maybe it would worth trying to expand ?= until it gets fixed upstream?

Comment 13

[(not currently active) Ted Mielczarek]

I don't know that there is an equivalent multi-line construct. It's one line because it's using a target-specific variable value:
http://www.gnu.org/software/make/manual/make.html#Target_002dspecific

Perhaps something like this would work:
ifdef TEST_PATH
REFTEST_PATH:=$(TEST_PATH)
else
REFTEST_PATH=layout/reftests/reftest.list
endif

reftest: TEST_PATH=$(REFTEST_PATH)

Kind of awkward, but might work.

Comment 14

[Mounir Lamouri (:mounir)]

This has been fixed upstream (I've tested the check locally). I guess we can keep ?= operator then :)

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Can we verify that pymake doesn't have the same bug?