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)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: benjamin, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•18 years ago
|
||
FWIW, that sounds like a good solution to me.
Comment 2•18 years ago
|
||
> 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
Comment 3•18 years ago
|
||
Comment 4•18 years ago
|
||
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.
Reporter | ||
Comment 5•18 years ago
|
||
> 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.
Reporter | ||
Updated•18 years ago
|
Priority: -- → P2
Reporter | ||
Comment 6•18 years ago
|
||
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.
Reporter | ||
Comment 7•18 years ago
|
||
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?
Reporter | ||
Updated•18 years ago
|
Attachment #260847 -
Flags: review? → review?(sspitzer)
Comment 8•18 years ago
|
||
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.
Comment 9•17 years ago
|
||
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.
Reporter | ||
Updated•17 years ago
|
Assignee: benjamin → nobody
Assignee | ||
Updated•16 years ago
|
Product: Firefox → Toolkit
Reporter | ||
Updated•15 years ago
|
Attachment #260847 -
Flags: review?(moco)
Comment 10•14 years ago
|
||
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.
Comment 11•14 years ago
|
||
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.
Comment 12•14 years ago
|
||
(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.
Comment 13•7 years ago
|
||
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.
Description
•