Clean the old updates directory on uninstall / in-place upgrade

RESOLVED FIXED in mozilla1.9beta5

Status

()

Toolkit
NSIS Installer
P1
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

Trunk
mozilla1.9beta5
x86
Windows Vista
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Similar to the VirtualStore cleanup performed by the fix for bug 387385
Target Milestone: --- → mozilla1.9 M9
Blocks: 376305
Drivers, if a user has a Software Update pending and runs the installer or for whatever reason Software Update doesn't cleanup the update files the app may be downgraded (see bug 376305). I think this is reason enough to block with a priority of P3.
Flags: blocking1.9?
Priority: -- → P3

Updated

10 years ago
Flags: blocking1.9? → blocking1.9+

Updated

10 years ago
Target Milestone: mozilla1.9 M9 → mozilla1.9 M11

Updated

10 years ago
Blocks: 407008
No longer blocks: 407008
Duplicate of this bug: 409745
Why was this ever a P3? This requires beta exposure ...
Flags: tracking1.9+ → blocking1.9+
Priority: P3 → P1

Updated

10 years ago
Whiteboard: [needs patch]
Target Milestone: mozilla1.9beta3 → mozilla1.9beta5

Updated

10 years ago
Whiteboard: [needs patch] → [needs status update]
To be clear, the reason why I think this is a Big Deal is that now that we're being less aggressive about fetching updates, in terms of popping UI up in front of users, I fear it's more likely that someone may have gotten an update and not realized it.

This concern might not be valid, though, as the situations would be:

User has 3.0.1, updater fetches 3.0.2, user downloads 3.0.2, no change
User has 3.0.1, updated fetches 3.0.2, user downloads 3.0.3, regressed

but after it regresses, won't it then fetch the newer update?
I should have a patch today after finishing the testing of the patch for bug 380015
Whiteboard: [needs status update] → [est 3/18]
Rob, what's up with this?
Turns out that I needed a Vista system since the update dir is located in a different dir for Vista. It is a bit more complicated than I first thought and I am working on it right now.
Created attachment 310900 [details] [diff] [review]
patch rev1

mconnor... any chance on you reviewing this?
Attachment #310900 - Flags: review?(mconnor)
Whiteboard: [est 3/18] → [has patch]
Attachment #310900 - Flags: review?(seth)

Comment 9

10 years ago
Comment on attachment 310900 [details] [diff] [review]
patch rev1

r=sspitzer

two questions:

1)  RmDir /r "$R6"

eyeballing the code, it looks like we'll never generate a bad value for $R6, but would it be worth double checking that $R9 and $R8 are not empty?  

I hate to miss seeing a scenario and this code could remove $LOCALAPPDATA\*
Attachment #310900 - Flags: review?(seth) → review+
Attachment #310900 - Flags: review?(mconnor)

Comment 10

10 years ago
> two questions:

sorry, just one.
I'm testing a patch with the additional checks and will submit shortly
Created attachment 311024 [details] [diff] [review]
patch w/ comments addressed

carrying forward r+
Attachment #311024 - Flags: review+
Attachment #311024 - Flags: approval1.9b5?

Comment 13

10 years ago
Comment on attachment 311024 [details] [diff] [review]
patch w/ comments addressed

thanks for adding the "IfFileExists "$R6\updates" +1 end" sanity check.
Attachment #311024 - Flags: review+
Attachment #311024 - Flags: review+
Whiteboard: [has patch] → [has patch][has review]
Comment on attachment 311024 [details] [diff] [review]
patch w/ comments addressed

a1.9b5=mconnor on behalf of drivers for checkin during baking period
Attachment #311024 - Flags: approval1.9b5? → approval1.9b5+
fixing summary... we don't clean out the updates dir after software update since it contains the update history log and software update manages the mar files.
Summary: Clean the old updates directory on uninstall / in-place upgrade / software update → Clean the old updates directory on uninstall / in-place upgrade
Checked in to trunk

Checking in mozilla/browser/installer/windows/nsis/uninstaller.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/uninstaller.nsi,v  <--  uninstaller.nsi
new revision: 1.19; previous revision: 1.18
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v  <--  common.nsh
new revision: 1.38; previous revision: 1.37
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has review]
Created attachment 311048 [details] [diff] [review]
followup patch - not required for beta 5 but I'd like to land it

Seth, this should really be called from installer.nsi since the profile path is app specific
Attachment #311048 - Flags: review?(seth)

Comment 18

10 years ago
Comment on attachment 311048 [details] [diff] [review]
followup patch - not required for beta 5 but I'd like to land it

r=sspitzer,

sorry for not catching the Mozilla\Firefox in toolkit/mozapps/installer/windows/nsis/common.nsh during my other review.

thanks for the fix.
Attachment #311048 - Flags: review?(seth) → review+
Attachment #311048 - Attachment description: followup patch - not required for beta 5 → followup patch - not required for beta 5 but I'd like to land it
Attachment #311048 - Flags: approval1.9b5?
Comment on attachment 311048 [details] [diff] [review]
followup patch - not required for beta 5 but I'd like to land it

a1.9beta5+=damons
Attachment #311048 - Flags: approval1.9b5? → approval1.9b5+
Comment on attachment 311048 [details] [diff] [review]
followup patch - not required for beta 5 but I'd like to land it

Followup patch checked in to trunk

Checking in mozilla/browser/installer/windows/nsis/installer.nsi;
/cvsroot/mozilla/browser/installer/windows/nsis/installer.nsi,v  <--  installer.nsi
new revision: 1.41; previous revision: 1.40
done
Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh;
/cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v  <--  common.nsh
new revision: 1.39; previous revision: 1.38
done
You need to log in before you can comment on or make changes to this bug.