Closed
Bug 610290
Opened 14 years ago
Closed 14 years ago
`TEST_PATH="foo/reftest.list" make reftest` doesn't run reftests
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla2.0b8
People
(Reporter: mounir, Assigned: mounir)
Details
Attachments
(1 file)
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
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
Assignee | ||
Comment 2•14 years ago
|
||
Thanks for the explanation ted :) changing = to ?= fixes it.
Attachment #490253 -
Flags: review?(khuey)
Attachment #490253 -
Flags: review+
Assignee | ||
Comment 3•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Comment 4•14 years ago
|
||
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•14 years ago
|
||
I filed bug 612677 about the issue in comment 4.
Comment 6•14 years ago
|
||
This seems to have broken passing TEST_PATH as a make variable for me; it now only works as an environment variable.
Comment 7•14 years ago
|
||
(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•14 years ago
|
||
That bug looks like it only applies to crashtest and jsreftest. Does it magically somehow apply to reftest as well?
Assignee | ||
Comment 9•14 years ago
|
||
This bug is changing TEST_PATH for reftest and bug 612677 for the other ones.
Assignee | ||
Comment 10•14 years ago
|
||
(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•14 years ago
|
||
(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?
Assignee | ||
Comment 12•14 years ago
|
||
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•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
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?
Updated•14 years ago
|
Flags: in-testsuite-
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•