Closed
Bug 498379
Opened 15 years ago
Closed 15 years ago
Checking that we aren't updating to an older version of the application fails in certain cases
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: mossop, Assigned: mossop)
References
Details
(Keywords: fixed1.9.0.12, fixed1.9.1)
Attachments
(1 file)
(deleted),
patch
|
robert.strong.bugs
:
review+
dveditz
:
approval1.9.0.12+
|
Details | Diff | Splinter Review |
When we startup we look for updates to apply. Since bug 313057 we check that the update isn't for an older version of the application already installed. The code that write the update.version includes a trailing newline in the file. THe code that reads the update.version fails to ignore it, so we start doing funky version comparisons line "3.5b99" against "3.5\n". Due to the fun of the version comparator "3.5\n" < "3.5b99" so we delete the update.
Flags: blocking1.9.1?
Flags: blocking1.9.0.12?
Comment 1•15 years ago
|
||
Is there any way to fix this problem for people on 3.5b99? If I understand what you're writing, there isn't.
Comment 2•15 years ago
|
||
This is a STOP SHIP issue for 3.0.12, IMO, and will likely require a RC1build2.
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Comment 3•15 years ago
|
||
(In reply to comment #1)
> Is there any way to fix this problem for people on 3.5b99? If I understand what
> you're writing, there isn't.
I've got an idea about how to work around this, I'm about to send mail to r-d.
Comment 4•15 years ago
|
||
Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
Also, ss pointed out that if we ship RC1build2 as a major update, we might be able to get away without needing to ship a 3.5b100 that contains a fix for this issue, right?
Comment 6•15 years ago
|
||
(In reply to comment #4)
> Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
>
> Also, ss pointed out that if we ship RC1build2 as a major update, we might be
> able to get away without needing to ship a 3.5b100 that contains a fix for this
> issue, right?
It would be tough to ship a 3.5b100 at this point, anyways. We should do 3.5rc1build2 and 3.5rc2 IMHO.
I can test doing build2 as a major update.
Comment 7•15 years ago
|
||
(In reply to comment #4)
> Also, ss pointed out that if we ship RC1build2 as a major update, we might be
> able to get away without needing to ship a 3.5b100 that contains a fix for this
> issue, right?
I just tested this - updates are still broken if they're done as a major update.
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Comment 8•15 years ago
|
||
(Sorry, bugzilla made me break the blocking flags)
Assignee | ||
Comment 9•15 years ago
|
||
This strips off any trailing newline that we read from update.version. We could alternately just not write the newline in the first place but I think this way works better in case people are manually editing things in the updates dir. We could do both of course, your call Rob. I don't think we have a way to test this code?
Attachment #383294 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Comment 10•15 years ago
|
||
At Mossop's suggestion I just tried using 'extv=3.5rc1' in the snippets, and that made the update work.
I don't know what the implications are for extensions if we actually ship with those, though.
Assignee | ||
Comment 11•15 years ago
|
||
Rob, there is some inconsistency between how bug 313057 does its version comparison and how bug 485624 does it. The former uses update.extensionVersion (which is what is written to update.version) and compares that to the application version on startup. The latter compares update.version to the application version during running.
We should I think make this consistent, maybe even while we were here. If we made them both use update.extensionVersion then it would actually allow build to go back to using any version name they liked in update.version which would resolve bug 496917 even more nicely. But I'm a little fuzzy on what the semantics of update.version and update.extensionVersion are meant to be, can you clarify?
Comment 12•15 years ago
|
||
If you need some spare tester, ping me. Sorry if this count`s as off topic.
Comment 13•15 years ago
|
||
(In reply to comment #4)
> Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
Is this a problem for the just-shipped FF3.0.11?
Comment 14•15 years ago
|
||
(In reply to comment #13)
> (In reply to comment #4)
> > Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
>
> Is this a problem for the just-shipped FF3.0.11?
AIUI, it will only cause problems for affected 3.x[abrc] builds that have the bug in them.
"3.0.11" < "3.0.12\n" will be True despite the newline. And since this bug (AFAIK) doesn't exist in any 3.0[abrc] products we should be fine there.
Assignee | ||
Comment 15•15 years ago
|
||
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #4)
> > > Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
> >
> > Is this a problem for the just-shipped FF3.0.11?
The code that is the problem was not shipped in 3.0.11. It has however been landed ready to ship in 3.0.12 so we should fix this before 3.0.12 ships
Updated•15 years ago
|
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Comment 16•15 years ago
|
||
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > (In reply to comment #4)
> > > > Oh, we haven't shipped 3.0.12 yet, nor code frozen, so yay.
> > >
> > > Is this a problem for the just-shipped FF3.0.11?
>
> The code that is the problem was not shipped in 3.0.11. It has however been
> landed ready to ship in 3.0.12 so we should fix this before 3.0.12 ships
Thanks for the confirm, mossap, that helps.
Updated•15 years ago
|
Flags: wanted1.8.1.x-
Comment 17•15 years ago
|
||
(In reply to comment #10)
> At Mossop's suggestion I just tried using 'extv=3.5rc1' in the snippets, and
> that made the update work.
So this is a manual edit to the snippets? How will it appear to users?
> I don't know what the implications are for extensions if we actually ship with
> those, though.
Can we get someone to test / confirm what happens here? I'd like to think those called 3.5.* will still be compatible, but that last "." scares me.
Comment 18•15 years ago
|
||
(In reply to comment #11)
> Rob, there is some inconsistency between how bug 313057 does its version
> comparison and how bug 485624 does it. The former uses update.extensionVersion
> (which is what is written to update.version) and compares that to the
> application version on startup. The latter compares update.version to the
> application version during running.
>
> We should I think make this consistent, maybe even while we were here. If we
> made them both use update.extensionVersion then it would actually allow build
> to go back to using any version name they liked in update.version which would
> resolve bug 496917 even more nicely. But I'm a little fuzzy on what the
> semantics of update.version and update.extensionVersion are meant to be, can
> you clarify?
I mistakenly used update.version instead of update.extensionVersion in bug 485624 which caused bug 496917.
extensionVersion was added so arbitrary string could override version so extensionVersion would always contain a version string... the years haven't been kind to the update xml.
Updated•15 years ago
|
Attachment #383294 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 19•15 years ago
|
||
Landed on m-c: http://hg.mozilla.org/mozilla-central/rev/1224a2fdf826
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 20•15 years ago
|
||
Landed on branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1aa4492403b2
Keywords: fixed1.9.1
Comment 21•15 years ago
|
||
Dave, how can we verify the fix? We need any build after this checkin, or?
Assignee | ||
Comment 22•15 years ago
|
||
(In reply to comment #21)
> Dave, how can we verify the fix? We need any build after this checkin, or?
Well this is a bit hacky, but it should give you a result:
Get a 3.5pre tinderbox build (building now)
Set app.update.url.override to https://aus2.mozilla.org/update/3/Firefox/3.5b99/20090605144508/Darwin_Universal-gcc3/en-US/betatest/Darwin%209.7.0/default/default/update.xml to make AUS think it is 3.5b99 on the betatest channel.
Do an update check
It should download the partial, let you restart, fail to apply the partial (because it is against the wrong build), download the complete, let you restart and then you'll magically have the version that the betatest channel is currently offering (3.5rc1build1 right now)
Assignee | ||
Comment 23•15 years ago
|
||
Pushed to the relbranch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bfada7b09c1b
Comment 24•15 years ago
|
||
Comment on attachment 383294 [details] [diff] [review]
patch rev 1
Should get this for 1.9.0 as well
Attachment #383294 -
Flags: approval1.9.0.12?
Comment 25•15 years ago
|
||
Comment on attachment 383294 [details] [diff] [review]
patch rev 1
Approved for 1.9.0.12, a=dveditz for release-drivers
Attachment #383294 -
Flags: approval1.9.0.12? → approval1.9.0.12+
Comment 26•15 years ago
|
||
Checked in for 1.9.0.12
Checking in mozilla/toolkit/xre/nsUpdateDriver.cpp;
/cvsroot/mozilla/toolkit/xre/nsUpdateDriver.cpp,v <-- nsUpdateDriver.cpp
new revision: 1.28; previous revision: 1.27
done
Keywords: fixed1.9.0.12
Comment 27•15 years ago
|
||
Juan, can we verify this fix with a current or the latest updates? Looks like everything works as expected, right?
Target Milestone: --- → mozilla1.9.2a1
Comment 28•15 years ago
|
||
Is there a way to verify this for 1.9.0 (as opposed to 1.9.1) since this in Firefox 3.0.12?
You need to log in
before you can comment on or make changes to this bug.
Description
•