Closed
Bug 759615
Opened 13 years ago
Closed 13 years ago
bgupdates breaks uninstalls if you uninstall after an update
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: bbondy, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file, 2 obsolete files)
3.40 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
1. Install the second newest Nightly build of m-c. 2. Do an update 3. Try to uninstall. Actual result: You get an error "An error occurred while tying to uninstall Nightly 15.0a1 (x86 en-US). It may have already been uninstalled. Expected result: The uninstall works. I think what's happening is the PostUpdate sets the install location to installdir/updated.
Reporter | ||
Updated•13 years ago
|
Comment 1•13 years ago
|
||
Brian, does it for you also apply to the silent mode of the uninstaller? At least I can't see this issue with our Mozmill update tests which do exactly the same but with the silent mode. > [mozilla-central_update] $ cmd.exe /C '"mozmill-env\run mozmill-automation\download.py --type=%BUILD_TYPE% --branch=mozilla-central --platform=%PLATFORM% --locale=%LOCALE% --build-id=%BUILD_ID% --directory=builds && exit %%ERRORLEVEL%%"' > Retrieving list of builds from https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/05/ > Downloading build: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/05/2012-05-29-03-05-18-mozilla-central/firefox-15.0a1.en-US.win32.installer.exe > [mozilla-central_update] $ cmd.exe /C '"mozmill-env\run mozmill-automation\testrun_update.py --target-buildid=%TARGET_BUILD_ID% --no-fallback --repository=mozmill-tests --junit=report.xml --report=%REPORT_URL% builds && exit %%ERRORLEVEL%%"' > *** Cloning repository to 'c:\docume~1\mozilla\locals~1\temp\tmpzbcpmb.mozmill-tests' > requesting all changes > adding changesets > adding manifests > adding file changes > added 2250 changesets with 5740 changes to 680 files (+5 heads) > updating to branch default > 346 files updated, 0 files merged, 0 files removed, 0 files unresolved > *** Installing 2012-05-29-03-05-18-mozilla-central-firefox-15.0a1.en-US.win32.installer.exe => c:\docume~1\mozilla\locals~1\temp\tmplidfaa.binary\ > *** Updating to branch 'default' > pulling from mozmill-tests > searching for changes > no changes found > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved [...] > INFO Passed: 7 > INFO Failed: 0 > INFO Skipped: 0 > Report document created at 'http://mozmill-ci.blargon7.com/db/fdec829b93b19c73985be1d38820744b' > *** Uninstalling c:\docume~1\mozilla\locals~1\temp\tmplidfaa.binary\ > *** Removing repository 'c:\docume~1\mozilla\locals~1\temp\tmpzbcpmb.mozmill-tests'
Reporter | ||
Comment 2•13 years ago
|
||
I suspect the mozmill script is not using background updates at all or else is only doing the apply operation and not the replace operation. I think it would apply to both silent and not although I haven't tested it with silent. I did notice if you create a folder called updated and copy everything into it, then uninstall with the uninstall option in control panel, it will find the uninstall/helper.exe in that case and launch the uninstall wizard.
Comment 3•13 years ago
|
||
Our scripts are using the old update ui which downloads and applies the update immediately before the restart of the browser. So I thought it could be the same code path. But yeah, could be a bit different.
Reporter | ||
Comment 4•13 years ago
|
||
> Our scripts are using the old update ui Is there a reason why we can't always use the current update UI code? I don't know much of how mozmill works so sorry for the potential ignorance embedded in that question. > ...which downloads and applies the > update immediately before the restart of the browser. So I thought it could be the same code path. That sounds like it's using background updates, so should be the same code path. > *** Uninstalling c:\docume~1\mozilla\locals~1\temp\tmplidfaa.binary\ > *** Removing repository 'c:\docume~1\mozilla\locals~1\temp\tmpzbcpmb.mozmill-tests' The silent unisntall option probably just fails silently. Do you actually check if the directory is empty after uninstall? By the way I often see bugs that are found towards the end of a release cycle about a folder or file not being removed from the uninstaller. Could you explicitly check if the directory is empty or doesn't exist after an uninstall?
Reporter | ||
Comment 5•13 years ago
|
||
More info: The /PostUpdate process can be started from one of these locations in updater.cpp: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2435 http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#2754 Or here from workmontior.cpp: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/maintenanceservice/workmonitor.cpp#251 What executes can be found here: http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/shared.nsh#5 It looks as though this line gets the wrong install dir inside SetUninstallKeys: http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/shared.nsh#480 > ${WriteRegStr2} $1 "$0" "UninstallString" "$8\uninstall\helper.exe" 0
Reporter | ||
Comment 6•13 years ago
|
||
Before an upgrade with bgupdates: HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Nightly 15.0a1 (x86 en-US) value: C:\Program Files (x86)\Nightly\uninstall\helper.exe After an upgrade with bgupdates: HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Nightly 15.0a1 (x86 en-US) C:\Program Files (x86)\Nightly\updated\uninstall\helper.exe
Assignee | ||
Comment 7•13 years ago
|
||
I'll look into this more deeply, should be easy to fix...
Assignee: nobody → ehsan
Reporter | ||
Comment 8•13 years ago
|
||
I think the best way to fix this bug is if you're using background updates, then don't run the post upgrade process until after the replace stage. Currently it runs after the apply stage but before the replace stage.
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #8) > I think the best way to fix this bug is if you're using background updates, > then don't run the post upgrade process until after the replace stage. > Currently it runs after the apply stage but before the replace stage. Agreed.
Comment 10•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #4) > Is there a reason why we can't always use the current update UI code? I > don't know much of how mozmill works so sorry for the potential ignorance > embedded in that question. We haven't been had a chance yet to fully move to the new ui. Mainly because the old UI is still in place when incompatible add-ons will be disabled on a major upgrade. For more details see bug 599290. > The silent unisntall option probably just fails silently. Do you actually > check if the directory is empty after uninstall? By the way I often see No, we don't check that. The uninstaller which will soon be part of mozinstall will simply remove the remaining installation folder. So we haven't seen that behavior. > file not being removed from the uninstaller. Could you explicitly check if > the directory is empty or doesn't exist after an uninstall? It's not something we could easily integrate into a Mozmill test. Therefore we would have to create tests on the Python side. So far any test is run inside the browser and reports also only include those results. Clint and Jeff, I think it would be a good idea to extend the Python side of Mozmill to easily allow additional tests and include those into the log/report.
Assignee | ||
Comment 11•13 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #10) > Clint and Jeff, I think it would be a good idea to extend the Python side of > Mozmill to easily allow additional tests and include those into the > log/report. Can we please have that discussion somewhere else? As it's not related to the bug at hand.
Comment 12•13 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #10) > It's not something we could easily integrate into a Mozmill test. Therefore > we would have to create tests on the Python side. So far any test is run > inside the browser and reports also only include those results. > > Clint and Jeff, I think it would be a good idea to extend the Python side of > Mozmill to easily allow additional tests and include those into the > log/report. Mozmill has no notion of install/uninstall. This has to happen outside of the tool, so therefore this is better suited to living in mozinstall and up leveling the failure status into the wrapping automation scripts. This is something that AutomatedTester's tool will be needing to do for Apps anyway, so work is already underway. So, yes, this is something we can do, it's not part of mozmill proper, it's part of the code that calls mozmill in your automation. We can take this up in a follow on bug (feel free to file) so we don't hijack Ehsan's bug here.
Assignee | ||
Comment 13•13 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #9) > (In reply to Brian R. Bondy [:bbondy] from comment #8) > > I think the best way to fix this bug is if you're using background updates, > > then don't run the post upgrade process until after the replace stage. > > Currently it runs after the apply stage but before the replace stage. > > Agreed. Robert, do you also agree with this? I'd like to know which kind of fix you think is appropriate here.
Comment 14•13 years ago
|
||
Agreed... that would be the best stage to run it.
Assignee | ||
Comment 15•13 years ago
|
||
Attachment #628501 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 16•13 years ago
|
||
Fixed a typo, sorry.
Attachment #628501 -
Attachment is obsolete: true
Attachment #628501 -
Flags: review?(robert.bugzilla)
Attachment #628504 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 17•13 years ago
|
||
We should also be passing the installation directory to LaunchWinPostProcess, not the callback file name.
Attachment #628504 -
Attachment is obsolete: true
Attachment #628504 -
Flags: review?(robert.bugzilla)
Attachment #628541 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 19•13 years ago
|
||
FWIW, I verified this fix on the Oak branch.
Updated•13 years ago
|
Attachment #628541 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 20•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/30babf8e5573
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•