Closed Bug 610290 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: mounir, Assigned: mounir)

Details

Attachments

(1 file)

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.
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
Attached patch Patch v1Splinter Review
Thanks for the explanation ted :) changing = to ?= fixes it.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #490253 - Flags: review?(khuey)
Pushed:
http://hg.mozilla.org/mozilla-central/rev/343ca3179e98
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
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
I filed bug 612677 about the issue in comment 4.
This seems to have broken passing TEST_PATH as a make variable for me; it now only works as an environment variable.
(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?
That bug looks like it only applies to crashtest and jsreftest.  Does it magically somehow apply to reftest as well?
This bug is changing TEST_PATH for reftest and bug 612677 for the other ones.
(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?
(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?
I've open a bug upstream:
http://savannah.gnu.org/bugs/?31743

Maybe it would worth trying to expand ?= until it gets fixed upstream?
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.
This has been fixed upstream (I've tested the check locally). I guess we can keep ?= operator then :)
Can we verify that pymake doesn't have the same bug?
Flags: in-testsuite-
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.