If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 19

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

Trunk
Firefox 19
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 671221 [details] [diff] [review]
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.
Attachment #671221 - Flags: review?(mwu)

Updated

5 years ago
Component: General → General
Product: Fennec → Firefox for Android
(Assignee)

Comment 1

5 years ago
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.
(Assignee)

Updated

5 years ago
Attachment #671221 - Flags: review?(mwu)
(Assignee)

Comment 2

5 years ago
Created attachment 671254 [details] [diff] [review]
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.
Attachment #671254 - Flags: review?(mwu)
(Assignee)

Updated

5 years ago
Attachment #671221 - Attachment is obsolete: true

Comment 3

5 years ago
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+
(Assignee)

Comment 4

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.