65678 - post-install replace

Status: VERIFIED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect, P1)

Description

[Daniel Veditz [:dveditz]]

Need to switch install logic to do in-use file replacement *after* the product
has shut down rather than waiting for the next Mozilla re-start to do it. This
will solve several problems:
- 8.3 filename cruft on Win9x
- re-starting user may not have appropriate permissions
- could never re-register the XPInstall component itself
- the XPInstall library had to be loaded at startup every time to check

Comment 1

[Daniel Veditz [:dveditz]]

Note we also have to continue to support the special WININIT replace mechanism
on Win9x for "system files", and we'll have to add a flag to
addFile/addDirectory to indicate which files should be treated this way.

Comment 2

[Daniel Veditz [:dveditz]]

Moz 0.8 tasks 

Comment 3

[dbragg]

Top priority tasks

Comment 4

[Daniel Veditz [:dveditz]]

Rafael,Ed: this is the bug that will eliminate the need for reboot on Win9x; a
restart of the browser will still be required.

Comment 5

[Grace Bush]

adding jcall (Julie Call) to cc list
Julie, this is the bug we discussed at meeting

Comment 6

[Blake Ross]

[ really cc'ing Julie ]

Comment 7

[dbragg]

Posting diffs.  There will be one for the xpinstall directories and one for the
xpfe/bootstrap directory (the apprunner stuff)

Comment 8

[dbragg]

Attachment: xpinstall related changes

Comment 9

[dbragg]

Attachment: Patch to nsAppRunner.cpp and support files

Comment 10

[dbragg]

Posting new diff based on comments from dveditz.

Comment 11

[dbragg]

Attachment: Apprunner-specific changes

Comment 12

[dbragg]

Attachment: Post Install Replace Utility and xpinstall changes

Comment 13

[dbragg]

Forgot to attach the Mac InstallCleanupMac.cpp

Comment 14

[dbragg]

Attachment: InstallCleanupMac.cpp diff

Comment 15

[dbragg]

Attachment: MANIFEST file to export InstallCleanupDefines.g

Comment 16

[dbragg]

Attachment: MANIFEST file to put the OS message in the res directory

Comment 17

[dbragg]

Attachment: Mac build list diff

Comment 18

[dbragg]

Attachment: InstallCleanupPrefix.h to be added to cleanup/macbuild

Comment 19

[dbragg]

Attachment: InstallCleanupPrefix_debug.h to be added to cleanup/macbuild

Comment 20

[dbragg]

Sorry for all the Mac postings but I'm not really great with Mac CVS.  The last 
two postings, InstallCleanupPrefix.h and InstallCleanupPrefix_debug.h were things 
we had to add to prevent the Mac compiler from optimizing out a bunch of code.   
They need to be added to xpinstall/cleanup/macbuild.

Comment 21

[Scott MacGregor]

minor nit you don't have to change if you don't want to:

In nsAppRunner.cpp:
NS_WITH_SERVICE(nsIProperties, directoryService,
NS_DIRECTORY_SERVICE_CONTRACTID, &rv);
could be written as
nsCOMPtr<nsIProperteries> directoryService
(do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv));

2) Does char* lastResortMessage = "..." need to be a localizeable string?
3) I wonder what the OS/2 implications of this showOSAlert dialog are? We should
fire an email off to mkaply letting him know about this file. (not a preq for
checking in tonight.
4)the last 2 InstallCleanuprefix diffs didn't look like there were changes
except to the copyright notice. Did those patches take correctly?
5) I didn't bother looking through the mac project / build list changes so I
hope you got those right =).

sr=-mscott

Comment 22

[dbragg]

Thanks very much Scott.
* The lastResortMessage is only used in case the localizable message which is
read from cmessage.txt (I checked that in before the localization freeze and
didn't include it in the posting) couldn't be read for some reason.

* I'll make sure to contact mkaply on this.  I've been in contact with him and
javier on most of this project.  In fact, Javier gave me the OS/2 code.

* The InstallCleanupPrefix.h files are new files to the Mac.  I haven't cvs
added them yet so you don't see any cvs diff info in them.

* I hope I got the Mac build script changes right too.  Actually Samir did
almost all of the Mac work.  I've tested the Mac build twice now (with cvs
updates in between) and it seems to build just fine.

Thanks again for taking the time to look at this Scott.  I really appreciate it.

-Don

Comment 23

[dbragg]

