Rework Windows installer for 64 bit (install into 'Program Files', write to 64 bit registry hives, etc.)

RESOLVED FIXED in Firefox 4.0b5

Status

()

RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

unspecified
Firefox 4.0b5
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 4 obsolete attachments)

4.56 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
14.71 KB, patch
Details | Diff | Splinter Review
62.49 KB, image/png
Details
27.80 KB, image/png
Details
57.95 KB, patch
rstrong
: review+
beltzner
: approval2.0+
Details | Diff | Splinter Review
58.12 KB, patch
rstrong
: review+
Details | Diff | Splinter Review
It will need to be reworked to write to the correct registry entries and likely need to perform some cleanup.

Updated

9 years ago
Blocks: 471090
NSIS provides "SetRegView 64" and "SetRegView 32" for this.  But on some situations, it may not work.  (I don't know why it doesn't work).

Comment 2

9 years ago
I am just trying an installer and I am choosing the custom route.
It tries to install the application in:
"C:\Program Files (x86)\Minefield\"
instead of:
"C:\Program Files\Minefield\"

I was able to successfully install it. I can also see it in "Windows->All Programs->Minefield"

I have also been able to uninstall and install with the standard installation.

Is there anything that the user can actually notice or is this just in the back-end of where Firefox gets registered in Windows' registry?
Depends on: 567315

Comment 3

9 years ago
I have just checked the Minefield installer used for the 64 bit-version.
The installer extracts setup.exe which is an 32-bit executable. So it will use "C:\Program Files (x86)\Minefield\" when querying the standard Programs folder.

Changing the installer to a 64 bit image would solve this problem.

An other issue are the missing MSVC runtime DLLs for 64 bit. The installer should check their presence and ask to download the "vcredist_x64.exe" from Microsoft if required.

The MSVCP100.dll is missing error message cannot be solved by most standard users, they do not know that it belongs to the MSVC runtime.
(In reply to comment #3)
> I have just checked the Minefield installer used for the 64 bit-version.
> The installer extracts setup.exe which is an 32-bit executable. So it will use
> "C:\Program Files (x86)\Minefield\" when querying the standard Programs folder.
Known... will be taken care of in this bug

> Changing the installer to a 64 bit image would solve this problem.
Not possible or necessary with NSIS... for more info lookup NSIS (nullsoft scriptable install system) on google

> An other issue are the missing MSVC runtime DLLs for 64 bit. The installer
> should check their presence and ask to download the "vcredist_x64.exe" from
> Microsoft if required.
No and this is already fixed by Bug 569268
Depends on: 570951
Duplicate of this bug: 576459
A couple of notes:

Besides the obvious changes such as installing under Program Files vs. Program File (x86) and writing to the 64 bit registry locations this bug should make it so the installer / uninstaller also performs cleanup of the 32 bit registry and the 32 bit installer should perform cleanup of the 64 bit registry in the same manner as the 32 bit installer  / uninstaller cleans up the registry for installations that no longer exist.

Not sure what we want to do if anything about an existing installations... for example, if there is a C:\Program File (x86)\Mozilla Firefox\ and a new install in C:\Program File\Mozilla Firefox\. I do think the uninstall name should differentiate itself in the name to lessen confusion... might be good to do that in a couple of other places as well.
The last two sentences of comment #7 indicates what is commented in bug #576466 comment #4. This could be made as a bug blocking this one.
(In reply to comment #8)
> The last two sentences of comment #7 indicates what is commented in bug #576466
> comment #4. This could be made as a bug blocking this one.
Bug 576466 has very little to do with this bug. The reporter of bug 568949 is running a 32 bit version of Firefox, launching a 64 bit version of Firefox, and expecting them both to work. The uninstaller issue would have needed a new bug report since it is a separate issue so it doesn't block this bug.
Duplicate of this bug: 577342
Summary: Rework Windows installer for 64 bit → Rework Windows installer for 64 bit (install into 'Program Files', write to 64 bit registry hives, etc.)
Comment on attachment 456819 [details] [diff] [review]
Part 1, use 64-bit Program Files instead of 32-bit

Instead of preprocessing common.nsh just add an NSIS !define to defines.nsi.in and use it where ever it is needed.
#ifdef HAVE_64BIT_OS
!define HAVE_64BIT_OS
#endif
Attachment #456819 - Flags: review-
Attachment #457025 - Flags: review?(robert.bugzilla)
Comment on attachment 457025 [details] [diff] [review]
Part 1, use 64-bit path instead of 32-bit path

Looks good but please don't land this until a couple of other installer fixes are ready to land. I want to check for regressions and make sure the changes don't make it more difficult to implement other changes in the future as well as fix bug 572162. Next thing that should probably be done is writing to the correct registry keys for 64 bit.
Attachment #457025 - Flags: review?(robert.bugzilla) → review+
We've hired a consultant to implement this so I'm taking the bug so no one will spend time on this only to find it is already being worked on. He has verified all plugins are behaving appropriately and has sent me a patch with everything almost completed. A little more work will be necessary for bug 572162 as well but I'll likely do that.
Assignee: nobody → robert.bugzilla
Patch sent to me by Amir who has been working on this.
Attachment #468257 [details] [diff] is mainly for reference since it contains some code we might use in the future. This patch is almost complete but it doesn't fix bug 572162.
Posted patch patch (obsolete) — Splinter Review
Attachment #468261 - Attachment is obsolete: true
Posted patch patch rev2 (obsolete) — Splinter Review
Attachment #469791 - Attachment is obsolete: true
Mike, to allow side by side installations of x86 and x64 I had to make the uninstall keys unique by adding x86 / x64. At the same time I added the locale. Instead of x86 / x64 we could go with 32 bit / 64 bit or 32-bit / 64-bit but from what I have seen the vast majority of x64 applications use x64.
Attachment #469812 - Flags: ui-review?(beltzner)
Same thing for the minimum version error.
Attachment #469813 - Flags: ui-review?(beltzner)
(In reply to comment #23)
> Created attachment 469813 [details]
> Mininum version error for x64
> 
> Same thing for the minimum version error.

Don't we support Windows XP x64 Edition (same version of Server 2003 x64)?
We might decide to but for the time being I'm going to go with Vista and above. I asked beltzner and a couple of other people and so far this is the consensus but it is simple enough to change if we decide to support XP x64.
I went over a couple of changes needed in the patch with Amir over email.
Attachment #469806 - Attachment is obsolete: true
Attachment #469975 - Flags: review+
Attachment #469975 - Flags: approval2.0?
Comment on attachment 469975 [details] [diff] [review]
patch rev3 from Amir

a=beltzner
Attachment #469975 - Flags: approval2.0? → approval2.0+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/1688aa462b49

A couple of notes:
Installing the x64 installer when there is already an x64 installation prior to this landing will not find the previous x64 installation. to work around this, please uninstall the pre-existing x64 installation and then install using an installer after this patch has landed. From this point on the installer will find the pre-existing installation as long as there is only one just as the x86 installer does.

Bug 572162 makes it so app update breaks when there is an x86 installation in c:\program files (x86)\ that has the same install directory name as an x64 installation in c:\program files\. To avoid this use a custom install directory name by performing a custom installation. For example, I use c:\program files\Minefield (x64)\

Shortcut names are the same as the x86 installation. We might change the name for nightly x64 installations to make it easier for nightly users to have both x86 and x64 installations. To workaround this, rename the x86 shortcut names and the x64 shortcut names. For example, I have Minefield (x86) and Minefield (x64) shortcuts.

We will likely want to provide the option to uninstall x86 installations when installing x64 and vice versa but that won't be implemented if at all until we are closer to releasing an official Windows x64 build which probably won't happen until the plugin story is better on Windows x64.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite-
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b5
cc'ing SeaMonkey and Thunderbird folks so they can implement this if they want. If you do, cc me to relevant bugs and I'll likely be able to provide a patch.
Duplicate of this bug: 567315
Removing litmus request since people actually installing 64 bit builds should suffice.
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.