198081 - Old GRE's left on disk when upgrading Mozilla

Status: RESOLVED FIXED

(Core Graveyard :: Installer: GRE, defect, P1)

Description

[Daniel Veditz [:dveditz]]

If you upgrade Mozilla into the same directory old versions of the daily GRE
installs are left on disk, filling it up at an alarming rate.

When you uninstall all those old versions go away, but they should go away on
installation. This is most likely a change to the Mozilla installer, which
already knows it's an upgrade in-place. In those situations it'd have to find
the matching GRE and run its uninstaller (which will decrement the references
and delete the files only if no other apps are using it).

Comment 1

[Patrick]

thanks for filing this one and pointing it out :)

Comment 2

[Juan José Mata]

Nominating nsbeta1

Comment 3

[Charles Rosendahl]

Installer:GRE

Upgrade spec:  When upgrading from one GRE to the next, the uninstallers of
prior GRE installations are to be called with the app name and app path params
to remove the app for the GRE ref count list.

Should the ref count go to 0, the prior GRE should uninstall itself.

If the app count is non-zero (i.e. other apps are still using the prior GRE) it
should remain installed

Comment 4

[Daniel Veditz [:dveditz]]

The component change means the fix goes into the GRE installer itself? But that
won't know if the GRE-using app is being upgraded in place or not.

Comment 5

[Henrik Gemal]

->GRE

Comment 6

[Charles Rosendahl]

http://www.mozilla.org/projects/embedding/GRE_install/win/Requirements.html
http://www.mozilla.org/projects/embedding/GRE_install/win/Inst_spec.html
http://www.mozilla.org/projects/embedding/GRE_install/win/Uninst_spec.html

After registering the GRE-using App in place, the GRE installer must iterate
through prior GRE installations and remove the ref count to the app using the
new GRE by calling each prior GRE installation's uninstaller with the proper params.

If installed to a new location, and the old location still exists, the old gre
will not be removed, if the old installation is deleted and the new install is
in a new location, the old gre uninstaller will detect the absence of the exe
path in the ref count and remove the non-existent exe from the ref list.

At least this is according to the specs above...

Comment 7

[Charles Rosendahl]

Should be fixed ASAP, correctly.

Comment 8

[Michael Buckland]

Discussed in edt bug triage.  Minusing - we don't see this as an issue that
affects end users enough to say top embed.  It's surely a bug, but not a highest
priority bug.

Comment 9

[Paul Wyskoczka]

adt: nsbeta1-

Comment 10

[Sean Su]

Attachment: patch v1.0

Comment 11

[Sean Su]

*** Bug 206719 has been marked as a duplicate of this bug. ***

Comment 12

[Hermann Schwab]

Had GREs 2003040105, -041416, -042308 and 9 GREs from May, -051305 ... -052108.
When I deinstalled Mozilla BuildID 2003052108 / Win98SE, all GREs from May were
deleted, those from April stayed.
Firebird 0.6 could be started, it must have used then one of those older GREs.
GRE 2003041416 was empty, containing only an empty folder 'components'

Comment 13

[Sean Su]

Attachment: patch v1.1

Comment 14

[Sean Su]

Attachment: patch v1.2

almost done.  Just need to comment the code, then will start seeking reviews.

Comment 15

[Sean Su]

Attachment: patch v1.2.1

this patch only contains the string to be displayed while the cleanup is
happening.  I need to check this in first so localization people can translate
it.

Comment 16

[(not reading, please use [e-mail] instead)]

Comment on attachment 124390 [details] [diff] [review]
patch v1.2.1

this has r= from the l10n folks (robinf)

sr/a=sspitzer for 1.4 branch

Comment 17

[Sean Su]

patch v1.2.1 checked in to the trunk and branch.
Still need to get the actual code part of the patch reviewed and checked in.

Comment 18

[Sean Su]

patch v1.2.1 checked in to trunk and branch on both mozilla and ns trees.

Comment 19

[Sean Su]

well... this is not quite fixed.  I just checked in a new string so l10n doesn't
have to wait for the main patch to be checked in.

The real patch is currently v1.2, but I'll be attached another patch cleaned up
and with comments.

Comment 20

[Samir Gehani]

Sean says this has not landed on the 1.4 branch, removing fixed1.4 keyword. 
Only the strings to be localized have landed on the trunk and 1.4 branch.

Comment 21

[Sean Su]

Attachment: patch v2.0

updated patch to trunk so it won't bit-rot, and also made it so the cleanup
code can be triggered in a more liberal fashion.  Still need to comment the
code.

Comment 22

[Sean Su]

Attachment: patch v2.1

comments added.

Comment 23

[Asa Dotzler [:asa]]

I think we should take this for 1.4.1.  Adding blocking flag.

Comment 24

[Sean Su]

*** Bug 207829 has been marked as a duplicate of this bug. ***

Comment 25

[chris hofmann]

