410006 - PatcherConfig should generate pretty version strings

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Robert Helmer [:rhelmer]]

Over in bug 359082, there's a desire to have nicer-looking version strings in the Firefox update UI. For example, instead of "Firefox 3.0b2" the update dialog would prompt the user to update to "Firefox 3 Beta 2".

The product name comes from the client, but AUS can provide the "3 Beta 2" part. There have been requests for more fanciful strings too.

We could attempt to automatically expand this (e.g. "3.0a2" == "3 Alpha 2") but maybe it's just better and safer to provide a way to override the "version" string in the snippet, and fall back to the old (current) behavior if not specified.

Took a quick look in PatcherConfig, I think we'd just need to do a check here:
http://mxr.mozilla.org/mozilla/source/tools/release/Bootstrap/Step/PatcherConfig.pm#212

extension-version should continue to point to $version, but version could point to e.g. "aus_version" in bootstrap.cfg if present, otherwise fall back.

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: untested, add ability to override patcher version with pretty version

I think this is all it should take to allow a gussied-up "version" string for the version in the client UI. 

If "pretty_aus_version" var is present in bootstrap.cfg use it, otherwise fall back to current behavior.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 294702 [details] [diff] [review]
untested, add ability to override patcher version with pretty version

This looks fine, glad to hear that it just works in patcher. Would it be useful to use a var called prettyVersion, and also use this for file renaming while staging (rather than hard coding it). It would also match the interCaps style for other vars.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

See also Ben's changes in bug 409394.

Comment 4

[Robert Helmer [:rhelmer]]

(In reply to comment #3)
> See also Ben's changes in bug 409394.

Hmm, yeah I thought about this, but I'm not sure if we should auto-generate the prettyName, because we may want it to say something fancier. Maybe we should auto-generate the fallback name? That makes less work for us in the beta/alpha case.

Comment 5

[Robert Helmer [:rhelmer]]

Attachment: [checked in] use prettyAusVersion not pretty_aus_version

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 295848 [details] [diff] [review]
[checked in] use prettyAusVersion not pretty_aus_version

Looks good, r+. Do you think we are settled enough on "Firefox 3 Beta 3" to specify prettyAusVersion in fx-moz19-*bootstrap.cfg ?

Comment 7

[Robert Helmer [:rhelmer]]

Comment on attachment 295848 [details] [diff] [review]
[checked in] use prettyAusVersion not pretty_aus_version

Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.pm
new revision: 1.13; previous revision: 1.12
done

Comment 8

[Robert Helmer [:rhelmer]]

Hm, so as-is this will cause the version in the detailsURL to be "3.0 Beta 3" as well. Not sure if this is what we want, it's different than what we do know for sure.

Patcher uses SubstitutePath to set detailsUrl with %version% coming from appv, which is the same string that gets used in the update.xml

If we want detailsUrl/extv to be 3.0b3 and appv to be "3.0 Beta 3", there are a couple ways I can think of to do this:

a) substitute %version% with extv in detailsUrl
b) don't substitute %version% at all, make PatcherConfig specify the desired version

(a) seems like it's using extv (extension compatibility version) in an unintended and potentially surprising way. I'm thinking (b) for this, what do you think Nick? Am I missing a better way here?

Comment 9

[Robert Helmer [:rhelmer]]

Attachment: bump relnotes from PatcherConfig

Bump details url version explicitly in PatcherConfig. This also gives us the option of pointing detailsUrl at the staging boxes, which might be nice to do later for auto-verification purposes.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 297593 [details] [diff] [review]
bump relnotes from PatcherConfig

Of the two proposals, I don't really mind, so this patch seems fine. AFAIK, extv was only different from appv for the 1.5 betas, and AMO has come a long way since then.

Why is there no %locale% between http:// and www. ? We use that in all the 1.8 and 1.8.0-major configs, but not for 1.8.0 and 1.9.

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to comment #10)
> (From update of attachment 297593 [details] [diff] [review])
> Of the two proposals, I don't really mind, so this patch seems fine. AFAIK,
> extv was only different from appv for the 1.5 betas, and AMO has come a long
> way since then.
> 
> Why is there no %locale% between http:// and www. ? We use that in all the 1.8
> and 1.8.0-major configs, but not for 1.8.0 and 1.9. 

I just copied this line from the moz19 config; happy to change this.

Comment 12

[Robert Helmer [:rhelmer]]

Attachment: [checked in] bump relnotes URL from PatcherConfig

Now with http://%locale% !

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 297774 [details] [diff] [review]
[checked in] bump relnotes URL from PatcherConfig

clouserw confirmed that this is the way to go.

Comment 14

[Robert Helmer [:rhelmer]]

Comment on attachment 297774 [details] [diff] [review]
[checked in] bump relnotes URL from PatcherConfig

Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.pm
new revision: 1.14; previous revision: 1.13
done

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: result from staging

Comment 16

[Robert Helmer [:rhelmer]]

Looks good to me. Note in this tests I've attached, version should be "3 Beta 3" not "Firefox 3 Beta 3", but that's a configuration issue. I'll retest and either verify or reopen this bug, but I think that this test is enough to show that the patch works as intended.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Super hot!

Comment 18

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Remove hard coded "firefox" in details

Attachment 297774 [details] [diff] caused bug 418450 by hardcoding firefox in the URL (super bit of reviewing that!). Switch to using $product instead.

Comment 19

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: [checked in] Remove hard coded "firefox" in details - take 2

Urgh.

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 304327 [details] [diff] [review]
[checked in] Remove hard coded "firefox" in details - take 2

Ouch, sorry :)

Comment 21

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 304327 [details] [diff] [review]
[checked in] Remove hard coded "firefox" in details - take 2

Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.pm
new revision: 1.17; previous revision: 1.16
done