Closed Bug 389793 Opened 16 years ago Closed 16 years ago

Firefox build failed on OpenSolaris without --disable-mochitest


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)




(1 file, 3 obsolete files)

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.
Attached patch patch (obsolete) — Splinter Review
Works for me on Solaris.
Assignee: nobody → ginn.chen
Attachment #274130 - Flags: review?
Attachment #274130 - Flags: review? → review?(jwalden+bmo)
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):
     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); \

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?
Attached patch patch (obsolete) — Splinter Review
Here's the concept of the fix.
Feel free to modify it. Just don't make UNIX down too long.

$(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.
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")
^ 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.

Attachment #274422 - Attachment is obsolete: true
Attachment #274422 - Flags: review?(jwalden+bmo)
Attached patch patch v2 (obsolete) — Splinter Review
OK, it's fine.

Do you want to update all the at this time?
(In reply to comment #9)
> Do you want to update all the 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.
Also, bsmedberg apparently wants to review the patch, so request review from him.
Attached patch patch v3Splinter Review
Attachment #274565 - Attachment is obsolete: true
Attachment #274729 - Flags: review?(benjamin)
Attachment #274729 - Flags: review?(benjamin) → review+
Comment on attachment 274729 [details] [diff] [review]
patch v3

blocking Solaris builds
Attachment #274729 - Flags: approval1.9?
Attachment #274729 - Flags: approval1.9? → approval1.9+
Closed: 16 years ago
Resolution: --- → FIXED
Component: Testing → Build Config
QA Contact: testing → build-config
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.