should try for 1.5a too

Comment 26

[Daniel Veditz [:dveditz]]

Comment on attachment 125253 [details] [diff] [review]
patch v2.1

r=dveditz

Comment 27

[jag (Peter Annema)]

Comment on attachment 125253 [details] [diff] [review]
patch v2.1

>Index: wizard/windows/setup/dialogs.c
>===================================================================
>   if(RetrieveResults == WIZ_OK)
>   {
>+    // Clean up old versions of gre previously installed.

GRE

>Index: wizard/windows/setup/extra.c
>===================================================================
>+  // If GRE is installed locally, then use the private GRE key, else
>+  // use the default global gre key in the windows registry.

GRE

>+      // Enumerate the GRE ID list built from above to get to each of it's AppList key.

its

>+        if(!rv && (greAppListToClean = NS_GlobalAlloc(sizeof(char *) * totalGREAppListSubKeys)) == NULL)

Is !rv the same as rv == WIZ_OK?

>+        // enumerating thru it's parent key is a Bad Thing(TM).

its

Comment 28

[Sean Su]

Comment on attachment 125253 [details] [diff] [review]
patch v2.1

seeking a=
risk is moderate.  It's not high because this code does not do the actual
cleanup.  It builds a list of orphaned GREs and simply calls the uninstaller on
each one of them to clean them up. 

The uninstaller already had code to cleanup orphaned GREs, but it was missing
the logic to find them.  This patch adds this logic to the installer, but only
triggered when installing GRE, not mozilla (meaning that GRE will clean itself
up).

I've tested this many times on my local tree because I had to keep updating it
to the tip of trunk as it changed.

The rewards are more disk space for users that install nightly builds all the
time.  No more cleaning up orphaned GREs by hand.

Comment 29

[Sean Su]

Attachment: patch v2.1.1

same as patch v2.1, but updated to the tip of trunk.

Comment 30

[Sean Su]

patch v2.1.1 checked in to trunk only.  leaving bug open until patch lands on
branch as well.

Comment 31

[Jason Bassford]

7/11-04 build under XP did not remove my 7/10-08 GRE directory when I installed
it.  Should it have by that build?

Comment 32

[Sean Su]

Yes, it the patch is in the 7/11 trunk build.

Did you install the 7/11 build ontop of the 7/10 build?  The cleanup will only
happen if you install ontop of the previous build, and also if there's no one
else needing the 7/10 GRE.

You can check if anyone requires the 7/10 GRE by using regedit and looking at:
  HKLM\Software\mozilla.org\GRE\1.4_[7/10 id]\AppList

Comment 33

[Sean Su]

I did a test with the trunk commercial build.  I installed 2003-07-10-04 first,
then 2003-07-11-08 ontop of it.

I saw the "Cleaning up orphaned GREs..." dialog at install time.  I checked the
windows registry and also [common files]\mozilla.org\GRE after installation
completed, the old GRE was cleaned up/removed from both places.

Comment 34

[Sean Su]

Comment on attachment 127490 [details] [diff] [review]
patch v2.1.1

moving r=dveditz and sr=jag to here and re-requesting approval for 1.4.x

Comment 35

[Jason Bassford]

> Did you install the 7/11 build ontop of the 7/10 build?

So - you're not supposed to remove the previous install at all but do an in
place upgrade?  I'll try that next time.  (But isn't it a little strange that
that's the unsupported / not recommended method?)

Comment 36

[Jason Bassford]

I just did an in place upgrade from 7/11-08 to 7/13-08 (keeping the old install
intact and installing on top of it) but it *still* did not remove the old GRE. 
I'm using XP.

Comment 37

[Will Levine]

I've installed a couple of new builds since this patch got checked in.  I told
it to remove the existing Mozilla installation  The "Cleaning up orpaned GREs"
dialog appeared, but the old GREs were not removed from the hard drive (common
files/mozilla.org/GRE) or the registry (HKLM/SOFTWARE/mozilla.org/GRE).

Comment 38

[Sean Su]

Jason and Will, what OS do you have?  Can you both email me ([e-mail]):
 * the export of HKLM\Software\mozilla.org\GRE (saved in the readable
"Win9x/NT4" text format please)
 * the dir list of [common files]\mozilla.org\GRE (*not* including subdirs).  I
just want to see if it matches the registry.

Comment 39

[Jason Bassford]

Update: Recent builds are now properly removing old GREs.  Did something get
checked in without a comment here?

Comment 40

[Sean Su]

Jason, nothing's been checked in recently.  I'm working on a patch that will log
more information in gre's install_status.log to see what might be going on in
your case.

Since it's working for you, my suspicion is that it might have been a windows
registry issue.

Please update this bug if you encounter the problem again.  I will attach the
patch that has the logging code to bug 212599.

Comment 41

[Asa Dotzler [:asa]]

Comment on attachment 127490 [details] [diff] [review]
patch v2.1.1

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 42

[Sean Su]

patch checked in on the MOZILLA_1_4_BRANCH tag now.

Comment 43

[lohphat]

this needs to be reopened.  it's been leaving old GRE files for weeks.  Dup bug
 230378 filed.  (Can't reopen this one)

Comment 44

[Patrick]

*** Bug 230378 has been marked as a duplicate of this bug. ***

Comment 45

[Patrick]

i'm doing proper uninstalls for a while now but not sure exactly HOW long.
Anyway, i got 12 GRE's left on my drive with 1.4f_2003062408 the oldest [not
sure if i properly uninstalled that one] and 1.7a_2004010808 the youngest
[definitely uninstalled that one via the control panel].

Therefore reopening bug.

Comment 46

[Patrick]

Sorry, i jumped the gun too early. I combined 
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&safe=off&threadm=btkn66%24cnv1%40ripley.netscape.com&rnum=1&prev=/groups%3Fq%3DGRE%2Bmozilla%2BRon%26hl%3Den%26lr%3D%26ie%3DUTF-8%26oe%3DUTF-8%26safe%3Doff%26selm%3Dbtkn66%2524cnv1%2540ripley.netscape.com%26rnum%3D1
and comment 43 and old GRE's left on my computer to the wrong conclusion.

Doing a proper uninstall via the Control Panel DOES remove old GRE's.
Apologies for the spam.

Comment 47

[lohphat]

The installer should remove them as part of an upgrade install.  Asking users to
do an uninstall first causes confusion and fear of potentially lost profile data.

The installer was doing the right thing before, why isn't it doing it now?

Think like a sysadmin, not a developer.

Comment 48

[Urban A. Haas]

I myself, have been concerned at a loss of profile data. I have stored passwords
I do not wish to loose, as well as years of organizing bookmarks. Asking someone
to uninstall and reinstall is not a solution, it's a workaround.

A workaround does not remove a bug, or solve it.

Comment 49

[Brian Rogers]

The installer definately should remove old GRE's.
Reopening.

Comment 50

[lohphat]

The install is now removing the old GRE files *but* it's now resetting the OS
browser/MAPI prefs.  You can see desktop icons change during the GRE cleanup
back to default IE icons.

See bug 235844 

Comment 51

[Patrick]

*** Bug 247538 has been marked as a duplicate of this bug. ***

Comment 52

[David E. Ross]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040616

Still happening with both files and registry entries.  

Comment 53

[David E. Ross]

When installing Mozilla 1.7.2 over Mozilla 1.7, old GRE files and registry
entries were removed.  

c:\Program Files\Common Files\mozilla.org\GRE\1.7_2004061609 and its contents
were deleted.  

[HKEY_USERS\.DEFAULT\Software\mozilla.org\GRE\1.7_2004061609]  and 
[HKEY_LOCAL_MACHINE\Software\mozilla.org\GRE\1.7_2004061609] were deleted.  

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.2) Gecko/20040803

