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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: patch, provide an empty argument to the sed -i option

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.

Comment 1

[Jonathan Kew [:jfkthame]]

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.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: fix sed errors generated by MOZ_POST_STAGING_COMMAND during 'make package' for mobile

How about replacing the sed command with perl? That should have a more consistent command line syntax, I hope.

Comment 3

[Michael Wu [:mwu]]

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.

Comment 4

[Jonathan Kew [:jfkthame]]

Yup, $(PERL) looks good - pushed with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb4344e9d628

Comment 5

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/cb4344e9d628