Closed Bug 366846 Opened 18 years ago Closed 7 years ago

Apply changes to updater.exe before launching it

Categories

(Toolkit :: Application Update, defect, P2)

1.8 Branch
x86
All
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: benjamin, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

We have several situations recently where we wanted to make changes to updater.exe that would take effect in that release, instead of the following release. Currently we launch the "old" updater.exe to run the new one. There are a couple ways to tackle this. Right now I'm thinking about extracting the MAR in the firefox process after we download it... this will involve some linkage changes for libmar.
FWIW, that sounds like a good solution to me.
> We have several situations recently where we wanted to make changes to > updater.exe that would take effect in that release one such situation is bug #354226. additionally, having this would allow us to put fixes for bugs into the "new" updater.exe, to fix issues (like bug #364599).
Blocks: 354226
benjamin, after your proposed fix: during an update, would the old updater.exe be the process that launches the new updater.exe? if not, we could get two UAC prompts on upgrade, as both updater.exe processes will need admin rights.
> during an update, would the old updater.exe be the process that launches the > new updater.exe? No, we would never run the old updater, only the new updater.
Priority: -- → P2
Attached patch Stage the update from the main process... WIP (obsolete) (deleted) — Splinter Review
This is a work in progress. The nsIUpdateStaging interface is fully implemented. I have not yet altered the updater code (it can be much simplified). I moved bspatch.{h,cpp} up one directory to toolkit/mozapps/update/src. I am now in the process of creating logging and testcase code. Then of course I'll need to hook it up to the update service.
This is a complete working patch, without testcases. I am in the process of writing testcases for the updater and the stager. (FWIW I have tested this end-to-end with several kinds of dummy MARs succesfully). I am fairly confident in both the updater and stager changes (aided with automated tests). I am less confident in the updateservice/UI changes, and I am much less certain how to appropriately unit-test them. Some design notes: * The basic idea is to move the "unpacking" of the MAR file from updater.exe to the main firefox process * When the update is finished downloading and we have verified the SHA1 checksum, we immediately "stage" the update ** This means that all the files are extracted to the appdir/updates/0/stage directory ** Any misapplied patches or other errors will cause the update to fail (this part is the part I'm especially not sure about... need help designing a test plan for this!). ** The staging happens asynchronously on a backround thread, which forwards progress notifications to the main thread. * When the updater runs, all it does is rename() files. This means there is very little I/O and it can even replace files that are in use. ** however, this means that if a file is in use, we will leave it around with a .moz-backup extension until the next time updater is run. I believe that this is an acceptable solution, but I'd like feedback. I did most of the development and testing of this patch on Linux. I know it builds on Windows, but haven't really tested it at all yet.
Attachment #259950 - Attachment is obsolete: true
Attachment #260847 - Flags: review?
Attachment #260847 - Flags: review? → review?(sspitzer)
benjamin, sorry for the delay and thanks for your patience with me on this one. perhaps I'm thinking about this the wrong way, and I may live to regret this, but: See a better risk/reward ratio for just solving the "make updater work with a .dll is in use" (bug #340535) than for solving this bug, which includes a fix for that problem, and more. Do you think we could rework this patch into just solving that problem? In this patch, you appear to have fixed the "copy(a,b) then remove (a)" pattern by using rename() [_rename() on windows], and that also saves us from copying files BUFSIZ bytes at a time ["BUFSIZ is 8192 on glibc systems, 1024 on BSD systems and 512 on Windows systems", according to google]. Can we just focus on that part of your fix, and possibly come up with something low risk enough for branch? See https://bugzilla.mozilla.org/show_bug.cgi?id=340535#c84 for a few more thoughts. in addition to switching to rename(), I think we'll have to fix the updater to gracefully handle the scenario where it can't remove the backup file [because it is in use], either after a successful update, and upon applying the next update. let me know what you think. cc'ing a bunch of other folks who have danced with the updater before.
after meeting with bsmedberg and rstrong, the plan is to use benjamin's patch to first fix #340535 (Automatic update restarts infinitely when files to be patched are in use [AccessibleMarshal.dll, MapiProxy.dll, mozMapi32.dll, etc]) as for having updater launch the new updater, that issue is separate and will investigate it separately, but we agree that this code doesn't need to be moved in the firefox process: the updater could be fixed to use the new updater to apply the new update.
Assignee: benjamin → nobody
Product: Firefox → Toolkit
Attachment #260847 - Flags: review?(moco)
Another issue with this is if we change the command line arguments, update the updater, the app won't know about the new command line arguments until after it has been updated.
From previous discussions, I thought we decided we didn't need this capability, because we could just push out a MAR file containing basically only the updater fix, if we needed to do this. EG 3.6.2 gets updated to an identical 3.6.2.1 + new updater, which is then able to update to 3.6.3 (which will be processed by the new updater). Another possibility might be to just add support for a special manifest directive, so that if the updater sees it it ignores the rest of the manifest, updates only itself, and then relaunches the new updater to finish the rest of the manifest.
(In reply to comment #11) > From previous discussions, I thought we decided we didn't need this capability, > because we could just push out a MAR file containing basically only the updater > fix, if we needed to do this. That is my position on this but it was recently brought up in another bug and I wanted to note this additional issue in case I ever get ran over by a bus and someone decides to fix it.
We're not likely ever going to do this.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: