Can't include an arbitrary version name in the version field of the update xml

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Toolkit
Application Update
P1
blocker
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bhearsum, Assigned: mossop)

Tracking

({fixed1.9.0.12, regression})

1.9.1 Branch
mozilla1.9.2a1
fixed1.9.0.12, regression
Points:
---
Bug Flags:
blocking1.9.1 -
blocking1.9.0.12 +
wanted1.9.0.x +
in-testsuite +

Firefox Tracking Flags

(status1.9.1 .2-fixed)

Details

(Whiteboard: [1.9.0 fix in bug 485624])

Attachments

(5 attachments)

(Reporter)

Description

9 years ago
However, 3.1b3 -> 3.5b99 does work.

I've also tried temporarily removing the 3.5b4 -> 3.5b99 partial update to make sure a complete was served - that didn't help.
(Reporter)

Updated

9 years ago
Priority: -- → P1
This is a blocker
Severity: normal → blocker
OS: Mac OS X → All
Hardware: x86 → All
(Reporter)

Comment 2

9 years ago
Mossop did some testing and says he thinks it's a code problem, I'm going to continue looking at a possible cause in the snippets/aus/etc, but we should look at possible code problems too.

Here's the pushlog query for everything between 3.1b3 and 3.5b4:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=FIREFOX_3_1b3_RELEASE&tochange=FIREFOX_3_5b4_RELEASE

There's not many update related changesets, here's the ones I found:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b05bd6c2858
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/dc680328af08
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/300fca604d16
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2176123cd7c0
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/25e053fc15b3
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/99a0415dbd3a
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a48dcceb93b8
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/55f67feda326


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2176123cd7c0 is the most substantial change, afaict.
Assignee: bhearsum → nobody
Component: Release Engineering → Application Update
Product: mozilla.org → Toolkit
QA Contact: release → application.update
Version: other → 1.9.1 Branch
(Reporter)

Comment 3

9 years ago
Some additional information
3.1b3 update url: https://aus2.mozilla.org/update/3/Firefox/3.1b3/20090305152042/WINNT_x86-msvc/en-US/betatest/Windows_NT%205.1/default/default/update.xml?force=1
3.5b4 update url: https://aus2.mozilla.org/update/3/Firefox/3.5b4/20090423204732/WINNT_x86-msvc/en-US/betatest/Windows_NT%205.1/default/default/update.xml?force=1
Dump from the console with app.update.log.all = true:
*** AUS:SVC UpdateService:canUpdate - testing /Users/bhearsum/Desktop/Firefox.app/Contents/MacOS/update.test
*** AUS:SVC UpdateService:canUpdate - testing /Users/bhearsum/Desktop/Firefox.app/Contents/MacOS/updates/0/update.test
*** AUS:SVC UpdateService:canUpdate - able to update
*** AUS:SVC General:readStatusFile - Reading Status File: /Users/bhearsum/Desktop/Firefox.app/Contents/MacOS/updates/0/update.status
*** AUS:SVC UpdateService:_postUpdateProcessing - no status, no update
*** AUS:SVC General:getLocale - getting locale from file: /Users/bhearsum/Desktop/Firefox.app/Contents/MacOS/updater.ini, locale: en-US
*** AUS:SVC Checker:getUpdateURL - update URL: https://aus2.mozilla.org/update/3/Firefox/3.5b4/20090423191946/Darwin_Universal-gcc3/en-US/betatest/Darwin%209.7.0/default/default/update.xml?force=1
*** AUS:SVC Checker:checkForUpdates - sending request to: https://aus2.mozilla.org/update/3/Firefox/3.5b4/20090423191946/Darwin_Universal-gcc3/en-US/betatest/Darwin%209.7.0/default/default/update.xml?force=1
*** AUS:SVC Checker:onLoad - request completed downloading document
*** AUS:SVC Checker:getUpdateURL - update URL: https://aus2.mozilla.org/update/3/Firefox/3.5b4/20090423191946/Darwin_Universal-gcc3/en-US/betatest/Darwin%209.7.0/default/default/update.xml?force=1
*** AUS:SVC Checker:onLoad - number of updates available: 1
*** AUS:SVC UpdateService:downloadUpdate - removing update for previous application version 3.5 Beta 99 (build 2)
*** AUS:SVC UpdateService:addDownloadListener - no downloader!
(Assignee)

Comment 4

9 years ago
I think the problem now is actually in the update XML, not the code. The version attribute is currently "3.5 Beta 99 (build 2)", but I believe we expect this to be a real application version, so when we look at the available updates it appears that the update we are downloading is for an older version than that already installed (3.5b4). This will upset the changes in bug 485624.
(Assignee)

Comment 5

9 years ago
That said we probably shouldn't be left in this crappy state when this happens. Means we are probably not checking that the update offered is less than the currently installed version when we do an update check.
(Reporter)

