Remove Mac case from AUTOCONF detection code.

RESOLVED FIXED

Status

defect
RESOLVED FIXED
11 years ago
Last year

People

(Reporter: romaxa, Assigned: kairo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

11 years ago
Posted patch Bugfix (obsolete) — Splinter Review
Please check it on Mac OSX environment...
Attachment #334168 - Flags: review?(ted.mielczarek)
Assignee

Updated

11 years ago
Blocks: 450967
Comment on attachment 334168 [details] [diff] [review]
Bugfix

Looks ok to me.
Attachment #334168 - Flags: review?(ted.mielczarek) → review+
Assignee

Comment 2

11 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

11 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)
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.
Assignee

Comment 6

11 years ago
Posted patch simple, slightly hacky fix (obsolete) — Splinter Review
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 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
Attachment #334473 - Attachment is obsolete: true
Attachment #334892 - Flags: review?(ted.mielczarek)
Assignee

Comment 9

11 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 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

11 years ago
Thanks, pushed as http://hg.mozilla.org/mozilla-central/index.cgi/rev/4cac63f5ffab - I hope this version sticks.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.