Closed Bug 598647 Opened 14 years ago Closed 4 years ago

Implement MSI installer

Categories

(Firefox :: Installer, enhancement, P3)

x86
Windows 7
enhancement

Tracking

()

RESOLVED DUPLICATE of bug 231062

People

(Reporter: khuey, Unassigned)

References

Details

Attachments

(21 files, 3 obsolete files)

3.34 KB, patch
Mitch
: review+
Details | Diff | Splinter Review
12.40 KB, patch
khuey
: review+
Details | Diff | Splinter Review
549 bytes, patch
khuey
: review+
Details | Diff | Splinter Review
985 bytes, patch
khuey
: review+
Details | Diff | Splinter Review
12.60 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
1.67 KB, patch
khuey
: review+
Details | Diff | Splinter Review
5.78 KB, patch
khuey
: review+
Details | Diff | Splinter Review
864 bytes, patch
khuey
: review+
Details | Diff | Splinter Review
4.24 KB, patch
khuey
: review+
Details | Diff | Splinter Review
2.25 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
2.00 KB, patch
Mitch
: review+
Details | Diff | Splinter Review
7.30 KB, patch
khuey
: review+
Details | Diff | Splinter Review
3.70 KB, patch
khuey
: review+
Details | Diff | Splinter Review
3.35 KB, patch
ted
: review+
Details | Diff | Splinter Review
4.19 KB, patch
Pike
: review+
robert.strong.bugs
: feedback+
Details | Diff | Splinter Review
1.01 KB, patch
khuey
: review+
Details | Diff | Splinter Review
2.40 KB, patch
Mitch
: review+
Details | Diff | Splinter Review
6.84 KB, patch
khuey
: review+
Details | Diff | Splinter Review
1.10 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
1.29 KB, patch
robert.strong.bugs
: review+
nthomas
: feedback+
Details | Diff | Splinter Review
742 bytes, patch
ted
: review+
Details | Diff | Splinter Review
When I have a go at this again soon I'm going to use this bug rather than the ridiculously long/noisy Bug 231062.
Parts of this have been previously reviewed, but it's changed enough that I think we should start from scratch on this one.
Attachment #487082 - Flags: review?
Attachment #487082 - Flags: review? → review?(robert.bugzilla)
Attachment #487082 - Flags: feedback?(mitchell.field)
Attachment #487077 - Flags: review?(mitchell.field) → review+
Attachment #487098 - Flags: review?(mitchell.field) → review+
Attachment #487097 - Flags: review? → review?(robert.bugzilla)
Attached patch Part 12: Add localized shorcuts (obsolete) — Splinter Review
This patch has r=Mitch,ted,Pike in Bug 231062.
Attachment #487149 - Flags: review+
Attachment #487152 - Flags: review? → review?(robert.bugzilla)
Attachment #487161 - Flags: review?(community) → review?(l10n)
Adds an 'msiexec' command to the updater that lets us pass an MSI file out to Windows Installer.
Attachment #487165 - Flags: review?(robert.bugzilla)
I have a few concerns with making the updater binary part of the msi process. A few of the concerns off the top of my head are.

1. the updater requires that Firefox is not running and MSI's don't have that requirement.
2. It adds complexity to the updater which is unnecessary.
3. releng packaging will require creating the msi then creating a mar containing the msi. Though minor it is an additional step that should be unnecessary.
4. it locks us in to only applying the msi when the app is restarted which should not be necessary with an msi.

We already use the update service to distribute apk files for Android and I would think it would be possible to do the same for msi / msp files.
OK, I'll take a look at what we do for Android.
Attachment #487174 - Flags: review?(mitchell.field) → review+
Attachment #487161 - Flags: review?(l10n) → review+
Comment on attachment 487153 [details] [diff] [review]
Part 16: Upload completed MSI

>diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk
>--- a/toolkit/mozapps/installer/packager.mk
>+++ b/toolkit/mozapps/installer/packager.mk
>@@ -669,16 +669,17 @@ QUOTED_WILDCARD = $(if $(wildcard $(subs
> CHECKSUM_ALGORITHM = 'sha512'
> 
> # This variable defines where the checksum file will be located
> CHECKSUM_FILE = "$(DIST)/$(PKG_PATH)/$(PKG_BASENAME).checksums"
> 
> UPLOAD_FILES= \
>   $(call QUOTED_WILDCARD,$(DIST)/$(PACKAGE)) \
>   $(call QUOTED_WILDCARD,$(INSTALLER_PACKAGE)) \
>+  $(call QUOTED_WILDCARD,$(DIST)/$(PKG_BASENAME).msi) \

If you use this in more than one place you might want to define MSI_PACKAGE or something like that.
Attachment #487153 - Flags: review?(ted.mielczarek) → review+
Attachment #487082 - Flags: review?(robert.bugzilla) → review+
Attachment #487097 - Flags: review?(robert.bugzilla) → review+
Attachment #487165 - Flags: review?(robert.bugzilla) → review-
Comment on attachment 487152 [details] [diff] [review]
Part 15: Registry/shell integration

Note: we will likely be removing the program directory from the start menu and only having a single shortcut in the root of the start menu. It will probably be easier if the msi did this from day one so cleanup doesn't have to happen when the change is made. There are a couple of bugs that need to be fixed for this to happen though.
Attachment #487152 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 487152 [details] [diff] [review]
Part 15: Registry/shell integration

btw: you named this patch "Part 15: Registry/shell integration" when all it really does is add the shortcuts. Is there already a patch for "Registry/shell integration"?
Attachment #487161 - Flags: feedback?(robert.bugzilla) → feedback+
(In reply to comment #25)
> Comment on attachment 487152 [details] [diff] [review]
> Part 15: Registry/shell integration
> 
> btw: you named this patch "Part 15: Registry/shell integration" when all it
> really does is add the shortcuts. Is there already a patch for "Registry/shell
> integration"?

Bah, I uploaded Part 12 as Part 12 and Part 15.
Comment on attachment 487152 [details] [diff] [review]
Part 15: Registry/shell integration

Minusing patch 15 due to comment #26 and comment #24 applies to patch 12 attachment #487149 [details] [diff] [review]
Attachment #487152 - Flags: review+ → review-
Regarding the Registry/shell integration we are planning on removing dde support in bug 491947. The one hassle is Win2K. I think we shouldn't support MSI's with Win2K and just disable it for MSI's.
Updated to the Firefox 4 behavior of one toplevel shortcut.
Attachment #487149 - Attachment is obsolete: true
Attachment #521666 - Flags: review+
The custom action DLL cannot rely on an appropriate version of the CRT already being installed.
Attachment #521668 - Flags: review?
Attachment #521668 - Flags: review? → review?(robert.bugzilla)
Asking for nthomas for feedback on the approach here, not the actual code.

There are two ways to do "partial" MSI updates in WiX.  You can generate the updates from a pair of the .msi packages themselves, or from the .wixpdb files that the toolset outputs.  If we use the .msi files, we have to do administrative installations to unpack them to generate the partials.  If we use the .wixpdbs we can avoid that, at the cost of having to have another type of file around.

It's basically a tradeoff between code complexity if we use .msis and some disk space on ftp if we use the .wixpdbs.  I'd like to use the wixpdbs, but I'd like nthomas's feedback on this.

If we use the wixpdbs, we need to bind the files into them (see http://windows-installer-xml-wix-toolset.687559.n2.nabble.com/error-PYRO0103-The-system-cannot-find-the-file-UI-Icons-appicon-ico-if-appicon-ico-is-in-my-latest-wb-td4600799.html for the type of error we hit otherwise)
Attachment #521675 - Flags: review?(robert.bugzilla)
Attachment #521675 - Flags: feedback?(nrthomas)
If we go with the wixpdb method, we'll need to upload the wixpdbs.
Attachment #521678 - Flags: review?(ted.mielczarek)
Comment on attachment 521678 [details] [diff] [review]
Part 23: Upload the wixpdb

Fine with me.
Attachment #521678 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 521675 [details] [diff] [review]
Part 22: Bind files into the WiX output file

So we would need to retain the .wixpdb's between releases but not actually ship them to end users ? If so, we can keep them in firefox/nightly/$version-candidates/ on the ftp server, and avoid the mirrors having to carry that weight in firefox/releases/. That would be fine so long as the .wixpdb's are reasonably sized, so how large are they, and the msi's ?

Something we'll need to consider is signing the windows binaries for releases. The current process is 
 * build en-US, package
 * repack locales, package
 * sign files inside the installers and complete mars, and the installers themselves
 * generate partial mars
So we'd need to adjust the signing scripts to unpack the files in a msi, sign them or replace with signed files, and put them back together. Possibly sign the msi, and generate the partial msi updates afterwards. Bug 509158 may change that process, making signed bits available on the build machine - probably life much simpler for you - but there's isn't a timeline there yet.
Attachment #521675 - Flags: feedback?(nrthomas) → feedback+
Comment on attachment 521675 [details] [diff] [review]
Part 22: Bind files into the WiX output file

Oops, setting feedback? again until khuey answers.
Attachment #521675 - Flags: feedback+ → feedback?(nrthomas)
The MSI installer itself is ~25MB uncompressed (that'll drop significantly once we turn on compression) and the wixpdb is about 12-13 MB.  Note that we only have to keep the wixpdbs around for any binary we want to create a partial update to/from, we can always create full updates (the MSI itself is the full update)

(In reply to comment #34)
> Something we'll need to consider is signing the windows binaries for releases.
> The current process is 
>  * build en-US, package
>  * repack locales, package
>  * sign files inside the installers and complete mars, and the installers
> themselves
>  * generate partial mars
> So we'd need to adjust the signing scripts to unpack the files in a msi, sign
> them or replace with signed files, and put them back together. Possibly sign
> the msi, and generate the partial msi updates afterwards. Bug 509158 may change
> that process, making signed bits available on the build machine - probably life
> much simpler for you - but there's isn't a timeline there yet.

Yeah catlee and I have been talking about this.  He's rewriting a good chunk of our packaging code as part of that effort.
Attachment #521668 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 521675 [details] [diff] [review]
Part 22: Bind files into the WiX output file

I think wixpdb would be the way to go
Attachment #521675 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 521675 [details] [diff] [review]
Part 22: Bind files into the WiX output file

Another GB per set of release builds isn't going to break the bank.
Attachment #521675 - Flags: feedback?(nrthomas) → feedback+
Assignee: khuey → nobody
Status: ASSIGNED → NEW
Attachment #487082 - Flags: feedback?(mitchell.field)
Priority: -- → P3

Can we close this?

Flags: needinfo?(mhowell)

Yes.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(mhowell)
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.