Closed Bug 699335 Opened 13 years ago Closed 13 years ago

Push Camino 2.1rc2 via software update

Categories

(Camino Graveyard :: Product Site, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alqahira, Assigned: stuart.morgan+bugzilla)

Details

Attachments

(5 files, 1 obsolete file)

Since we're going to have rc2, we should try to push it via Software Update.  I think this is possible, since it keys off the CFBundleVersion, but I haven't thought this 100% through yet and have some lingering remembrance of Stuart saying updating wasn't possible past RC (but that may also have been before we implemented min and max version stuff, which I think would protect us).

At the very least, the UI will be a bit wonky ("you have 2.1 (CFBV-1), but 2.1 (CFBV-2) is available"), but for RCs I'm not so concerned about that.  The website also won't be right, since it can't tell RC2 from RC1, only "2.1" from "2.1b2" and so forth, but, again, don't care much, and that could be fixed with a little js-hacking.
OK, my recollection of Stuart was right; we can't currently push an RC2.

Problem 1 seems to be that |version_is_release| only considers a VersionString that ends in "rc" to be a release candidate, so we can't use "2.1rc2" as a VersionString or we'll send *everyone* the update:

http://mxr.mozilla.org/camino/source/camino/tools/autoupdate/update-check#173

I think we can fix that by moving "rc" inside of "(?:a|b)\d*", so (?:a|b|rc)\d*, and then make an additional fix to the regex inside |version_components| so that it parses components properly…

Problem 2 is that, then, having fixed that (or at least the |version_is_release| part), the client version is going to be given to the script as "2.1" for everyone using rc1, which is a release version (because it doesn't end in a*, b*, rc*, or pre), and release versions don't want RCs ;-)

I think "fixing" that is going to require some sort of special-case hack right now, but I'm not sure how/where to write it (in |client_wants_pre_release_builds| maybe?), and I'm not sure what other things might break, presenting us with Problem 3, Problem 4, etc.

The UI is probably still going to be confusing even if we fix that; right now it says "2.1rc2 is available; you have 2.1" (when I've broken update-check-test trying to hack around these problems and it offers the update to 2.0.9, too); I'm not sure if a correct fix/hack can make that better, or if we can't control the "you have" value at all.

:sigh:
Attached patch possible hack (obsolete) — Splinter Review
I *think* this patch, in conjunction with appropriate an update definition (including the right signature) and update description, will serve 2.1rc2 to everyone running 2.0.*pre, 2.1-anything-less-than-2.1, and 2.1rc1.

I'm currently running it at https://caminobrowser.org/update-check-test and it appears to work when I throw 2.1rc1 and 2.0.9 at it.  I'm somewhat skeptical/surprised I didn't hit Problem 4 … Problem N making this work.

Obviously this needs a very thorough review/audit, because I don't remember this code very well from when I used to review Stuart's changes to it, and my brain is not anywhere near up to "I can hack perl if my life depended on it" mode tonight, so trying to follow the code is making my head hurt.

I'll also attach the update description and definition (which still has 2.1rc1's signature) that I have staged for update-check-test to use.

(If this does work, I'm not sure we want to check it in, because a] it's a giant hack, not working support for multiple-RC situations and b] we'll probably want to back out the changes in production once 2.1.1 is ready to go, right?)
Attachment #571882 - Flags: superreview?(stuart.morgan+bugzilla)
Attached file Update Definition
This needs the correct signature before it can be used for real, but the rest of the info should be correct.
Attached file Update Description
This is just s/Candidate 1/Candidate 2/g against the file for rc1.
The hack makes me sad. I'll take a crack at this tonight or early tomorrow; I think what we should do is make it so that X -> XrcY (but not something-less-than-X -> XrcY) is something we'll offer as a general rule, since there's nothing about this situation that's actually unique. I'll have to dig into the code to make sure that won't have weird side effects, but it seems like the right solution in theory.

As for the UI, we're pretty much stuck there. The "you have" value comes from the client.
(In reply to Stuart Morgan from comment #5)
> The hack makes me sad. I'll take a crack at this tonight or early tomorrow

:)

> I think what we should do is make it so that X -> XrcY (but not
> something-less-than-X -> XrcY) is something we'll offer as a general rule,
> since there's nothing about this situation that's actually unique.

One other thing I just thought of; we'll need to make sure that final releases (actually, everybody running anything less than 2.1rc2, but most concerned about 2.0.x release users) less than 2.1 get offered "2.1" instead of "2.1rc2" (in terms of update description and version in the UI) when we open up the update; I'm not sure that would have worked correctly with my hack.
This should do it. It's your regex changes, but instead of the 2.1 hack it makes X->Xrc* a valid update path. It needed a big comment since it's weird, but it still makes me feel better than having the hack, even if we never use it again.

As for what happens when we release 2.1, yours would have been fine too; 2.1 sorts higher than 2.1rc*, so once the 2.1 definition is in place it will be the first one checked, and thus the one offerred.
Attachment #571882 - Attachment is obsolete: true
Attachment #571882 - Flags: superreview?(stuart.morgan+bugzilla)
Attachment #572183 - Flags: feedback?(alqahira)
Comment on attachment 572183 [details] [diff] [review]
update script fix, v2

My perl brain is still not really working, but I did follow the changes, and they seemed to make sense.

I checked this against 2.0.9, 2.1b1, 2.1rc, and 2.1rc2, with and without 2.1's update description installed, and every time we did the right thing.  As we know, the UI is a little wonky for 2.1rc->2.1rc2 and 2.1rc->2.1, but that's life :(

So f/r=ardissone; feel free to land this, and thanks for fixing it sanely :)

So all we need now is for you to sign rc2 and we're off!
Attachment #572183 - Flags: feedback?(alqahira) → feedback+
Landed as http://hg.mozilla.org/camino/rev/121d3c13e8fe.

Sorry about the signing; somehow I didn't make the obvious link from "everything else is right" to "that means the build is done and the URL is there". Signature is:
MC4CFQCmpyE0+Lyf6g6UmOCzXacAQzfXJAIVALVR09OYPAt9T60Zh8aqplwxesdb
2.1rc2 is live!  I'll leave this open until I investigate fixing the cbo js tonight, but otherwise we're done here; thanks, Stuart!
Here's the JS changes (hack!) to make rc1 trigger an alert on the homepage and /start/.  I wish we'd fixed bug 548148, though; it's annoying to do all the changes two places, with slight differences.

Two notes:

1) Need to change this slightly once 2.1 is released; at least the message will have to change, but also the way we do the detection, because right now "latestPreRelease" in versions.js is 2.1 and that's not valid once 2.1 is out ;)

2) Note that buildID is only available in Gecko 1.8.1 or later, so you have to use it carefully or you break things for older versions of Camino.  Luckily, this usage is inside a "latestPreRelease" block, which is guaranteed to only get newer.
-> Stuart, since there was code landed here, and that was his.
Assignee: nobody → stuart.morgan+bugzilla
Status: NEW → RESOLVED
Closed: 13 years ago
Component: General → Product Site
QA Contact: general → product.site
Resolution: --- → FIXED
For reference, here's the state of the js hacks to make sure 2.1rc1 and 2.1rc2 users are all shown the "update to 2.1" message on final 2.1 release.

Once 2.1 is no longer the latest release, we can rip all of the messages out and restore some measure of sanity to those two message files (but we'll always have these two patches here for reference in case we ever have to do this again :P ).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: