474530 - WinCE Installer Needed

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[John Wolfe (:wolfe)]

Attachment: v1.0 patch

A CAB installer for FENNEC is needed.

Later, a MSI version of this CAB installer should be created - but for now a simple CAB installer would be great!

Several files are duplicated within the fennec mobile dist tree.  In order to make a CAB installer, one of those duplicated file pairs needs to be renamed, then the CABWIZ.EXE INF file needs to handle renaming the now-renamed second file of the file pair -- so the final on-device installed fennec directory has the appropriately named files in the correct places.

This means a matching between the contents of the new FENNEC.inf file and the mobile/installer/Makefile.in within the WINCE section.

Also, I modified the mobile/installer/Makefile.in to define PACKAGER_NO_LIBS for WinCE so I can define my own LIBS to automatically generate the fennec.CAB file during mobile install directory's build (lib phase, of course).

NOTE: A working CAB installer depends upon the copy corruption of mozce_shunt.dll to be fixed!

Comment 2

[John Wolfe (:wolfe)]

Comment on attachment 357898 [details] [diff] [review]
v1.0 patch

blassey suggested that this should be reviewed by ted.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 357898 [details] [diff] [review]
v1.0 patch

drive by comments

> ../../dist/$(MOZ_APP_NAME)/xulrunner/mozce_shunt.dll 

You can use $(DIST) in Makefiles to point to the objdir/dist folder.

> +Shortcuts   = 0, %CE14%             ; \Windows\Start Menu\Programs\Games

Games?

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 357898 [details] [diff] [review]
v1.0 patch

So, I think this is the wrong approach. What you ought to do is edit packager.mk, and up top, add a new condition for WinCE to define a MOZ_PKG_FORMAT = CAB, and then below that, define the various variables like MAKE_PACKAGE to the correct commands if MOZ_PKG_FORMAT is CAB. If the set of commands is going to be complicated, you can write an accessory script and call that, like the DMG case does with pkg-dmg. If you do that correctly, a simple "make package" in the installer/ dir should get you a .cab out the other end.

Some other notes:
+# NOTE: This does not work because the python processing changes ALL
+#       CR-LF pairs into LF, which causes CABWIZ.EXE to fail!!!

That sounds broken. Does this patch actually work?

+; Duplicated Filenames ==> One of the two files 
+;   needs renaming before packaging up the CAB

CAB files can't have two files with the different name in different subdirectories? That's pretty...horrid.

+[SourceDisksFiles]                  ; list of files to be included in .cab

I think ideally we'd have a packages-static file (like we do for Firefox), and generate this list via preprocessing somehow.

Comment 5

[John Wolfe (:wolfe)]

Attachment: v2.0 patch

So, I think this is the wrong approach. What you ought to do is edit
  packager.mk, and up top, add a new condition for WinCE to define a
  MOZ_PKG_FORMAT = CAB, and then below that, define the various variables like
  MAKE_PACKAGE to the correct commands if MOZ_PKG_FORMAT is CAB. If the set of
  commands is going to be complicated, you can write an accessory script and 
  call that, like the DMG case does with pkg-dmg. If you do that correctly, a 
  simple "make package" in the installer/ dir should get you a .cab out the 
  other end.

DONE - and darned if that was not a great suggestion!

3 files touched: 

toolkit/mozapps/installer/packager.mk - changes to support CAB files for WinCE

build/package/wince/fennec.inf        - New File
build/package/wince/make-cab-package  - New File


    Some other notes:
    +# NOTE: This does not work because the python processing changes ALL
    +#       CR-LF pairs into LF, which causes CABWIZ.EXE to fail!!!

  That sounds broken. Does this patch actually work?

The patch works.  The section listed as causing CABWIZ.EXE to fail - that section is not used in the v1.0 patch.  With v2.0 patch, I no longer need to modify the mobile/installer/Makefile.in file, and so those changes have been removed.

    +; Duplicated Filenames ==> One of the two files 
    +;   needs renaming before packaging up the CAB

  CAB files can't have two files with the different name in different
  subdirectories? That's pretty...horrid.

Not quite right.  CAB files do not do well with two source files which have the same name.  That is, two files of different sizes with the same filename.  Turns out that CABWIZ.EXE is not very smart - and will cause at least one of the files to be the incorrect version.  