Fix checked in.
To test this:
1. Do a clean installation.
2. Run the product and do an update installation. (update.html)
3. BEFORE exiting, look in the program's bin directory.  You should see some
files with .old or .new extenstions on them.  These are files that were locked
by the OS during the update installation.
4. Exit the program.
5. Look in the program's bin directory again.  You should see those .new/.old
files are now gone.  There should also be no xpicleanup.dat (XPICleanup Data on
the Mac) file in the bin directory.  If there is, some file is probably still
locked.
6. Restart the program.  If there was no xpicleanup.dat file it should just
start normally.  If there was/is an xpicleanup.dat file you should get an alert
saying a previous install didn't complete and the program needs to shut down.

Comment 24

[Jimmy Lee]

Reopening as requested by dbragg.  Have something to add? ;-)

Comment 25

[dbragg]

Why yes I do.  This bug/feature is fixed as far as 6.1 is concerned but 6.0
users doing an update install will have a problem.  I need to modify the
browser.jst files.  These eventually become install.js within the browser.xpi
file.  The modification checks to see if the user is running 6.0.  If so, it
explicitly executes xpicleanup.  6.1 has code to call xpicleanup when it exits
but 6.0x doesn't.

Poof!  I've got it fixed for Windows, Mac and Unix.  Posting patch.

Comment 26

[dbragg]

Attachment: Browser.jst (install.js files in browser.xpi) changes to execute xpicleanup

Comment 27

[Samir Gehani]

r=sgehani

Comment 28

[dbragg]

Attachment: NS version of the browser.jst files.

Comment 29

[Samir Gehani]

r=sgehani for ns jst patch.

Comment 30

[Scott MacGregor]

rs=mscott

Comment 31

[dbragg]

Zee feex eez checked in to both mozilla and NS trees.
Thanks to sgehani and mscott for the incredibly quick turn around.

Comment 32

[Jimmy Lee]

Unable to fully verify due to Bug 80383.  It appears to work fine, if I 
artificially get around the problem.

Comment 33

[Jimmy Lee]

Build: 2001-06-07-11-0.9.1(MAC), 2001-06-07-13-0.9.1(WIN), 
2001-06-07-13-0.9.1(LINUX)

Cleanup is after exiting browser.  This was checked in before branch, so marking 
Verified.

Comment 34

[Jimmy Lee]

Build: 2001-08-28-09-trunk(WIN)

This is back as being a problem on Win 98/ME.  We are having problems with 8.3.
 The browser will not launch after an update.html.

Marking Reopened!

Comment 35

[Daniel Veditz [:dveditz]]

Setting milestone, reassigning

Comment 36

[Syd Logan]

Jimmy, I need more data. What exactly is the problem with 8.3? Not sure what to
look for here. Some steps to duplicate would be appreciated.

Comment 37

[Jimmy Lee]

1. From Win 98/ME, launch browser
2. Open an update.html to update to new build
(ftp://sweetlou/products/client/seamonkey/windows/32bit/x86/2001-09-04-00-trunk/update.html)
3. Click Launch XPInstall button to update
4. Click OK button from Items to Install dialog
5. When updating is completed, exit browser and restart machine
6. Launch updated browser

RESULT:
Browser will not launch.  Checking Netscape directory shows that there are files
whose name ends in ~1.  Many of these files can be found in /chrome and /components

EXPECTED RESULT:
Browser launches.  All files replaced are properly named.

Comment 38

[Syd Logan]

*** This bug has been marked as a duplicate of 97334 ***

Comment 39

[Daniel Veditz [:dveditz]]

Closing this fixed bug as a duplicate will mess with the stats

Comment 40

[Daniel Veditz [:dveditz]]

This can't be a duplicate of 97334 because that bug is a subset of this bug,
most of which is fixed.  Bug 97334 has two possible fixes, one of which is to
fix this one on Win98, too, the other is to resurrect the old ren8dot3 hack.

I think we should do the ren8dot3 hack to get us through the near future, then
fix this one the right way in the mozilla1.0-ish timeframe.

Re-closing as fixed, but 97334 will represent both approaches, I guess.

Comment 41

[Daniel Veditz [:dveditz]]

How ironic that the very first problem listed in this bug as things to be solved
by "post-install replace" is the very one that didn't get implemented.

Comment 42

[Jimmy Lee]

Ok, we will track 8.3 file rename from Bug 97334.  If we want a more elegant
solution, this needs to be addressed outside this bug.  Marking this one verified.