Last Comment Bug 786886 - Force Pymake to spin up a shell for the signing command
: Force Pymake to spin up a shell for the signing command
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla18
Assigned To: Siddharth Agarwal [:sid0] (inactive)
: Siddharth Agarwal [:sid0] (inactive)
Mentors:
Depends on:
Blocks: 593585 799095
  Show dependency treegraph
 
Reported: 2012-08-29 17:03 PDT by Siddharth Agarwal [:sid0] (inactive)
Modified: 2012-10-22 20:53 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch (606 bytes, patch)
2012-08-29 17:03 PDT, Siddharth Agarwal [:sid0] (inactive)
ted: review+
gps: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Siddharth Agarwal [:sid0] (inactive) 2012-08-29 17:03:07 PDT
Created attachment 656677 [details] [diff] [review]
patch

Pymake doesn't spin up a shell for the signing command while building the installer. Since this command contains a few MSYS-style paths inherited from MOZ_SIGN_CMD (set in the environment by buildbot), things don't quite work out.

I had a look at what it would take to fix buildbot. It needs both toolsdir and basedir set to a Windows-style path. Seems like some sorts of builds do have that, some don't, and I don't want to dig in and find out why -- this is much easier.

Is there a better way to force Pymake to use a shell?

I'll take either ted's or gps's review.
Comment 1 Gregory Szorc [:gps] 2012-08-29 17:15:48 PDT
Comment on attachment 656677 [details] [diff] [review]
patch

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

The hack, it burns. I guess this will fit right in with the rest of the packaging code!
Comment 2 Siddharth Agarwal [:sid0] (inactive) 2012-08-29 17:20:10 PDT
Pushed to build-system: https://hg.mozilla.org/projects/build-system/rev/31f60d0435cd
Comment 3 Siddharth Agarwal [:sid0] (inactive) 2012-08-30 06:41:44 PDT
https://hg.mozilla.org/mozilla-central/rev/31f60d0435cd
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-09-04 06:44:16 PDT
Comment on attachment 656677 [details] [diff] [review]
patch

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

There are probably other ways to do this, but I'm not going to lose sleep over it, since hopefully this will all go away in bug 780561.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-09-04 06:44:41 PDT
Comment on attachment 656677 [details] [diff] [review]
patch

post-hoc r+
Comment 6 Mike Hommey [:glandium] 2012-09-04 06:59:08 PDT
(In reply to Ted Mielczarek [:ted] from comment #4)
> There are probably other ways to do this, but I'm not going to lose sleep
> over it, since hopefully this will all go away in bug 780561.

Note that package signature is out of scope for bug 780561. But further packager changes will make this go away.
Comment 7 Siddharth Agarwal [:sid0] (inactive) 2012-10-08 05:38:50 PDT
Comment on attachment 656677 [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. Trivial patch.
String or UUID changes made by this patch: None.
Comment 8 Alex Keybl [:akeybl] 2012-10-10 15:49:16 PDT
Comment on attachment 656677 [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 9 Ryan VanderMeulen [:RyanVM] 2012-10-22 20:53:27 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/66d8e19b22a6

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