Closed
Bug 450948
Opened 16 years ago
Closed 16 years ago
Remove Mac case from AUTOCONF detection code.
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: romaxa, Assigned: kairo)
References
Details
Attachments
(2 files, 2 obsolete files)
1.72 KB,
patch
|
Details | Diff | Splinter Review | |
547 bytes,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
Please check it on Mac OSX environment...
Attachment #334168 -
Flags: review?(ted.mielczarek)
Comment 1•16 years ago
|
||
Comment on attachment 334168 [details] [diff] [review] Bugfix Looks ok to me.
Attachment #334168 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 2•16 years ago
|
||
Ok, this breaks heavily. I don't think it's exactly what we want. cb-sea-miniosx01:~ seabld$ which autoconf-2.13 autoconf2.13 autoconf213 | head -1 no autoconf-2.13 in /opt/local/bin /opt/local/sbin /bin /sbin /usr/bin /usr/sbin A basically working version is: which autoconf-2.13 autoconf2.13 autoconf213 2>&1 | grep -v 'no autoconf' | grep -v 'unknown command' | head -n1 but this sound like it easily might break again. Why it needs to be that way: 1) mac sends an error like "no autoconf-2.13 in ..." to stdout 2) linux sends an error like "which: no autoconf213 in ..." to stderr 3) Windows sends an error like "which: autoconf2.13: unknown command" I wonder if we should completely revert all this.
Assignee | ||
Comment 3•16 years ago
|
||
actually, the 1) case is 10.4 only, 10.5 sends to stderr like it should. So this would work as well: # discard errors from 'which' # MacOS X 10.4 sends "no autoconf*" errors to stdout, discard those via grep AUTOCONF ?= $(shell which autoconf-2.13 autoconf2.13 autoconf213 2>/dev/null | grep -v 'no autoconf' | head -1)
Comment 4•16 years ago
|
||
Instead of relying on which, and effectively on the filename, why not just invoke the various possible command names (trying plain 'autoconf' last) as: $> $cmd --version And picking the first one that correctly reports being autoconf 2.13
Comment 5•16 years ago
|
||
Feels like overkill, especially since this will be executed every time client.mk is run. I'd be fine with Kairo's suggestion there.
Assignee | ||
Comment 6•16 years ago
|
||
Here's the fix I proposed in the comment in patch form. It's simple, but slightly hacky, mainly because of the which bug on Tiger.
Assignee: nobody → kairo
Attachment #334168 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #334473 -
Flags: review?(ted.mielczarek)
Comment 7•16 years ago
|
||
Comment on attachment 334473 [details] [diff] [review] simple, slightly hacky fix Can you make that "grep -v '^no autoconf'" ?
Attachment #334473 -
Flags: review?(ted.mielczarek) → review+
Comment 8•16 years ago
|
||
change to "grep -v '^no autoconf'" as suggested in comment #7
Attachment #334473 -
Attachment is obsolete: true
Attachment #334892 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 9•16 years ago
|
||
gozer, he already has reviewed it with this patch, so I'll check it in that way, but I'm waiting for the tree to reopen ;-)
Comment 10•16 years ago
|
||
Comment on attachment 334892 [details] [diff] [review] simple, slightly hacky fix Yeah, you didn't really need to request review again, but here it is anyway.
Attachment #334892 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 11•16 years ago
|
||
Thanks, pushed as http://hg.mozilla.org/mozilla-central/index.cgi/rev/4cac63f5ffab - I hope this version sticks.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•