Comment 54

[Hermann Schwab]

*** Bug 259614 has been marked as a duplicate of this bug. ***

Comment 55

[Hermann Schwab]

*** Bug 260456 has been marked as a duplicate of this bug. ***

Comment 56

[Hermann Schwab]

*** Bug 263092 has been marked as a duplicate of this bug. ***

Comment 57

[David E. Ross]

Oops!  When installling Mozilla 1.7.3 over 1.7.2, registry key 
  HKEY_LOCAL_MACHINE\Software\Mozilla\Mozilla 1.7.2
was not removed.  GRE files and the registry key in [HKEY_USERS\.DEFAULT] were
removed.  (See my comment 53.)

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.3) Gecko/20040910

Note:  Installation instructions no longer recommend removing the old version of
Mozilla before installing a new version.  

Comment 58

[Marvin George]

The GRE should be removed and not tell a person uninstall the last version as I
load the new beta every week so I could test out the program and get burn if
there are some programing errors.  This should remove when you install a new
version as I was taught in the 1960's and 1070's in writing programs.

Comment 59

[Daniel Veditz [:dveditz]]

It's working for me, the old GRE's are gone, and the only ones I have left
correspond with registry entries and various existing installations on my disk.
I do have a bunch of **empty** GRE directories; annoying, but that should be a
new bug, not a REOPEN of this one.

The registry entry in comment 57 is likewise a different bug. The Mozilla
installer launches the GRE install, which is supposed to clean up after itself.
The Mozilla installer should clean up the Mozilla entries. Two separate programs
(yes, shared code, but two processes, driven by two different config files).

Comment 60

[Daniel Veditz [:dveditz]]

I noticed that the dupe that caused the reopen was on nightly builds. Releases
do have some different build-fu applied, it's quite possible that the GRE
cleanup-trigger was not in nightly builds. The functionality exists -- this bug
-- but it may not be called in nightlies.

I'm reopening bug 230378 to deal with the regression.