Closed Bug 494576 Opened 13 years ago Closed 13 years ago

Make ident generates wrong output on Mac

Categories

(MailNews Core :: Build Config, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(1 file, 1 obsolete file)

It's almost unbelievable, but the current output of "make ident" for comm-central apps on Mac is e.g.

-n comm_revision 
1a25ea0c8358
-n moz_revision 
a7eb03446bed

while other platforms output

comm_revision 1a25ea0c8358
moz_revision a7eb03446bed

Digging around what's up there, |man echo| on a mac slave told me "[...] the builtin echo in sh(1) does not accept the -n option." and |man builtin| told me that the builtin version is not used when a / is in the command name, e.g. /bin/echo - which is the version that actually supports the -n option.

As we can trust that (our) Macs have that external echo command in /bin/ we should defer to that on Mac. Patch upcoming. Sigh.
Here is a patch that makes us use the echo -n from /bin/ on mac while using standard echo -n for the other platforms. It sucks somewhat, but IMHO it's the easiest way to do this correctly.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #379336 - Flags: review?
Comment on attachment 379336 [details] [diff] [review]
create a var for echo -n that is different on Mac

I'd like Axel to look at this as well as a comm-central buildsystem peer.
Attachment #379336 - Flags: review?(l10n)
Attachment #379336 - Flags: review?(dmose)
Attachment #379336 - Flags: review?
Ted, do you have an alternative suggestion? Does make come with a good know echo?

Or we would set SHELL to /bin/bash in the makefile for osx.

Not sure if I'd prefer a configure test and a $(ECHO) variable. Depends on how often we could hit this.

CCing Armen, too, he's about to trigger this for Firefox, too.

Note, this code is copied from fennec, where we don't have the problem because that isn't repacked on OSX
Use printf instead of echo. printf is completely portable
Attachment #379336 - Flags: review?(l10n) → review-
Comment on attachment 379336 [details] [diff] [review]
create a var for echo -n that is different on Mac

Thanks Benjamin, good idea. r- based on that. I'll do a follow up patch for fennec just to make things cool.

KaiRo, do you want to sneak in BuildID right away?
Here's a variant that uses printf and prints build_id as well.
Attachment #379336 - Attachment is obsolete: true
Attachment #379353 - Flags: review?(l10n)
Attachment #379336 - Flags: review?(dmose)
Attachment #379353 - Flags: review?(l10n) → review+
Pushed as http://hg.mozilla.org/comm-central/rev/e0164ae0561f
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b3
Depends on: 496196
Flags: in-testsuite-
No longer blocks: 547518
You need to log in before you can comment on or make changes to this bug.