Closed Bug 801418 Opened 9 years ago Closed 9 years ago

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


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)

Firefox 19


(Reporter: jfkthame, Assigned: jfkthame)



(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:
Target Milestone: --- → Firefox 19
Closed: 9 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.