Closed Bug 399095 Opened 14 years ago Closed 11 years ago

offer updates between buildX -> buildY builds

Categories

(Release Engineering :: General, defect, P5)

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jbecerra, Assigned: bhearsum)

References

Details

(Whiteboard: [updates] See comment 10)

Attachments

(2 files, 4 obsolete files)

Firefox 2.0.0.5rc1 does not get offered an update to rc2 or anything else.

We may want to offer updates to RC users to the latest RC for that version. The latest RC gets offered an update to the latest released version, in this case and point in time Fx2007.
Assignee: nobody → build
Component: Build Config → Build & Release
Product: Firefox → mozilla.org
QA Contact: build.config → preed
Version: unspecified → other
We're leaving users out in the cold. We should always offer updates for any rc/beta build we do for the users who try those builds out.
Severity: normal → major
I thought AUS had a "fallback" snippet for unexpected release versions/channels. People on non-current, non-nightly, versions should get the complete update to the most recent build. (nightly channels should get the most recent nightly, but that seems to be happening.)

We may need to have certain channels not fallback, such as some of the old-style partner builds, but those should be explicit exceptions.
(In reply to comment #2)
> I thought AUS had a "fallback" snippet for unexpected release
> versions/channels. People on non-current, non-nightly, versions should get the
> complete update to the most recent build. (nightly channels should get the most
> recent nightly, but that seems to be happening.)

Only nightly AUS does this. For releases, each and every release path is explicitly configured. For what it's worth, I think having one "latest release" entry and having all previous releases on that branch automatically fall back to it would be a good thing to explore.

In the interim, we could teach the patcher tool (mozilla/tools/patcher) to do this. Currently we only have one release entry per release, and only use "RC" to refer to major releases (e.g. "2.0RC1", "2.0RC2" and so on).

> We may need to have certain channels not fallback, such as some of the
> old-style partner builds, but those should be explicit exceptions.

This kind of conditional fallback could continue to be done on a per-channel basis, so I don't think that's any reason not to have AUS forward all requests on the "Firefox/2.x" branch to the latest 2.x release that AUS has configured.
Is this a dup of bug383676?
Priority: -- → P3
Assignee: build → nobody
QA Contact: mozpreed → build
From discussion in build triage, this is not a dup. More accurately, this is probably a bug in our current release process. This does end up putting people as orphaned groups, but people are getting orphaned for other reasons too...
Component: Release Engineering → Release Engineering: Future
QA Contact: build → release
We now call these build1, build2, ... instead of rc1, rc2, ... I think we only need this if we push a build to the beta channel and then have to respin for the final. This has happened quite infrequently - Fx3.0.2 and 2.0.0.15 are the examples I can find.

IIRC patcher would need some serious surgery, so the outlay of effort is large for a relatively small gain. AUS3 would probably resolve this by making update paths more flexible.
Summary: offer updates between release candidate builds → offer updates between pre-release builds
We're still doing RCs for major releases (e.g. FF 3.5 or SM 2.0), which have the "release" channel already set but we only offer updates between them on the "beta" channel.
For Firefox, we're defining separate 3.5rc1, 3.5rc2, etc releases and provide updates between them. We add release to the channels list in <current-update> after rc1 so that we bring rc1 people to rc2. That's a process change since Juan originally filed this bug for 2.0rc's. We still have the problem of not updating people from 3.5rc1 build1 to 3.5rc1 build2 automatically.
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Whiteboard: [updates]
Priority: P3 → P5
Summary: offer updates between pre-release builds → offer updates between buildX -> buildY builds
To describe this problem again in light of a recent example:
Firefox 3.6.4 had 7 builds before release, and we were manually creating updates to the latest build.
Whiteboard: [updates] → [updates] See comment 10
I don't think getting this into patcher is going to be worth the effort, since there is upcoming AUS work that will probably deprecate it in https://bugzilla.mozilla.org/show_bug.cgi?id=583244, but it shouldn't be hard to script this or otherwise automate it.
Assignee: nobody → bhearsum
This is a bigger patch than I was expecting to write for this bug, but I think it's got some good ground work in it for other python based scripts we may need in the future.

One fairly major caveat upfront: It doesn't work at all for betas, for two reasons:
- It completely misses locales that don't intersect between version and oldVersion. Generally, this shouldn't be a problem for security&stability releases.
- The assumptions in getOldSnippetDirname are incompatible with useBetaChannel=0. I could work around this, but the locale issue is bad enough that I don't think it's worth it.

It does print out warnings for anything in version's locales that it misses.

I've modified the existing "getPlatformLocales" function in platforms.py. I can't find any current use of it, so there shouldn't be any bustage from that.

I'm not sure yet whether I'm going to attach this to the automation or not. I think I could attach it right inside of the 'updates' builder without _too_ much work. Doing so would get us automated uploading and pushing of snippets. What do you think?
Attachment #469984 - Flags: feedback?(nrthomas)
Comment on attachment 469984 [details] [diff] [review]
python script to generate buildX -> buildY updates

>diff --git a/lib/python/release/platforms.py b/lib/python/release/platforms.py
>+# buildbot -> update platform mapping
>+update_platform_map = {
>+    'linux': 'Linux_x86-gcc3',
>+    'linux64': 'Linux_x86_64-gcc3',
>+    'macosx': 'Darwin_Universal-gcc3',

Another place to cope with Darwin_Universal-gcc3 changing, woo.

>diff --git a/release/generate-candidate-build-updates.py b/release/generate-candidate-build-updates.py
>+            oldVersionBuildid = getBuildID(platform, product, oldVersion,
>+                                           oldBuildNumber,
>+                                           server=stageServer,
>+                                           verbose=verbose)
>+            oldSl = getShippedLocales(product, oldVersion, oldBuildNumber,
>+                                      sourceRepo, hg, verbose)
>+            oldPlatformLocales = getPlatformLocales(oldSl, (platform,))[platform]

Any reason you evaluate these for every channel & platform, rather than just the oldBuildid per platform ? Maybe the readability is better this way. I think I prefer oldBuildID to oldBuildid, even if it's not strictly conforming to the naming scheme.

(In reply to comment #12)
> This is a bigger patch than I was expecting to write for this bug, 

Ah snippets, how you bloat things so. *stab stab*

> One fairly major caveat upfront: It doesn't work at all for betas, for two
> reasons:
> - It completely misses locales that don't intersect between version and
> oldVersion. Generally, this shouldn't be a problem for security&stability
> releases.

I think that's OK. Fixing the new locale issue requires a whole other bunch of code to generate snippets, and what you have here gets us almost all of the way to where we want to be.

> I'm not sure yet whether I'm going to attach this to the automation or not. I
> think I could attach it right inside of the 'updates' builder without _too_
> much work. Doing so would get us automated uploading and pushing of snippets.
> What do you think?

If it's relatively small amount of work then I think it's worth automating. Releases need streamlining again. 

Good stuff Ben.
Attachment #469984 - Flags: feedback?(nrthomas) → feedback+
(In reply to comment #13)
> >diff --git a/release/generate-candidate-build-updates.py b/release/generate-candidate-build-updates.py
> >+            oldVersionBuildid = getBuildID(platform, product, oldVersion,
> >+                                           oldBuildNumber,
> >+                                           server=stageServer,
> >+                                           verbose=verbose)
> >+            oldSl = getShippedLocales(product, oldVersion, oldBuildNumber,
> >+                                      sourceRepo, hg, verbose)
> >+            oldPlatformLocales = getPlatformLocales(oldSl, (platform,))[platform]
> 
> Any reason you evaluate these for every channel & platform, rather than just
> the oldBuildid per platform ? Maybe the readability is better this way. I think
> I prefer oldBuildID to oldBuildid, even if it's not strictly conforming to the
> naming scheme.

I suspect this happened because I changed the order of variables being iterated upon a few times and didn't think to move that block. It should be a no-op to move it down, I'll do that.

I'll give integrating this into ReleaseUpdatesFactory a go, too. Should be fairly simple.
This is completely untested other than a syntax check. I'm testing it while doing a staging run for bug 586837, will have the results tomorrow.

What do you think of integrating like this? I could move it to its own factory too, I suppose. I'm not super happy about the way the pushing is done, but until I sat down to write it I didn't realize that I wouldn't be able to iterate over directory names in the master-side code. This means that we have to use a yucky glob to transfer them, and can't push them from the automation. I don't think this is the end of the world, though.

I _could_ hack up some bash that reads the properties and pushes the right things...but I'd rather not add more of that to our factories.
Attachment #470579 - Flags: feedback?(nrthomas)
Attached patch updated tools patch (obsolete) — Splinter Review
This patch updates the generation script with your suggestions, Nick. And it turns out we can determine almost all of the information we need in the outermost loop.

I also added --generate-partials so we can properly work on 3.5/3.6 and trunk/2.0.

The snippets generated by this are available on staging-stage:/opt/aus2/snippets/staging, if you'd like to inspect them. I did some basic comparisons (beta channel, comparing Darwin and Windows between the main set of snippets and the earlier build ones) and all snippets were identical.

Ready to go as far as I'm concerned.
Attachment #469984 - Attachment is obsolete: true
Attachment #471146 - Flags: review?(nrthomas)
Attached patch updated buildbotcustom patch (obsolete) — Splinter Review
Mostly some bug fixes in this update. Fully tested now, and ready to go AFAIK.
Attachment #470579 - Attachment is obsolete: true
Attachment #471147 - Flags: review?(nrthomas)
Attachment #470579 - Flags: feedback?(nrthomas)
Priority: P5 → P3
I put the snippets in ~ffxbld too, just in case a cronjob wipes the original ones away.
Priority: P3 → P5
Comment on attachment 471147 [details] [diff] [review]
updated buildbotcustom patch

>diff --git a/process/factory.py b/process/factory.py
>+    def createBuildNSnippets(self):
>+        command = ['python',
>+                   '../../../../tools/release/generate-candidate-build-updates.py',

>+         env={'PYTHONPATH': '../../../../tools/lib/python'},

For both of these WithProperties("%(toolsdir)s/.... would be better I think.

>+        # There may be updates from earlier builds to push

s/push/transfer/. Why no pushsnip for betatest and releasetest like we do for the regular snippets ? Maybe we should be bold and munge the vanilla snippet dirs, then let the existing machinery transfer over to AUS and run pushsnip.
Attachment #471147 - Flags: review?(nrthomas) → review-
Comment on attachment 471146 [details] [diff] [review]
updated tools patch

>diff --git a/lib/python/release/platforms.py b/lib/python/release/platforms.py
>+update_platform_map>+def getSupportedPlatforms():
>+    return ('linux', 'linux64', 'win32', 'win64', 'wince', 'macosx', 'macosx64')

Would be good to add to update_platform_map for win64 now. Armen's nightlies are publishing as WINNT_x86_64-msvc.

>diff --git a/release/generate-candidate-build-updates.py b/release/generate-candidate-build-updates.py
>+def getSnippetDirname(brandName, version, channel):
>+    return "%s-%s-%s-earlier-builds-%s" % (strftime('%Y%m%d'), brandName,
>+                                           version, channel)
>+
>+def getOldSnippetDirname(oldBaseSnippetDir, channel):
>+    if channel == 'release':
>+        ausdir = 'aus2'

I didn't notice that the new directories are named differently from the existing ones. Do you want to move to more explicit names ? I'd be a little worried we'd forget to push releasetest (or betatest) if they are handled differently, or worse forget to do 'release' for a .x release.

>+    oldIDs = findOldBuildIDs(product, version, buildNumber, platforms,
>+                             server=stageServer, verbose=verbose)
>+    for platform in oldIDs.keys():
>+        update_platform = buildbot2updatePlatform(platform)
>+        oldVersionBuildID = getBuildID(platform, product, oldVersion,

The first time I read this I found oldID and oldVersionBuildID confusing. Perhaps you could s/oldIDs/previousCandidateIDs/, or versionPreviousIDs, or something.

>+        oldSl = getShippedLocales(product, oldVersion, oldBuildNumber,
>+                                  sourceRepo, hg, verbose)
>+        oldPlatformLocales = getPlatformLocales(oldSl, (platform,))[platform]
>+        sl = getShippedLocales(product, version, buildNumber,
>+                               sourceRepo, hg, verbose)
>+        platformLocales = getPlatformLocales(sl, (platform,))[platform]

I think you could get oldSl and sl outside the platform loop, and get oldPlatformLocales & platformLocales for each platform. Might save a whole second or two so this is obviously super important.

>+                        for f in snippets:
>+                            if verbose:
>+                                "  %s" % f
>+                            shutil.copy(oldFile, os.path.join(newDir, f))

I don't see anything changing 'type=complete' to 'type=partial' when generatePartials is True. r- is for this
Attachment #471146 - Flags: review?(nrthomas) → review-
(In reply to comment #19)
> Comment on attachment 471147 [details] [diff] [review]
> updated buildbotcustom patch
> 
> >diff --git a/process/factory.py b/process/factory.py
> >+    def createBuildNSnippets(self):
> >+        command = ['python',
> >+                   '../../../../tools/release/generate-candidate-build-updates.py',
> 
> >+         env={'PYTHONPATH': '../../../../tools/lib/python'},
> 
> For both of these WithProperties("%(toolsdir)s/.... would be better I think.
> 
> >+        # There may be updates from earlier builds to push
> 
> s/push/transfer/. Why no pushsnip for betatest and releasetest like we do for
> the regular snippets ? Maybe we should be bold and munge the vanilla snippet
> dirs, then let the existing machinery transfer over to AUS and run pushsnip.

Hrm. That's an interesting idea. We can't safely push betatest and releasetest because the factory doesn't know the names of the snippet directories. I could make it happen with same awful bash, but I don't like going that route. I'll try out munging the existing directories and see how dangerous it seems.


(In reply to comment #20)
> Comment on attachment 471146 [details] [diff] [review]
> updated tools patch
> 
> >diff --git a/lib/python/release/platforms.py b/lib/python/release/platforms.py
> >+update_platform_map>+def getSupportedPlatforms():
> >+    return ('linux', 'linux64', 'win32', 'win64', 'wince', 'macosx', 'macosx64')
> 
> Would be good to add to update_platform_map for win64 now. Armen's nightlies
> are publishing as WINNT_x86_64-msvc.

Sure

> 
> >diff --git a/release/generate-candidate-build-updates.py b/release/generate-candidate-build-updates.py
> >+def getSnippetDirname(brandName, version, channel):
> >+    return "%s-%s-%s-earlier-builds-%s" % (strftime('%Y%m%d'), brandName,
> >+                                           version, channel)
> >+
> >+def getOldSnippetDirname(oldBaseSnippetDir, channel):
> >+    if channel == 'release':
> >+        ausdir = 'aus2'
> 
> I didn't notice that the new directories are named differently from the
> existing ones. Do you want to move to more explicit names ? I'd be a little
> worried we'd forget to push releasetest (or betatest) if they are handled
> differently, or worse forget to do 'release' for a .x release.

This goes away if we're munging the existing dirs, so I'm not going to worry about it unless that doesn't pan out.

> >+    oldIDs = findOldBuildIDs(product, version, buildNumber, platforms,
> >+                             server=stageServer, verbose=verbose)
> >+    for platform in oldIDs.keys():
> >+        update_platform = buildbot2updatePlatform(platform)
> >+        oldVersionBuildID = getBuildID(platform, product, oldVersion,
> 
> The first time I read this I found oldID and oldVersionBuildID confusing.
> Perhaps you could s/oldIDs/previousCandidateIDs/, or versionPreviousIDs, or
> something.

Will do

> 
> >+        oldSl = getShippedLocales(product, oldVersion, oldBuildNumber,
> >+                                  sourceRepo, hg, verbose)
> >+        oldPlatformLocales = getPlatformLocales(oldSl, (platform,))[platform]
> >+        sl = getShippedLocales(product, version, buildNumber,
> >+                               sourceRepo, hg, verbose)
> >+        platformLocales = getPlatformLocales(sl, (platform,))[platform]
> 
> I think you could get oldSl and sl outside the platform loop, and get
> oldPlatformLocales & platformLocales for each platform. Might save a whole
> second or two so this is obviously super important.

Yeah, good point.

> 
> >+                        for f in snippets:
> >+                            if verbose:
> >+                                "  %s" % f
> >+                            shutil.copy(oldFile, os.path.join(newDir, f))
> 
> I don't see anything changing 'type=complete' to 'type=partial' when
> generatePartials is True. r- is for this

Ouch, that's a pretty big mistake. I'll fix it.
Nick, I think I've addressed all of your comments. I re-tested this w/ an updated patch to the factory and everything seemed to work well. I used 3.6.4build7 as the version, to verify the fix for the partial snippets. The only differences between the snippet directories before and after this script ran were the addition of snippets for 3.6.4 build1s through 6.
Attachment #471928 - Flags: review?(nrthomas)
I used WithProperties('%(toolsdir)s') as you suggested and ended up dropping all the additions to uploadSnippets(), so the comment fix wasn't necessary.

I'm not back until Thursday, so no rush on these.
Attachment #471146 - Attachment is obsolete: true
Attachment #471147 - Attachment is obsolete: true
Attachment #471929 - Flags: review?(nrthomas)
Comment on attachment 471928 [details] [diff] [review]
review comments addressed

interdiff is nice!
Attachment #471928 - Flags: review?(nrthomas) → review+
Attachment #471929 - Flags: review?(nrthomas) → review+
Comment on attachment 471929 [details] [diff] [review]
address comments in factory

changeset:   1000:65be24bdd096
Attachment #471929 - Flags: checked-in+
Comment on attachment 471928 [details] [diff] [review]
review comments addressed

changeset:   860:911c93dcc345
Attachment #471928 - Flags: checked-in+
Fully landed, documentation updated: https://wiki.mozilla.org/index.php?title=Release%3ARelease_Automation_on_Mercurial%3ADocumentation&action=historysubmit&diff=252317&oldid=252312
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 604337
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.