Last Comment Bug 786791 - MSYS mangles the signtool shlibsign cmd with Pymake because it begins with a space
: MSYS mangles the signtool shlibsign cmd with Pymake because it begins with a ...
Status: RESOLVED FIXED
[god-dammit-gnu-make]
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla18
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 593585 799095
  Show dependency treegraph
 
Reported: 2012-08-29 12:45 PDT by Siddharth Agarwal [:sid0] (inactive)
Modified: 2012-10-22 20:53 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch (1.23 KB, patch)
2012-08-29 12:45 PDT, Siddharth Agarwal [:sid0] (inactive)
gps: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Siddharth Agarwal [:sid0] (inactive) 2012-08-29 12:45:20 PDT
Created attachment 656558 [details] [diff] [review]
patch

I wish I were kidding.
Comment 1 Siddharth Agarwal [:sid0] (inactive) 2012-08-29 12:51:44 PDT
Checked into build-system pending review: http://hg.mozilla.org/projects/build-system/rev/c85bc58f10ba
Comment 2 Gregory Szorc [:gps] 2012-08-29 13:29:19 PDT
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.
Comment 3 Siddharth Agarwal [:sid0] (inactive) 2012-08-29 13:39:44 PDT
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 Gregory Szorc [:gps] 2012-08-29 13:42:32 PDT
We need a whiteboard annotation for bugs caused by the stupidity that is make.
Comment 5 Siddharth Agarwal [:sid0] (inactive) 2012-08-30 06:41:54 PDT
https://hg.mozilla.org/mozilla-central/rev/c85bc58f10ba
Comment 6 Siddharth Agarwal [:sid0] (inactive) 2012-10-08 05:41:06 PDT
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
Comment 7 Alex Keybl [:akeybl] 2012-10-10 15:49:08 PDT
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.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-10-22 20:53:43 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/a7b4aedef3fa

Note You need to log in before you can comment on or make changes to this bug.