Closed Bug 534954 Opened 10 years ago Closed 10 years ago

Generate nightly updates for project branches

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: joduinn, Assigned: nthomas)

References

Details

Attachments

(7 files, 2 obsolete files)

For mozilla-central/mozilla-191/mozilla-192, we generate nightly builds, and nightly updates. However, for project branches (tracemonkey, places, etc) we only generate nightly builds - without any nightly updates to bring users forward from one build to the next.

Quick summary of issues so far:
1)  If we attempt to enable nightly updates for project branches, users will get updated to the next nightly update on the branch which project branch is based *from*. For example, a tracemonkey nightly user would be updated to mozilla-central nightly. To avoid this, we need to use different version/channel info.

2) The updater code allows us to upgrade a user from 1.9.0.16 -> 1.9.1.6. However, it does not currently (afaict) allow users to downgrade from 1.9.1.6->1.9.0.16 (this may be a safety feature, but unconfirmed). This also means we cant downgrade from a fictitious-large-number-in-the-future (like 4.0) down to a real 1.9.1.6. It also means we dont know what will happen if/when we finally do ship a real FF4.0. 

3) According to morgamic, AUS should be able to handle having alphanumberic strings for version (version=="tracemonkey" for example). However, its unclear what the client side updater code could handle that. 

In discussions with nthomas while waiting in SFO last week, one idea to get around the problems above was to not use alpha-numeric versions, and not use fake version numbers. Instead, use the version from the branch being based *from*, and use a different AUS channel for that specific project branch. For example, tracemonkey is based from mozilla-central, so have tracemonkey use:

version = "1.9.3a1pre" (or maybe even "1.9.3a0pre"?)
channel = nightly-tracemonkey.

Pro:
====
* This feels safe from point-of-view of AUS and client-side-updater code. 
* This handles cases where some project branches are based from m-c, while some planned upcoming project branches will be based from various release branches. 
* When a project branch ends, and all patches are landed back to production branches, this should allow us to repatriate those nightly users whenever a project branch ends. 

Con:
====
* Unknown impact of having lots of extra nightly channels like this.
* Never been done before, so not sure what testing is involved.



Not sure what else might be involved. Any thoughts?



(ps: after all the irc discussions and emails on this over previous months, I thought we already had a bug on this, but now cant find it. I've found other bugs which morphed into related, but different, installer bug, Please DUP if there is a bug for this RelEng work already and I'm missing it.)
Perhaps use a unique MOZ_APP_DISPLAYNAME instead. The update url contains this and the browser will display this name. On Windows the installer, shortcuts, and install dir will contain this name as well.

