Closed Bug 450948 Opened 12 years ago Closed 12 years ago
Remove Mac case from AUTOCONF detection code
Please check it on Mac OSX environment...
Attachment #334168 - Flags: review?(ted.mielczarek)
Comment on attachment 334168 [details] [diff] [review] Bugfix Looks ok to me.
Attachment #334168 - Flags: review?(ted.mielczarek) → review+
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.
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)
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
Feels like overkill, especially since this will be executed every time client.mk is run. I'd be fine with Kairo's suggestion there.
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.
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+
change to "grep -v '^no autoconf'" as suggested in comment #7
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 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+
Thanks, pushed as http://hg.mozilla.org/mozilla-central/index.cgi/rev/4cac63f5ffab - I hope this version sticks.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.