Closed Bug 801418 Opened 8 years ago Closed 8 years ago

mobile 'make package' generates a bunch of sed errors; fails to strip comment lines from properties files

Categories

(Firefox for Android :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 19

People

(Reporter: jfkthame, Assigned: jfkthame)

Details

Attachments

(1 file, 1 obsolete file)

During "make package" for fennec (Android) on OS X, I'm seeing a series of error messages from sed, as below:

cd ../../../dist/fennec && find chrome -type f -name *.properties -exec sed -i '/^#/d' {} \;
sed: 1: "chrome/chrome/content/l ...": command c expects \ followed by text
sed: 1: "chrome/en-US/locale/bra ...": command c expects \ followed by text
sed: 1: "chrome/en-US/locale/en- ...": command c expects \ followed by text
sed: 1: "chrome/en-US/locale/en- ...": command c expects \ followed by text
...and many more.

It appears that sed is interpreting '/^#/d' as the backup-file extension for the -i option, and then interpreting the intended target file from the find command's "{}" as a sed command line, so it's failing to actually remove the comment lines as intended.

The attached patch provides an explicit empty backup-file extension, so that the sed arguments are interpreted as intended.
Attachment #671221 - Flags: review?(mwu)
Product: Fennec → Firefox for Android
Hmm, the patch here resolves the problem when building on OS X, but I don't think it'll work on Linux; it looks like sed there does not allow a space before the (optional) file extension, whereas the BSD-ish version on OS X requires a space and the extension. Sigh.
Attachment #671221 - Flags: review?(mwu)
How about replacing the sed command with perl? That should have a more consistent command line syntax, I hope.
Attachment #671254 - Flags: review?(mwu)
Attachment #671221 - Attachment is obsolete: true
Comment on attachment 671254 [details] [diff] [review]
fix sed errors generated by MOZ_POST_STAGING_COMMAND during 'make package' for mobile

Looks ok. BTW, we have a $(PERL) variable - that should probably be used if it works.
Attachment #671254 - Flags: review?(mwu) → review+
Yup, $(PERL) looks good - pushed with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb4344e9d628
Target Milestone: --- → Firefox 19
https://hg.mozilla.org/mozilla-central/rev/cb4344e9d628
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.