MSYS mangles the signtool shlibsign cmd with Pymake because it begins with a space

RESOLVED FIXED in Firefox 17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sid0, Unassigned)

Tracking

Trunk
mozilla18
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 fixed, firefox18 fixed)

Details

(Whiteboard: [god-dammit-gnu-make])

Attachments

(1 attachment)

Created attachment 656558 [details] [diff] [review]
patch

I wish I were kidding.
Attachment #656558 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 1

5 years ago
Checked into build-system pending review: http://hg.mozilla.org/projects/build-system/rev/c85bc58f10ba

Comment 2

5 years ago
Comment on attachment 656558 [details] [diff] [review]
patch

Review of attachment 656558 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine. But, it's only a band-aid. I think a better fix would be for pymake to strip() the first argument. I'm pretty sure there is never a legitimate scenario where the command name or the first argument after |sh -c| would have leading or trailing whitespace.
Attachment #656558 - Flags: review?(ted.mielczarek) → review+
(Reporter)

Comment 3

5 years ago
Pymake actually does strip the first argument. This is somewhere deep in the middle of a command, with $(SIGN_CMD) enclosed in quotes and passed as a parameter to a Python command.

Comment 4

5 years ago
We need a whiteboard annotation for bugs caused by the stupidity that is make.
Whiteboard: [god-dammit-gnu-make]
(Reporter)

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c85bc58f10ba
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Reporter)

Comment 6

5 years ago
Comment on attachment 656558 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: we'll still need to keep using GNU make for a year from now
Testing completed (on m-c, etc.): Yes
Risk to taking this patch (and alternatives if risky): Zero risk.
String or UUID changes made by this patch: None
Attachment #656558 - Flags: approval-mozilla-aurora?
(Reporter)

Updated

5 years ago
Blocks: 799095

Updated

5 years ago
Attachment #656558 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?

Comment 7

5 years ago
Comment on attachment 656558 [details] [diff] [review]
patch

[Triage Comment]
Very low risk, Callek let us know that if anything seems fishy we can easily back this out.
Attachment #656558 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/a7b4aedef3fa
status-firefox17: --- → fixed
status-firefox18: --- → fixed
You need to log in before you can comment on or make changes to this bug.