Comment 6

9 years ago
(In reply to comment #4)
> I think the problem now is actually in the update XML, not the code. The
> version attribute is currently "3.5 Beta 99 (build 2)", but I believe we expect
> this to be a real application version, so when we look at the available updates
> it appears that the update we are downloading is for an older version than that
> already installed (3.5b4). This will upset the changes in bug 485624.

I just confirmed this behaviour by temporarily changing the OS X 3.5b4 snippets to use version=3.5b99. We can do this for the purposes of getting 3.5b99 out but we'll have to do the same thing for the 3.5b4 and 3.5b99 snippets on at least the beta and betatests channels when we do rc1 - since neither will work with version=3.5 RC 1
(Assignee)

Comment 7

9 years ago
What we should be doing is moving to have AUS output the nice name for the version in the update xml in the name attribute (bug 359082 I believe) and keep the version attribute for the real app version since we need to rely on knowing that. Unfortunately the UI is slightly broken and won't use the nice name currently, though it is a very easy fix.
(Assignee)

Updated

9 years ago
Status: ASSIGNED → NEW
Summary: updates from 3.5b4 -> 3.5b99 are not working on any platform → Can't include an arbitrary version name in the version field of the update xml
(Assignee)

Comment 8

9 years ago
Created attachment 382170 [details] [diff] [review]
fix update name display

If we want to continue to be able to display arbitrary version names to the user in the update dialog then we have basically two options that I can see:

We could back out bug 485624 which is the cause of the brokenness we see right now. It isn't really a broken patch, it is doing what it is intended, we are just throwing bogus data into the update system with the current method of showing pretty version names.

We could change AUS to include the pretty version as the name attribute in the XML (not bug 359082 at all), and we would also need to patch the client with the attachment. This isn't ideal either, the patch only affects the main display of the version, there are a couple of other places where it would still be displayed as the real version number, but fixing those looks to be a string change.
(Assignee)

Comment 9

9 years ago
Created attachment 382171 [details] [diff] [review]
ignore old update versions

Additionally, if we keep bug 485624 then we ought to consider taking this patch which makes us ignore old versions in the available update list and causes the broken UI we see with that.
(Assignee)

Comment 10

9 years ago
We ought to block until we decide what the appropriate course of action is here I think.
Flags: blocking1.9.1?
Flags: blocking1.9.0.12?
(Reporter)

Comment 11

9 years ago
Will this support the release candidate case? In that case, we'll have appv=3.5rc1, extv=3.5, and the internal version number will be '3.5'.
(Assignee)

Updated

9 years ago
Blocks: 485624
(Assignee)

Comment 12

9 years ago
(In reply to comment #11)
> Will this support the release candidate case? In that case, we'll have
> appv=3.5rc1, extv=3.5, and the internal version number will be '3.5'.

So, RC's work fine, if you set the version field to be what it really is, "3.5". In that case for the first RC any user of a beta or lower would see "Firefox 3.5" offered for install and would install it. Later when RC2 is available (also with a version of "3.5" in the XML), all users of RC1 or betas or lower would see it and get to install it, still named "Firefox 3.5".

It gets hairy if you try to actually use "3.5rc1" in the version field so the update dialog displays it as "Firefox 3.5rc1". That would work for the first RC. But if/when the second came along, the application running would be 3.5, it would see the available update as 3.5rc2 which is lower than 3.5 so we hit the failure state.
(In reply to comment #12)
> So, RC's work fine, if you set the version field to be what it really is,

Sounds fine to me.  Let's stay on the cowpaths here if we can; that's a major motivation for doing b99 instead of pre.
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Keywords: regression
By my understanding of comment 12 and comment 13, this is no longer a blocking issue, so minusing. Renom if I've misinterpreted.
Flags: blocking1.9.1? → blocking1.9.1-
(Assignee)

Comment 15

9 years ago
(In reply to comment #14)
> By my understanding of comment 12 and comment 13, this is no longer a blocking
> issue, so minusing. Renom if I've misinterpreted.

If you want to be able to do the sort of version strings in the update dialog that we have been using for security updates up till now ("3.0.12 (build 1)" f.e.) then we need to fix this in the client before release. If you don't want to be able to do that anymore then this is basically a wontfix.
Flags: blocking1.9.1- → blocking1.9.1?

Updated

9 years ago
Duplicate of this bug: 497073
(In reply to comment #15)
> (In reply to comment #14)
> > By my understanding of comment 12 and comment 13, this is no longer a blocking
> > issue, so minusing. Renom if I've misinterpreted.
> 
> If you want to be able to do the sort of version strings in the update dialog
> that we have been using for security updates up till now ("3.0.12 (build 1)"
> f.e.) then we need to fix this in the client before release. If you don't want
> to be able to do that anymore then this is basically a wontfix.

That's a false dichotomy. I don't think this issue blocks the final release. Right now it affects users on the beta channel, and while it sucks, I don't think we should keep Firefox out of the hands of the primary audience to fix it.

That said, I'd totally take a fix if one comes along before the other blockers are finished :)
Flags: wanted1.9.1.x+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
(Assignee)

Updated

9 years ago
Attachment #382170 - Flags: review?(robert.bugzilla)
(Assignee)

Comment 18

9 years ago
Comment on attachment 382170 [details] [diff] [review]
fix update name display

Well this is the simple fix that would allow us to display a custom name for the update in the UI
(Assignee)

Comment 19

9 years ago
Comment on attachment 382171 [details] [diff] [review]
ignore old update versions

And this makes us ignore updates that we would eventually fail to install anyway
Attachment #382171 - Flags: review?(robert.bugzilla)
(Reporter)

Comment 20

9 years ago
Dave, do we need any AUS changes to support these patches? What does 'appv' need to look like in the snippets?
(Assignee)

Comment 21

9 years ago
(In reply to comment #20)
> Dave, do we need any AUS changes to support these patches? What does 'appv'
> need to look like in the snippets?

Yes, in order to display a custom name in the update dialog the xml will need to contain a name="" attribute on the update element. Unfortunately that needs to be the full update name i.e. "Firefox 3.5" rather than just "3.5" which probably complicates AUS a little. Let's see what Rob thinks of the patch before we start digging too deep in there.
(Reporter)

Comment 22

9 years ago
(In reply to comment #21)
> Let's see what Rob thinks of the patch
> before we start digging too deep in there.

Okay. I want to note that this will require both AUS and automation tooling changes - we should think hard about whether we want to do something like that this late in the cycle.
(Assignee)

Comment 23

9 years ago
(In reply to comment #22)
> (In reply to comment #21)
> > Let's see what Rob thinks of the patch
> > before we start digging too deep in there.
> 
> Okay. I want to note that this will require both AUS and automation tooling
> changes - we should think hard about whether we want to do something like that
> this late in the cycle.

If we take the client patch everything will continue to behave as it does at the moment. It will merely give us the option to make AUS and automation changes to use custom version names, which we can choose to do at any time after the client has been patched, not necessarily during the current release cycle.
Assignee: nobody → dtownsend
Comment on attachment 382171 [details] [diff] [review]
ignore old update versions

Could you create a unit test for this as well?
Attachment #382171 - Flags: review?(robert.bugzilla) → review+
Attachment #382170 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 25

9 years ago
Created attachment 382487 [details] [diff] [review]
testcases

This adds testcases to make sure we don't select updates for older application versions and we do select updates for the current application version.
Attachment #382487 - Flags: review?(robert.bugzilla)
Comment on attachment 382487 [details] [diff] [review]
testcases

Thanks!
Attachment #382487 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 28

9 years ago
Created attachment 383320 [details] [diff] [review]
follow-up patch

Per bug 498379 comment 18, let's fix this so we use extensionVersion for the version comparisons so we can use the plain version for our own evil reasons again.
Attachment #383320 - Flags: review?(robert.bugzilla)
Attachment #383320 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 29

9 years ago
Landed the follow-up: http://hg.mozilla.org/mozilla-central/rev/d18a8082a617

This is now resolved on trunk.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [1.9.0 fix in bug 485624]
Fixed for 1.9.0.12 by the landing of the followup patch in bug 485624
Keywords: fixed1.9.0.12
(Assignee)

Updated

9 years ago
Target Milestone: --- → mozilla1.9.2a1
We still need this on 1.9.1, no? It only landed on the release branch, not on the 1.9.1 branch?
(Assignee)

Comment 32

9 years ago
Created attachment 387148 [details] [diff] [review]
rollup patch

This is a combined patch of all the above patches that were landed on trunk. I think it serves us to take this for 1.9.1.1 to keep ourselves in sync as much as possible. This code has been on trunk now for a while with no noticeable problems.
Attachment #387148 - Flags: review+
Attachment #387148 - Flags: approval1.9.1.1?
Comment on attachment 387148 [details] [diff] [review]
rollup patch

a1912=beltzner
Attachment #387148 - Flags: approval1.9.1.1? → approval1.9.1.2+
status1.9.1: --- → wanted
Flags: wanted1.9.1.x+
Pushed to mozilla-1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9fd8f0e6e6f5
status1.9.1: wanted → .2-fixed
Flags: in-testsuite+
(Reporter)

Comment 35

9 years ago
Juan asked me to comment here on verification of this in 3.5.2:
We don't really have a way to do any verification because we have no updates available for 3.5.2 builds. I might be able to hack up something manually if it's really important to verify...but we aren't using the features this enables anymore (now that we're out of the beta period), so it doesn't seem greatly useful.
You need to log in before you can comment on or make changes to this bug.