The easiest fix for this issue is to rename one of the two source files (in the fennec directory, in this case). Then, we have the CABWIZ.EXE's .INF file copy the renamed second file to the original (duplicated) filename.  This is a very confusing way of saying that the .INF file has to have each source file name be unique, and can rename any source file's name when coping the source file into it's installed location.

    +[SourceDisksFiles]                  ; list of files to be included in .cab

  I think ideally we'd have a packages-static file (like we do for Firefox), 
  and generate this list via preprocessing somehow.

A big issue to deal with is the required layout of the CABWIZ.EXE's INF file.  This file requires a lot of knowledge of source files, source file locations, and directory structures.  Each filename is located at least 3 separate places, and duplicated files have to have a renamed filename in two of those 3 places - with the 3rd place having the original filename.

All told, I could spend a number of days trying to program the production of this INF file - or I could hard-code things for now, and file a bug against the hard-coded INF file.

I am strongly leaning toward the latter.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 358521 [details] [diff] [review]
v2.0 patch

+++ b/build/package/wince/fennec.inf

Don't put this file here. This file needs to live in the mobile-browser repo. You can make the bits in packager.mk depend on a variable being set, then stick the file in mobile-browser/installer/wince or something, and then the mobile-browser/installer/Makefile.in can have like:
MOZ_PKG_CAB_INF := $(srcdir)/wince/fennec.inf

Make sense? You can see a similar example here in Firefox's installer Makefile for the mac DMG package:
http://mxr.mozilla.org/mozilla-central/source/browser/installer/Makefile.in#101

+++ b/build/package/wince/make-cab-package
+echo cp $MOZ_PKG_DIR/xulrunner/mozce_shunt.dll                                     $MOZ_PKG_DIR/xulrunner/mozce_shunt_2.dll
+cp $MOZ_PKG_DIR/xulrunner/mozce_shunt.dll

So, I don't want files that live in mozilla-central/build/package to have hard-coded lists of files like this. Can you change this to take another file as a parameter, that we could keep in the mobile-browser repo, that would just be a mapping of filenames that need to be renamed/copied? Then this script could read that in and rename/copy files in the staging directory as appropriate.

+echo rm $MOZ_PKG_DIR/xulrunner/mozce_shunt_2.dll

I don't think you should have to clean up files in $MOZ_PKG_DIR, it's just a staging dir and should get wiped out anytime you do a "make package" anyway.

+++ b/build/package/wince/make-cab-package
+UNMAKE_PACKAGE	= $(UNZIP) $(UNPACKAGE)

Does info-zip actually know how to unpack a CAB file?

Still r-, but a lot closer this time.

Comment 9

[John Wolfe (:wolfe)]

Attachment: v3.0a patch (mobile-browser repository patch)

(From update of attachment 358521 [details] [diff] [review])
  +++ b/build/package/wince/fennec.inf
  
  Don't put this file here. This file needs to live in the mobile-browser repo.
  You can make the bits in packager.mk depend on a variable being set, then 
  stick the file in mobile-browser/installer/wince or something, and then the
  mobile-browser/installer/Makefile.in can have like:
  MOZ_PKG_CAB_INF := $(srcdir)/wince/fennec.inf
  
  Make sense? You can see a similar example here in Firefox's installer 
  Makefile for the mac DMG package:
  http://mxr.mozilla.org/mozilla-central/.../installer/Makefile.in#101
  
  +++ b/build/package/wince/make-cab-package
  +echo cp $MOZ_PKG_DIR/xulrunne/mozce_shunt.dll                                
    $MOZ_PKG_DIR/xulrunner/mozce_shunt_2.dll
  +cp $MOZ_PKG_DIR/xulrunner/mozce_shunt.dll

  So, I don't want files that live in mozilla-central/build/package to have
  hard-coded lists of files like this. Can you change this to take another file
  as a parameter, that we could keep in the mobile-browser repo, that would just
  be a mapping of filenames that need to be renamed/copied? Then this script
  could read that in and rename/copy files in the staging directory as
  appropriate.
  
Moved WinMobile-specific packaging files into mobile-browser/installer/wince.

Created two new environment variables to be set for WINCE builds:

MOZ_PKG_CAB_SCRIPT - fully qualified path to CAB creating script

MOZ_PKG_CAB_INF - fully qualified path to CABWIZ INF file for creating CAB

These two files are now located within mobile-browser/installer/wince.

The CAB file is currently hard-coded with file renames, as is the CAB creating script.  However, since the two files are now effectively out of mozilla-central/build/package - can I squeak by your review?

