Closed
Bug 790096
Opened 12 years ago
Closed 12 years ago
Please create a hotfix add-on to address the update bustage on Firefox 15
Categories
(Firefox :: General, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: ehsan.akhgari, Assigned: MattN)
References
Details
Attachments
(1 file, 2 obsolete files)
See bug 789958.
Here is the summary of my conversaion with MattN on #fx-team: http://pastebin.mozilla.org/1814739
This hotfix add-on is only applicable to Linux and Mac OS X, the bug does not exist on Windows.
Comment 1•12 years ago
|
||
A possible alternative?
1) Fix updater code, release as 15.0.2
2) When generating updates for 15.0.0 & 15.0.1 to 15.0.2, hand-tweak update manifest to ensure updater is treated as a complete update and not a partial (ie, no binary diffs, just a file replace)
3) Create hotfix addon that contains the 15.0.2 updater, and replaces your 15.0 / 15.0.1 updater with it.
4) Allow 15.0.2 update process to happen naturally (perhaps restarting it if it already failed?)
(I was thinking about how it would be nice if we had a way to update _just_ the updater, for situations like this. And then realized we basically could.)
Reporter | ||
Comment 2•12 years ago
|
||
(In reply to comment #1)
> A possible alternative?
>
> 1) Fix updater code, release as 15.0.2
> 2) When generating updates for 15.0.0 & 15.0.1 to 15.0.2, hand-tweak update
> manifest to ensure updater is treated as a complete update and not a partial
> (ie, no binary diffs, just a file replace)
> 3) Create hotfix addon that contains the 15.0.2 updater, and replaces your 15.0
> / 15.0.1 updater with it.
> 4) Allow 15.0.2 update process to happen naturally (perhaps restarting it if it
> already failed?)
>
> (I was thinking about how it would be nice if we had a way to update _just_ the
> updater, for situations like this. And then realized we basically could.)
The original bug affects both partial and complete updates the same way (and indeed, right after the partial update fails, we fall back into downloading and applying the full update, which will also fail.)
However, the idea of releasing the hotfix add-on to replace the updater binary with a potential 15.0.2 binary which fixes this issue seems reasonable after some initial thought.
Reporter | ||
Comment 3•12 years ago
|
||
Robert, please see comments 1 and 2.
Comment 4•12 years ago
|
||
The hotfix add-on won't always have privileges to update the updater binary. I also think using the hotfix add-on to disable bgupdates for the affected platforms and then updating has less moving parts that could go wrong. It is an alternative that we could use if necessary but I don't see additional value in the alternative.
Assignee | ||
Comment 5•12 years ago
|
||
Still some TODOs to address but this is most of the way there.
Some questions:
1) Which Firefox versions are we targeting to flip the pref to false?
2) Isn't there a directory service entry for the application install directory? I didn't see it when I looked quickly but I could have sworn there was one.
3) This patch doesn't handle flipping the pref back to true on a fixed build. I'm guessing we will just use Services.vc to compare the version against a known fixed one. Will we know the fixed version before releasing this or are we going to use two hotfixes (one to flip to false and another to flip to true when fixed)?
Tested quickly locally on FC 17 and OS X 10.7.4
Test build: https://people.mozilla.com/~mnoorenberghe/hotfix-v20120910.01-wip1.xpi
Uncomment two debug lines in check_app_permissions.sh and then disable+enable for more debug info.
Attachment #659977 -
Flags: feedback?(ehsan)
Attachment #659977 -
Flags: feedback?(dtownsend+bugmail)
Attachment #659977 -
Flags: feedback?(dolske)
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 659977 [details] [diff] [review]
WIP Doesn't handle all error cases and only flips stage pref to false
Review of attachment 659977 [details] [diff] [review]:
-----------------------------------------------------------------
::: v20120910.01/check_app_permissions.sh
@@ +22,5 @@
> +# find $1 -not -uid $uid -or -not \( $groupQuery \) >> /tmp/hotfix.txt
> +
> +# Find all files that are not owned by the user or are for a group which the
> +# user is not part of. If the result is empty, return 0, otherwise return 1.
> +if [ -z "`find $1 -not -uid $uid -or -not \( $groupQuery \)`" ]; then
Just realized I didn't include "-type f" like Ehsan said on IRC. Can someone confirm I should only check files and not directories?
Reporter | ||
Comment 7•12 years ago
|
||
Comment on attachment 659977 [details] [diff] [review]
WIP Doesn't handle all error cases and only flips stage pref to false
Review of attachment 659977 [details] [diff] [review]:
-----------------------------------------------------------------
So, I gave this a bit more thought, and I think I now agree with Robert in that we should just disable background updates for OS X and Linux on the affected versions. The affected versions will be 15.0 and 15.0.1 in the release channel. I don't know which versions in other channels we want this on. That is a call to be made on the release drivers mailing list, and I'll post about it right now (CCing you.)
Sorry for changing strategies here, but I think we should opt for as much simplicity in what the hotfix add-on does as possible.
Attachment #659977 -
Flags: feedback?(ehsan) → feedback-
Comment 8•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #7)
> So, I gave this a bit more thought, and I think I now agree with Robert in
> that we should just disable background updates for OS X and Linux on the
> affected versions.
So, basically the hot fix is just a simple pref-flip for everyone on those two platforms, for 15.0+? Works for me.
Reporter | ||
Comment 9•12 years ago
|
||
(In reply to comment #8)
> (In reply to Ehsan Akhgari [:ehsan] from comment #7)
>
> > So, I gave this a bit more thought, and I think I now agree with Robert in
> > that we should just disable background updates for OS X and Linux on the
> > affected versions.
>
> So, basically the hot fix is just a simple pref-flip for everyone on those two
> platforms, for 15.0+?
Precisely, with the extra caveat that we need to handle version numbers across all branches.
Comment 10•12 years ago
|
||
Assigning myself as QA Contact. I'll test this if/when we decide to move forward.
QA Contact: anthony.s.hughes
Assignee | ||
Comment 11•12 years ago
|
||
My understanding is that this won't be released until after 16.0 is on the release channel and therefore this hotfix would have to be combined with bug 774509 as they both target OS X. The hotfix also doesn't set the stage pref to false if it doesn't exist since that would be either an old build without this feature or one that has the new pref (assuming we'll remove the old pref when we add the new one).
I'd like feedback on this standalone version before combining hotfixes.
https://people.mozilla.com/~mnoorenberghe/hotfix-v20120910.01-wip2.xpi
Attachment #659977 -
Attachment is obsolete: true
Attachment #659977 -
Flags: feedback?(dtownsend+bugmail)
Attachment #659977 -
Flags: feedback?(dolske)
Attachment #660266 -
Flags: feedback?(ehsan)
Attachment #660266 -
Flags: feedback?(dolske)
Reporter | ||
Comment 12•12 years ago
|
||
Comment on attachment 660266 [details] [diff] [review]
Approach 2 (standalone) - flip the pref, set a hotfix pref, and uninstall
Review of attachment 660266 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good with the below comments addressed.
::: README
@@ +16,5 @@
> uninstalls itself immediately.
> v20120430.01 - Bug 741004 - A hotfix rolled out to 10.0 - 12.* Windows users to
> communicate the de-support of Windows 2000 and XP <= SP1 in Firefox 13.
> +v20120910.01 - Bug 790096 - Disable staged updates for OS X and Linux for
> + 15.0 - 18.0a1. The pref will be changed in fixed builds.
This comment should probably be updated.
::: v20120910.01/bootstrap.js
@@ +9,5 @@
> +
> +const APP_UPDATE_STAGE_ENABLED_PREF = "app.update.stage.enabled";
> +// Preference to track that we flipped the above pref so we can flip it back
> +// when the updater is fixed.
> +const HOTFIX_APPLIED_PREF = "extensions.firefox-hotfix.20120910";
We don't need to flip the pref back on any more. We'll just rename the pref on the fixed versions. So, you can remove this and the related code below.
@@ +55,5 @@
> + */
> +function shouldHotfixApp() {
> + // Ensure that this is the correct version in case compatability checking is overridden.
> + if (Services.vc.compare(Services.appinfo.version, "15.0") < 0 ||
> + Services.vc.compare(Services.appinfo.version, "18.0a1") > 0) {
Does this also include 15.0a1 and 15.0b1? It probably should it if doesn't.
Attachment #660266 -
Flags: feedback?(ehsan) → feedback+
Assignee | ||
Comment 13•12 years ago
|
||
Addressed the two code comments.
(In reply to Ehsan Akhgari [:ehsan] from comment #12)
> ::: README
> > +v20120910.01 - Bug 790096 - Disable staged updates for OS X and Linux for
> > + 15.0 - 18.0a1. The pref will be changed in fixed builds.
>
> This comment should probably be updated.
Doesn't it reflect the current plan? Do you want it to be more verbose? I tweaked it slightly in this version.
Ehsan, since you're a Firefox module peer, your review is sufficient. I'd like to check this hotfix code into the hotfix repo and then make a separate hotfix version which combines this with bug 774509.
Attachment #660266 -
Attachment is obsolete: true
Attachment #660266 -
Flags: feedback?(dolske)
Attachment #660292 -
Flags: review?(ehsan)
Reporter | ||
Comment 14•12 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #13)
> Created attachment 660292 [details] [diff] [review]
> Standalone v2 - Flip pref if true and uninstall
>
> Addressed the two code comments.
>
> (In reply to Ehsan Akhgari [:ehsan] from comment #12)
> > ::: README
> > > +v20120910.01 - Bug 790096 - Disable staged updates for OS X and Linux for
> > > + 15.0 - 18.0a1. The pref will be changed in fixed builds.
> >
> > This comment should probably be updated.
>
> Doesn't it reflect the current plan? Do you want it to be more verbose? I
> tweaked it slightly in this version.
No, the current plan is to rename the pref in fixed builds.
Reporter | ||
Comment 15•12 years ago
|
||
Comment on attachment 660292 [details] [diff] [review]
Standalone v2 - Flip pref if true and uninstall
Review of attachment 660292 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM, but I have never reviewed the code for the hotfix add-on before, and I'm not sure if I was looking for everything that I should be watching out for. Can you please ask someone else to have a second look here as well? Thanks!
::: README
@@ +16,5 @@
> uninstalls itself immediately.
> v20120430.01 - Bug 741004 - A hotfix rolled out to 10.0 - 12.* Windows users to
> communicate the de-support of Windows 2000 and XP <= SP1 in Firefox 13.
> +v20120910.01 - Bug 790096 - Disable staged updates on OS X and Linux for
> + 15.0a1 - 18.0a1. The flipped pref name will be renamed in fixed builds.
Oh, you changed this. Looks good!
Attachment #660292 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 16•12 years ago
|
||
Comment on attachment 660292 [details] [diff] [review]
Standalone v2 - Flip pref if true and uninstall
Mossop/dolske/others, mind taking a look at this. It's a pretty simple hotfix.
(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> No, the current plan is to rename the pref in fixed builds.
That's what I was trying to say but I can see how it was a bit ambiguous so I fixed it. Changing the pref was referring to changing the pref name, not flipping it. :)
Attachment #660292 -
Flags: review?(dtownsend+bugmail)
Attachment #660292 -
Flags: review?(dolske)
Updated•12 years ago
|
Attachment #660292 -
Flags: review?(dtownsend+bugmail) → review+
Assignee | ||
Comment 17•12 years ago
|
||
Pushed: https://hg.mozilla.org/users/dtownsend_mozilla.com/hotfixes/rev/4b7995fd09f6
Test build:
https://people.mozilla.com/~mnoorenberghe/hotfixes/hotfix-v20120910.01-v2.xpi
Filed bug 790821 to combine hotfixes for OS X. Linux can probably use this standalone version.
Status: NEW → RESOLVED
Closed: 12 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Comment 18•12 years ago
|
||
Matthew, can you please confirm what this hotfix does before I test it? If I understand correctly, this will flip the old background update pref to "false" on an "affected" build. Those builds being:
* Firefox 15.0a1/a2/b*
* Firefox 15.0/15.0.1
* Firefox 16.0a1/a2/b*
* Firefox 17.0a1/a2
* Firefox 18.0a1
Is this correct?
Comment 19•12 years ago
|
||
Oh, and this only applies to builds on Mac and Linux, right? It will not do anything if installed in a Windows build?
Also, it only applies to "normal" builds, right? It will not do anything if installed in an ESR build?
Assignee | ||
Comment 20•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #18)
> Matthew, can you please confirm what this hotfix does before I test it? If I
> understand correctly, this will flip the old background update pref to
> "false" on an "affected" build. Those builds being:
> * Firefox 15.0a1/a2/b*
> * Firefox 15.0/15.0.1
> * Firefox 16.0a1/a2/b*
> * Firefox 17.0a1/a2
> * Firefox 18.0a1
>
> Is this correct?
Yes, except it doesn't target the affected builds using the specific version number ranges for each major version. Instead, it installs in all builds from 15.0a1 to 18.* and checks if app.update.stage.enabled exists and is true. If so, it flips the pref to false and uninstalls, Otherwise, it just uninstalls itself after doing nothing.
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #19)
> Oh, and this only applies to builds on Mac and Linux, right? It will not do
> anything if installed in a Windows build?
Yes, the targetApplication is set to Linux and Darwin so it should not even install on other operating systems.
> Also, it only applies to "normal" builds, right? It will not do anything if
> installed in an ESR build?
Correct. If it gets installed in an ESR build (for the affected version range above), it will uninstall itself immediately. Is this correct?
I'm just going repeat here again the known issue that the hotfix may temporarily show in the add-ons manager after uninstallation if you have it open to the extensions tab while it uninstalls. Re-opening the AOM or toggling the category on the left should update the list of extensions.
Comment 21•12 years ago
|
||
I've just sent an email to Softvision instructing them to test this hotfix overnight based on a testplan Matthew and I worked together over the last hour or so. I will report back tomorrow morning with the results of that testing.
For reference, you can see the test plan here:
https://etherpad.mozilla.org/background-update-hotfix
Comment 22•12 years ago
|
||
Marking this verified fixed. All results are a PASS. I won't go into the details here, please read the etherpad if interested. For posterity, I've archived the last etherpad revision here:
https://etherpad.mozilla.org/ep/pad/view/background-update-hotfix/RY3JRug9BE
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
Attachment #660292 -
Flags: review?(dolske) → review+
Comment 23•12 years ago
|
||
I've retested this on Mac and Linux using Fx15 and the updated hotfix from bug 803596, and the hotfix is still addressing this scenario.
You need to log in
before you can comment on or make changes to this bug.
Description
•