port bug 568949 (64-bit installer fix) to Seamonkey

RESOLVED FIXED in seamonkey2.12

Status

SeaMonkey
Installer
--
enhancement
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: m_kato, Assigned: Atsushi Okawa)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.12
x86_64
Windows Vista
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Although comm-central supports Win64 build, Seamonkey installer doesn't consider 64-bit package.
(Assignee)

Comment 1

7 years ago
Created attachment 569005 [details]
to make 64-bit installer

This patch is the fix similar to the Firefox one.
(Reporter)

Comment 2

7 years ago
(In reply to Atsushi Okawa from comment #1)
> Created attachment 569005 [details]
> to make 64-bit installer
> 
> This patch is the fix similar to the Firefox one.

Okawa-san, could you create a new patch for comm-central instead of comm-beta?

Win64 fix should not be landed to aurora and beta.
(Assignee)

Comment 3

7 years ago
Created attachment 569060 [details] [diff] [review]
to make 64-bit installer 2

Kato-san
>Win64 fix should not be landed to aurora and beta.
I'm sorry. I don't understand it enough.

I make the patch for comm-central.
Attachment #569005 - Attachment is obsolete: true
(Reporter)

Updated

7 years ago
Attachment #569060 - Flags: review?(bugspam.Callek)
Comment on attachment 569060 [details] [diff] [review]
to make 64-bit installer 2

I'm behind on reviews, doing some catchup today, but since win64 is low priority for me atm, I'm deferring this for a while, I likely won't get to it until early January (sorry)
Comment on attachment 569060 [details] [diff] [review]
to make 64-bit installer 2

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

I think this looks good, feedback requesting from rstrong just to be sure I am not missing some obvious gotchya in this code.

::: suite/installer/windows/nsis/installer.nsi
@@ -1037,5 @@
> -  IntOp $R8 $R5 + $R6
> -  ; Add 1024 Kb to the diskspace requirement since the installer makes a copy
> -  ; of the MAPI dll's (around 20 Kb)... also, see Bug 434338.
> -  IntOp $R8 $R8 + 1024
> -  SectionSetSize ${APP_IDX} $R8

Please change this section Size stuff to keep that comment and do:
IntOp $R5 $R5 + 1024
Attachment #569060 - Flags: review?(bugspam.Callek)
Attachment #569060 - Flags: review+
Attachment #569060 - Flags: feedback?(robert.bugzilla)
(Assignee)

Comment 6

7 years ago
Created attachment 584592 [details] [diff] [review]
the patch to make 64bit installer

(In reply to Justin Wood (:Callek) from comment #5)
I made a patch and upload it. I fixed the mistake of previous patch you pointed out.
Attachment #569060 - Attachment is obsolete: true
Attachment #569060 - Flags: feedback?(robert.bugzilla)

Comment 7

6 years ago
Am I guess right, and you forget to set review request flag after remaking patch?

Comment 8

6 years ago
Comment on attachment 584592 [details] [diff] [review]
the patch to make 64bit installer

> Am I guess right, and you forget to set review request flag after remaking
> patch?
Hmm. First Patch. He might not have editBugs permissions. I'll set it for him.

Thank you very much Atsushi-san!
Attachment #584592 - Flags: review?(bugspam.Callek)

Updated

6 years ago
Assignee: installer → htguard

Comment 9

6 years ago
Comment on attachment 584592 [details] [diff] [review]
the patch to make 64bit installer

Callek said that mcsmurf would also be a suitable reviewer. So whoever gets to this first...
Attachment #584592 - Flags: review?(bugzilla)
Comment on attachment 584592 [details] [diff] [review]
the patch to make 64bit installer

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

sorry for the delay here.
Attachment #584592 - Flags: review?(bugzilla)
Attachment #584592 - Flags: review?(bugspam.Callek)
Attachment #584592 - Flags: review+

Updated

6 years ago
Keywords: checkin-needed
Landed with some bitrot fixing.
http://hg.mozilla.org/comm-central/rev/c30bddba3513

Thanks for the patch, Atsushi-san! One request - to make life easier for those checking in patches on your behalf, please follow the directions below for any future patches you submit. Thanks again!
https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12
(Assignee)

Comment 12

6 years ago
I'm sorry for it.I don't know how to check in c-c.I will read the page.
Not a problem!
You need to log in before you can comment on or make changes to this bug.