I know it is not perfect, but can we file a bug against moving the renamed files out to a script, as well as generating the CABWIZ.inf file dynamically from a distribution directory?

  
  +echo rm $MOZ_PKG_DIR/xulrunner/mozce_shunt_2.dll
  
  I don't think you should have to clean up files in $MOZ_PKG_DIR, it's just a
  staging dir and should get wiped out anytime you do a "make package" anyway.

DONE

  +++ b/build/package/wince/make-cab-package
  +UNMAKE_PACKAGE    = $(UNZIP) $(UNPACKAGE)
  
  Does info-zip actually know how to unpack a CAB file?

YES - since the CAB file format is just a ZIP file with some special files inside - _setup.xml, setup.dll (optional).

  Still r-, but a lot closer this time.

Please?  Please?  

By the way - there are two attachments for this bug now, and both must be applied.  This attachment is for files located in the mobile-browser repository.

I have marked this patch as binary to preserve the CR-LF line endings inside the FENNEC.INF file, since bugzilla seems to love to strip them out.  Otherwise, this patch is a text file.  If someone resets the content type of this patch to text, and the downloaded patch causes CABWIZ.EXE to mess up - try putting ^M (Control-M, also known as CR) at the end of each line of FENNEC.INF and running make inside the OBJDIR/mobile/mobile/installer directory.

Comment 10

[John Wolfe (:wolfe)]

Attachment: v3.0b patch (mozilla-central patch)

Part II of the two patches, this patch is for mozilla-central.  This patch only adds new processing for WINCE builds which allows for packaging CAB files via a script and INF file which are to be contained within the mobile-browser repository.

Comment 11

[(not currently active) Ted Mielczarek]

Just for future reference, when quoting previous comments, it's a bit easier to read if you use bugzilla's "reply" link on the comment header. (It will quote the comment with right angle brackets.)

Comment 12

[John Wolfe (:wolfe)]

Attachment: v4.0a patch - mobile-browser repository

Moves installed FENNEC shortcut from Start Menu | Programs | Games to now be located in Start Menu | Programs.

Otherwise, all else is the same as v3.0a patch.

Again, this patch is uploaded as binary to preserve the CR-LF pairs at the end of each line of the FENNEC.INF file.

P.S. - That REPLY link is fantastic!

Comment 13

[John Wolfe (:wolfe)]

ted, I did not move the list of files out to a separate python directory-parsing script with either (a) a list of exception files in a separate text file, or (b) figuring out duplicate filenames.

This work is fairly complex.  Since the INF file is already in existence, and we are trying to get an ALPHA build out, I propose the following:

We file a bug against the WinCE INF file.  The next time changes to the WinCE build breaks the INF file, we do the work to make the python script for building the INF based upon the dist dynamically-created tree which serves as input to the CAB making process.

What do you think?

Comment 14

[(not currently active) Ted Mielczarek]

Comment on attachment 359657 [details] [diff] [review]
v3.0b patch (mozilla-central patch)

Looks good, can you just stick inside this block:
+ifeq ($(MOZ_PKG_FORMAT),CAB)
MOZ_PKG_CAB_INF ?= $(error MOZ_PKG_CAB_INF not specified)

so that if that variable isn't set it will error out.

Comment 15

[(not currently active) Ted Mielczarek]

Comment on attachment 359862 [details] [diff] [review]
v4.0a patch - mobile-browser repository

Ok, on the condition that you file a followup bug on not having those hardcoded filenames in the script. (Having them hardcoded in the .inf is ok, I'd just like to have a generic "make cab file given a list of files" script that lives in mozilla-central.

Comment 16

[John Wolfe (:wolfe)]

Attachment: v3.1b patch (mozilla-central patch)

Added error reporting for missing MOZ_PKG_CAB_INF and MOZ_PKG_CAB_SCRIPT (both needed to successfully create a WinCE CAB file).

Filed a new bug covering dynamic creation of both the script and INF files - bug 476733.

Also set review flag of this attachment to r+ted, as per his previous comment.

Comment 17

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/80de0d994697

Comment 18

[Doug Turner (:dougt)]

http://hg.mozilla.org/mobile-browser/rev/17b22642a33d

Comment 19

[Brad Lassey [:blassey] (use needinfo?)]

follow up for using the /compress flag: bug 476770

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 360395 [details] [diff] [review]
v3.1b patch (mozilla-central patch) 

a191=beltzner

Comment 21

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b39d6b3b56d

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

verified with alpha2 release