Closed Bug 786886 Opened 8 years ago Closed 8 years ago

Force Pymake to spin up a shell for the signing command

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(firefox17 fixed, firefox18 fixed)

RESOLVED FIXED
mozilla18
Tracking Status
firefox17 --- fixed
firefox18 --- fixed

People

(Reporter: rain1, Assigned: rain1)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #656677 - Flags: review?(ted.mielczarek)
Attachment #656677 - Flags: review?(gps)
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!
Attachment #656677 - Flags: review?(gps) → review+
Pushed to build-system: https://hg.mozilla.org/projects/build-system/rev/31f60d0435cd
Status: NEW → ASSIGNED
QA Contact: sagarwal
https://hg.mozilla.org/mozilla-central/rev/31f60d0435cd
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
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.
Assignee: nobody → sagarwal
Comment on attachment 656677 [details] [diff] [review]
patch

post-hoc r+
Attachment #656677 - Flags: review?(ted.mielczarek) → review+
(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 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.
Attachment #656677 - Flags: approval-mozilla-aurora?
Blocks: 799095
Attachment #656677 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
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.
Attachment #656677 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.