As for repatriation, I suspect that project branches will at times add files that don't end up in the main branch and if they don't add these to the removed-files.in app update won't know to remove these files on update from the project branch to the main branch. It might be better to ask the user to uninstall instead due to this.
(In reply to comment #1)
> As for repatriation, I suspect that project branches will at times add files
> that don't end up in the main branch and if they don't add these to the
> removed-files.in app update won't know to remove these files on update from the
> project branch to the main branch.

Can we not force full updates for that case? Or do full updates not clobber unknown files?
They don't... if we clobbered unknown files then we would clobber 3rd party plugins for example though I'm sure some of us would see that as a good thing. On the other hand come to think of it, with the components.list a complete update should be fine though it will leave behind cruft.
(In reply to comment #1)
> Perhaps use a unique MOZ_APP_DISPLAYNAME instead. The update url contains this
> and the browser will display this name. On Windows the installer, shortcuts,
> and install dir will contain this name as well.

Would we have to change the in-repo branding to get this to work? With the right config file, we can already change the /Firefox/ part of the update URL without touching any product code.

Will the product accept updates for a different app?
Been discussing this with joduinn off-and-on for the past few weeks.

If we use a unique channel for each project branch (e.g. 'tracemonkey', 'lorentz', etc) and enforce uniqueness even across products (we could resort to 'fennec-e10s' or similar if required), do we still need to worry at all about the product name and/or version?

Our feeling was that unless we come up with a crazy new product at some point, all of these project branches are going to be folded back into the main development line at some point. As such, a project branch isn't going to require different channels for nightlies and alphas and releases. 

In fact, under this scenario, project branches could adopt whatever update period they wanted, with the default being nightly generated updates.
Assignee: nobody → ccooper
Component: Release Engineering: Future → Release Engineering
Priority: -- → P3
I think that we are going to see wanting to do "alpha" or "beta" releases off project branches, for at least some of them.  Happy to discuss that here in this bug, or elsewhere, but am quite biased towards whatever initial state keeps this work from being blocked.  We can create another mini-project-branch for alpha/beta stuff if we want, for example, since spinning them up is pretty much trivial now IIRC.

(#2 in the original comment isn't really a problem, AIUI, which is why we'll be able to rescue the people on 4.0a1pre builds -- we can put a large version number in the update metadata, and they'll accept the update, and it can have whatever internal version it wants after.)
(In reply to comment #6)
> I think that we are going to see wanting to do "alpha" or "beta" releases off
> project branches, for at least some of them.  Happy to discuss that here in
> this bug, or elsewhere, but am quite biased towards whatever initial state
> keeps this work from being blocked.  We can create another mini-project-branch
> for alpha/beta stuff if we want, for example, since spinning them up is pretty
> much trivial now IIRC.

Fair enough, it can be channel-per-project-branch or channels-per-project-branch depending on the release needs of the branch. If the channel-based approach is solid, there's no reason why this would even need to be decided at project branch creation time.

Is the product/version discussion more about how these builds are presented to users, i.e. so they know they're using a project branch build vs. Firefox? AIUI, we can update users to whatever we want given a larger version number and the correct update channel.
Yeah, we will have to figure out how to present that stuff.  The problem exists today, they just stay on a fixed version and we can't merge them back to something else via update trickery.
Duplicate of this bug: 469197
Blocks: 469161
Hardware: x86 → All
Looks like WinMo updates will take precedence over this for now.
Assignee: ccooper → nobody
Component: Release Engineering → Release Engineering: Future
What does that mean for the ETA?  We're planning to use project branches for UI work that really really wants updates.  This bug being in the "very soon now" component led me to believe that it would be viable soon, but now I don't know how to plan around it.
This is part of the whole "how the heck do we version lorentz" problem which is next on my plate.
Assignee: nobody → nrthomas
Component: Release Engineering: Future → Release Engineering
Priority: P3 → P2
(In reply to comment #11)
> What does that mean for the ETA?  We're planning to use project branches for UI
> work that really really wants updates.  This bug being in the "very soon now"
> component led me to believe that it would be viable soon, but now I don't know
> how to plan around it.

Yeah, sorry about that. There was some confusion within releng about who would be picking up this bug. Thanks, Nick.
Attached patch WIP AUS change (obsolete) — Splinter Review
Mike, this patch lets us choose a snippet store for nightly builds based on both the version and the channel. We don't currently store the channel in the directory tree for nightlies, and doing it this way is a smaller change when considering all the systems in play (also a Q1 goal). What do you think ?

Also removes nightly support for the now-shelved WinMo.
Attachment #434452 - Flags: feedback?(morgamic)
Attached patch AUS supportSplinter Review
A small improvement from attachment 434452 [details] [diff] [review], making the obvious change to the Synthetic data source for testing. Don't have a setup to verify that is correct though.

I have confirmed that day-old nightlies for Fx3.0.x, Fx3.5.x, Fx3.6.x, Fx3.7.x and Tb2.0.0.x get the correct partial update from a patched copy of aus2. And that the patched aus2 returns exactly the same same update information as aus2.m.o for each update request. 

Ready for full review.
Attachment #434452 - Attachment is obsolete: true
Attachment #435523 - Flags: review?(morgamic)
Attachment #434452 - Flags: feedback?(morgamic)
Nick, thanks for working on this.  It'll take me a couple of days to test -- when do you need to launch this?
(In reply to comment #16)
Mike, it's a Q1 goal arriving at the last minute, so unfair to make you rush on it. Maybe just into Q2 ? AIUI findPatch() will have returned prior to reaching getBranch() in the release case, but a php guru I am not.
I'll look at it by Tuesday.  Not sure if we'll be able to launch by Thursday though.  We'll see.
That'd be great if you have time, thanks!
(Start of the build side changes)

This is a large patch to break a bunch of symlinks and create actual mozconfigs for electrolysis and tracemonkey nightlies, in order to change the update channel to nightly-electrolysis and nightly-tracemonkey. Those are the only project branches producing nightly builds at the moment, but staging + production make this a big patch.
Attachment #435547 - Flags: review?(ccooper)
This diff is the result of dereferencing the symlinks (with rsync -aL), comparing a clean and modified copy of buildbot-configs. The idea is to show that there are only the nightly channel changes I want, and one minor fix to the unused mozilla2-staging/win32/tracemonkey/xulrunner/mozconfig.
Tested on linux/mac 10.5 & 10.6/win32 for electrolysis (linux64 works on mozilla-central, assuming that carries over).
Attachment #435549 - Flags: review?(ccooper)
Tested on linux/mac 10.5/win32 for electrolysis, both partial creation and getting & applying that partial (via patched-aus on staging-stage) work. 

To land after we have a nightly build for all 3 platforms on both branches.
Attachment #435550 - Flags: review?(ccooper)
Comment on attachment 435547 [details] [diff] [review]
[buildbot-configs] mozconfig changes

Looks good.
Attachment #435547 - Flags: review?(ccooper) → review+
Comment on attachment 435549 [details] [diff] [review]
[buildbot-configs] Create complete mars for electrolysis and tracemonkey

You *might* need to specify aus2_base_upload_dir_l10n even if it's not used, but if this is passing in staging, I guess not.
Attachment #435549 - Flags: review?(ccooper) → review+
Attachment #435550 - Flags: review?(ccooper) → review+
Comment on attachment 435549 [details] [diff] [review]
[buildbot-configs] Create complete mars for electrolysis and tracemonkey

Oh, also, when just creating the snippets on the slave, I always left the aus2_user and aus2_ssh_key pointing to cltbld, but I don't think it's strictly necessary. That's just how the previous system did it.
Comment on attachment 435549 [details] [diff] [review]
[buildbot-configs] Create complete mars for electrolysis and tracemonkey

http://hg.mozilla.org/build/buildbot-configs/rev/768b6668bff2

I added aus2_base_upload_dir_l10n for completeness. I think I should leave the user as ffxbld since I'm also pushing into .../incoming/2/. Could well end up with permissions problem if the solitary complete snippet goes in as cltbld, and the later partial can't overwrite as ffxbld. I could switch to cltbld and push to build/, but don't want to give old partial generator a chance to screw things up.
Attachment #435549 - Flags: checked-in+
pm02 reconfig'd. We should get completes with the 2010-03-30 nightlies, but not visible to anyone.
Attachment #436072 - Flags: review?(ccooper)
Attachment #436072 - Flags: review?(ccooper) → review+
Attachment #436072 - Flags: checked-in+
Committed 
  http://hg.mozilla.org/build/buildbot-configs/rev/0df4670ccf5a
to put back the shark mozconfigs.
Comment on attachment 435550 [details] [diff] [review]
[buildbot-configs] Enable partial generation on the build slave

http://hg.mozilla.org/build/buildbot-configs/rev/263140adbdd5

pm02 reconfig'd. Ball is in morgamic's court now.
Attachment #435550 - Flags: checked-in+
(In reply to comment #32)
> (From update of attachment 435550 [details] [diff] [review])
> http://hg.mozilla.org/build/buildbot-configs/rev/263140adbdd5
> 
> pm02 reconfig'd. Ball is in morgamic's court now.

morgamic - ping on the r?

(nthomas is out until 19th, so if you have questions, can you ping catlee/myself?)
Assignee: nrthomas → morgamic
(In reply to comment #33)
> (In reply to comment #32)
> > (From update of attachment 435550 [details] [diff] [review] [details])
> > http://hg.mozilla.org/build/buildbot-configs/rev/263140adbdd5
> > 
> > pm02 reconfig'd. Ball is in morgamic's court now.
> 
> morgamic - ping on the r?

morgamic: re-ping?
As I understand it, our cases are:
1. nightly updates offered for existing nightlies as-is (there are many tests for this)
2. for designated nightly branches, nightly updates are offered if the corresponding directories contain updates (needs new tests)

There's another case I need clarified:
3. for a nightly branch that is undefined, no update should be offered

or

3. for a nightly branch that is undefined, updates should be offered for the main branch

Our existing nightly test cases are:
   1. outdated (>1 off) update serving only a complete (1.5.0.5->1.5.0.x)
   2. one-off update serving both complete and partial (1.5.0.5->1.5.0.x)
   3. client already at the latest build requests an update (no update should be offered)
   4. version with no branchVersion in AUS config (6.0->?)
(In reply to comment #35)
> There's another case I need clarified:
> 3. for a nightly branch that is undefined, no update should be offered
> or
> 3. for a nightly branch that is undefined, updates should be offered for the
> main branch
In this situation, no update should be offered. This is an error case, and not sending updates is safer here, imho.


> Our existing nightly test cases are:
>    1. outdated (>1 off) update serving only a complete (1.5.0.5->1.5.0.x)
ok.
>    2. one-off update serving both complete and partial (1.5.0.5->1.5.0.x)
ok.
>    3. client already at the latest build requests an update (no update should
> be offered)
No updates should be offered.
>    4. version with no branchVersion in AUS config (6.0->?)
No updates should be offered.
Alright, talked to John.  Our cases:
1. nightly updates offered for existing nightlies as-is (there are four test cases
for this)
2. for designated nightly branches, nightly updates are offered if the
corresponding directories contain updates (needs new tests)
3. for a nightly branch that is undefined, no update should be offered

The rest of the tests (230 of them) cover default behaviors and show no changes due to the patch.
Could this eventually be used to give updates to project branches that only have checkin-triggered builds?
Blocks: 518508
(In reply to comment #38)
> Could this eventually be used to give updates to project branches that only
> have checkin-triggered builds?

No. This bug is to handle how we generate updates for nightlies for project branches.

We dont generate updates for checkin-triggered builds (unlike nightly builds, where we do generate updates), and given the number of checkin-triggered builds, its unclear we would want to do this. For sparse branches, if updates were needed, it would be easier & safer to do a nightly (with updates) when needed.

Not sure if that answers your question - but feel free to followup with me offline (and not confuse this bug).
(In reply to comment #37)
> Alright, talked to John.  Our cases:
> 1. nightly updates offered for existing nightlies as-is (there are four test
> cases
> for this)
> 2. for designated nightly branches, nightly updates are offered if the
> corresponding directories contain updates (needs new tests)
> 3. for a nightly branch that is undefined, no update should be offered
> 
> The rest of the tests (230 of them) cover default behaviors and show no changes
> due to the patch.

morgamic: sounds good. whats next step?
Comment on attachment 435523 [details] [diff] [review]
AUS support

Nick, I'm really sorry it's taken so long.  Comments below:

patch.class.php:
338         if (!isset ($this->productBranchVersions[$product]) && !isset ($this->productBranchVersions[$product][$channel])) {
339             return false;
340         }
341         foreach ($this->productBranchVersions[$product][$channel] as $versionPattern => $branch) {

To prevent PHP notices you should check to make sure these are actually arrays before performing array-specific actions on them (foreach, accessing an index, etc.).  Acceptance tests caught PHP notices that generated 17 errors (7% failures out of a total of 230).

Notices/warning generated that should be avoided:
Notice:  Undefined index:  channel in /home/morgamic/public_html/aus/inc/patch.class.php on line 341
Warning:  Invalid argument supplied for foreach() in /home/morgamic/public_html/aus/inc/patch.class.php on line 341

Suggested changes:
getBranch should check to make sure the nightly channel config is actually an array before looping through it.

    function getBranch($product,$version,$channel) {
        if (!isset($this->productBranchVersions[$product])
            || !isset($this->productBranchVersions[$product][$channel])
            || !is_array($this->productBranchVersions[$product][$channel])) {
            return false;
        }

Nightly test cases are breaking, but this is probably expected due to the change in behavior.  But this shows that we're not making changes in a way that is backwards-friendly.  The changes cause the following nightly tests to fail:

http://people.mozilla.org/~morgamic/aus_failures.png
Attachment #435523 - Flags: review?(morgamic) → review-
Have we already landed any of those patches? I wonder if I see a fallout from that bug or not:

The last weeks I was using builds from the addonsmgr project branch. Everything was fine until I got update offers to regular nightly builds lately. Right now I always get updated to trunk builds.

Steps:
1. Install any of those builds: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/addonsmgr-win32/
2. Wait until the update offer shows up or check for updates manually.

We shouldn't offer updates which take users away from project branches.
(In reply to comment #42)
> Have we already landed any of those patches? 

The change to the update server hasn't landed yet, needs some improvements to pass review.

> The last weeks I was using builds from the addonsmgr project branch. Everything
> was fine until I got update offers to regular nightly builds lately. Right now
> I always get updated to trunk builds.

AFAIK nothing has changed that meant we started offering updates to project branches. More recent nightly builds for tracemonkey and electrolysis would not be getting offered updates (on account of using a different channel) when they would have before.

> We shouldn't offer updates which take users away from project branches.

I agree, except when project branch closes after the changes have been merged back to the parent. 

The whole point of this bug is to create builds which AUS can tell apart, and then for it to provide the correct nightly updates. That will prevent branch users being sent back to the parent. However you have made me realize that for branches like addonsmgr and places, which we don't do nightlies for, we've left the channel as 'nightly'. And that will mean they still get repatriated to mozilla-central. Will have to figure something out there.
(In reply to comment #41)
Thanks for your commments Mike. I will try modifying productBranchVersions in a way which is backwards compatible, which implies all the is_array checking to support both scalar and array at the product version level.
Attached patch AUS support, v2 (obsolete) — Splinter Review
This patch does almost the right thing for my set of test URLs for Firefox nightly builds. The known problem is that if you use an old style config then you can get an update with something like version=3.6.4pre and channel=nightly-tracemonkey. ie anything in $nightlyChannels gets served an update. Could do something like 
  if (is_string($branches) && ($channel == 'nightly')) {
but that seems a bit arbitrary given $nightlyChannels could be quite different for other consumers. Ideas ? How general do we really need to make this ?

The config-test.php changes are just a stab. I haven't looked at how that interacts with the aus/tests and aus/sanity directories. If aus/tests/README is still up to date I can take a stab at running the tests locally.
Assignee: morgamic → nrthomas
Status: NEW → ASSIGNED
Attachment #443590 - Flags: feedback?(morgamic)
Attached patch AUS support, v2Splinter Review
Adds passing tests and doesn't regress old ones - upgrading to a review request.
Attachment #443590 - Attachment is obsolete: true
Attachment #443849 - Flags: review?(morgamic)
Attachment #443590 - Flags: feedback?(morgamic)
Duplicate of this bug: 559105
No longer blocks: 469161
Duplicate of this bug: 469161
No longer blocks: 559105
morgamic, could you please take a look at attachment 443849 [details] [diff] [review] ?
Comment on attachment 443849 [details] [diff] [review]
AUS support, v2

This looks pretty awesome.  I found that some of the test data isn't matching up with the Verify.txt strings (complete000 vs. complete001 for example).  These mismatches are causing the 8 new tests to fail.
Attachment #443849 - Flags: review?(morgamic) → review-
Some additional test observations/notes:

For the first 2.0.0.x test, it's asserted that no partial will be offered but there is one.  Is that expected?  A complete with no partial seems odd.

Same for the 4th 2.0.0.x test.  Complete with no partial asserted.  Should probably flip both to true in the hasPartial column.

2nd and 5th 2.0.0.x tests both fail with no updates.  I noticed that the complete and partial are coming from different directories and the build on each is different.  Need to figure out why these keep failing.
Comment on attachment 443849 [details] [diff] [review]
AUS support, v2

Looks like patch is failing to create the zero-byte files in 2.0.0.x{,-branch}/platform/1100000003/locale/, even though CVS put them in the diff. Which screws up AUS's idea about what the newest build is, and you get partials when you shouldn't.

If I take a fresh checkout, apply the patch, do
 cd tests/data/2/Synthetic/
 mkdir -p 2.0.0.x{,-branch}/platform/1100000003/locale/
 touch 2.0.0.x{,-branch}/platform/1100000003/locale/{partial,complete}.txt
(and update the fitnesse wiki + symlink config-test.php) then the tests all pass.
Attachment #443849 - Flags: review- → review?
Attachment #443849 - Flags: review? → review?(morgamic)
Comment on attachment 443849 [details] [diff] [review]
AUS support, v2

This works for me -- gotta figure out a way to store the data right, I guess.  Though, I'd not be opposed to replacing the filesystem with a better form of storage.

Nick - thanks for sending review.  I didn't look at it after comment #52 until today, so feel free to ping/call/slap me if I take that long on something.  <3
Attachment #443849 - Flags: review?(morgamic) → review+
Thanks for the review Mike. Should this go out in an IT downtime, perhaps combined with bug 507408 ? Will be a bit fun to tag AUS2_PRODUCTION, although I could do that manually on the files I've changed.
I don't see a reason to not push both at the same time.  Do you want me to tag it?
Comment on attachment 443849 [details] [diff] [review]
AUS support, v2

Checked in at 2010-05-25 19:51:

File 	Rev
mozilla/webtools/aus/xml/inc/config-dist.php 	1.73
mozilla/webtools/aus/xml/inc/config-test.php 	1.2
mozilla/webtools/aus/xml/inc/patch.class.php 	1.24
mozilla/webtools/aus/tests/Verify.txt 	1.15
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000001/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000001/locale/partial.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000002/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000002/locale/partial.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000003/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x/platform/1100000003/locale/partial.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000001/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000001/locale/partial.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000002/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000002/locale/partial.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000003/locale/complete.txt 	1.1
mozilla/webtools/aus/tests/data/2/Synthetic/2.0.0.x-branch/platform/1100000003/locale/partial.txt 	1.1
Attachment #443849 - Flags: checked-in+
(In reply to comment #55)
> I don't see a reason to not push both at the same time.  Do you want me to tag
> it?

I've verified the changes for this bug work as expected against the real datastores on aus2-staging. But spotted something to check with bug 507408 when we point to non-bouncer hosts (more details over there).

When it comes time to tag I'd go
  cvs tag AUS2_RTM_201005DDHHMM xml/
  cvs tag -F AUS2_PRODUCTION xml/
assuming that matches up with your normal tactics. Previously I'd been tagging all of aus/ instead of just xml/. :-(
That's what I normally do.
I had to temporarily back out the AUS patch to config-dist.php in order to safely land an emergency throttling patch. I re-landed the patch, so it should in the same state as before.
Depends on: 569160
Blocks: 569160
No longer depends on: 569160
No longer blocks: 518508
Comment on attachment 443849 [details] [diff] [review]
AUS support, v2

Tagged and deployed in bug 569160.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 581791
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.