Closed
Bug 389793
Opened 17 years ago
Closed 17 years ago
Firefox build failed on OpenSolaris without --disable-mochitest
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
benjamin
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
gmake[6]: Entering directory `/export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/content/base/test'
/export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/config/nsinstall -R test_bug5141.html test_bug51034.html test_bug218236.html test_bug218277.html test_bug238409.html test_bug276037-1.html test_bug276037-2.xhtml test_bug308484.html test_bug311681.xml test_bug337631.html test_bug338541.xhtml test_bug338679.html test_bug339494.html test_bug339494.xhtml test_bug339494.xul test_bug343596.html test_bug352728.html test_bug352728.xhtml test_bug355026.html test_bug357450.js test_bug357450.html test_bug357450.xhtml test_bug357450.xul test_bug357450_svg.xhtml test_bug357509.html test_bug358660.html test_bug362391.xhtml test_bug364092.xhtml test_bug364413.xhtml test_bug366946.html test_bug367164.html test_bug371576-1.html test_bug371576-2.html test_bug371576-3.html test_bug371576-4.html test_bug371576-5.html test_bug372086.html test_bug373181.xhtml test_bug375314.html test_bug382113.html bug382113_object.html test_CrossSiteXHR.html file_CrossSiteXHR_fail1.xml file_CrossSiteXHR_fail2.xml file_CrossSiteXHR_fail2.xml^headers^ file_CrossSiteXHR_fail3.xml file_CrossSiteXHR_fail4.xml file_CrossSiteXHR_pass1.xml file_CrossSiteXHR_pass1.xml^headers^ file_CrossSiteXHR_pass2.xml file_CrossSiteXHR_pass3.xml ../../../_tests/testing/mochitest/tests/content/base/test
/bin/sh: headers: not found
/export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/config/nsinstall: cannot change directory to file_CrossSiteXHR_fail2.xml: Not a directory
/bin/sh: file_CrossSiteXHR_fail3.xml: not found
/bin/sh: file_CrossSiteXHR_pass2.xml: not found
/bin/sh: headers: not found
gmake[6]: *** [libs] Error 1
It seems Solaris /bin/sh doesn't like "^" in filename with no quotation marks.
It's fine with /bin/bash.
Works for me on Solaris.
Attachment #274130 -
Flags: review? → review?(jwalden+bmo)
Comment 3•17 years ago
|
||
I'm not entirely sure why this would fix the problem; it seems like the caret would still get passed through to the shell with this, but there'd be at most one FOO^headers^ file passed this way instead of possibly many. At the very least I'd like to understand why the one/many difference matters (assuming I'm not just misreading the shell script, since I usually avoid shell like the plague). Better would be to figure out a way to quote the arguments and make only one call to $(INSTALL), since this must run the install command once per test/data file.
Also, if you do it this way you'll be playing a constant game of whack-a-mole when mochitests in other directories need to use the new header functionality. We should come up with The One True Solution and update all the locations in the tree at once.
From man sh(1):
Quoting
The following characters have a special meaning to the shell
and cause termination of a word unless quoted:
; & ( ) | ^ < > newline space tab
That's why it doesn't work.
for i in $^; do \
$(INSTALL) "$$i" $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir); \
done
doesn't work, either.
Because ^ is still in the "for" command without quoting.
It works if we use
for i in "$^"; do \
In fact, nsinstall is only called once, because $^ is quoted.
I'm still trying to find an easier way to fix it.
BTW: Do we really want "^" in filename?
Here's the concept of the fix.
Feel free to modify it. Just don't make UNIX down too long.
BTW:
$(INSTALL) "$^" $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir);
doesn't work,
because nsinstall will consider the whole file list as one file with a very long name.
If we assign the file name list to a variable, we can get around.
I didn't get this fix at first place,
because I was trying to use FILES_TO_INSTALL = "$^";
I should not add extra space around "=". :(
Attachment #274130 -
Attachment is obsolete: true
Attachment #274422 -
Flags: review?(jwalden+bmo)
Attachment #274130 -
Flags: review?(jwalden+bmo)
And $(_TEST_FILES) doesn't work, either.
Because it contains <CR> rather than whitespace.
Comment 7•17 years ago
|
||
Interesting; I didn't know some shells treated ^ specially.
I'd ideally like to avoid a comment here, since I think it's more a distraction than helpful when reading the source. Since we're just trying to quote each filename, how's the following work for doing that, and in what I think is a fairly clear way that makes it obvious what's being done?
$(foreach f,$^,"$f")
Comment 8•17 years ago
|
||
^ was the original | (pipe) char, IIRC; like original Steve Bourne shell meaning of "original". But later it was retasked by csh for quick substitution in the last history event. God I'm old.
/be
Attachment #274422 -
Attachment is obsolete: true
Attachment #274422 -
Flags: review?(jwalden+bmo)
OK, it's fine.
Do you want to update all the Makefile.in at this time?
Comment 10•17 years ago
|
||
(In reply to comment #9)
> Do you want to update all the Makefile.in at this time?
Yes, I think that best given that this is code that's copy-pasted all the time. Also, even tho the chrome tests don't actually use the HTTP server, I think we should make the change there as a correctness measure (and also because I bet we'll eventually need the server there), so please change all those as well.
http://mxr.mozilla.org/mozilla/search?string=testing%2Fmochitest%2Fchrome
Comment 11•17 years ago
|
||
Also, bsmedberg apparently wants to review the patch, so request review from him.
Assignee | ||
Comment 12•17 years ago
|
||
Attachment #274565 -
Attachment is obsolete: true
Attachment #274729 -
Flags: review?(benjamin)
Updated•17 years ago
|
Attachment #274729 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 14•17 years ago
|
||
Comment on attachment 274729 [details] [diff] [review]
patch v3
blocking Solaris builds
Attachment #274729 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #274729 -
Flags: approval1.9? → approval1.9+
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Component: Testing → Build Config
QA Contact: testing → build-config
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
•