682805 - Generate snippets using new attributes

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

Bug 459972 is about adding support for several new attributes in the update.xml, the minimum set being comment #45 over there. The client apps already support those, this bug is about adding them to snippets, and we'll leave bug 459972 to be AUS turning them into an update.xml.

I've got a proof of concept up at
  http://hg.mozilla.org/users/nthomas_mozilla.com/patcher-investigation-bug459972/pushloghtml

Comment 1

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

Attachment: [cvs] patcher code changes

The change to write out a new style of snippets (with 'version=2') is in write_patch_info() at the end. Everything else leads up to that, using loops because patcher has far too much copy and paste already.

Comment 2

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

Attachment: Snippets

This what you get from http://hg.mozilla.org/users/nthomas_mozilla.com/patcher-investigation-bug459972/rev/88cf832361dd

They're not sensible snippets, as they include lots of new properties that are mutually exclusive.

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 556495 [details] [diff] [review]
[cvs] patcher code changes

It looks fine, especially foreach loops eliminating boilerplate code.

Comment 4

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

FTR, I'm not planning to modify the patcher config bumper beyond writing 'schema   2' (instead of 1) in new <release> blocks. We'll create static entries for all the other variables, and adjust manually as required. This assumes that prior releases should all be treated equally.

The bumper will still get a new argument to control the schema written (since 1.9.2 still isn't dead yet), the release config variable to control that, and some more logic added to the updates factory. Mighty tempting just to hardcode in the bump script now that I write all that out. :-S

I want to take another look at this code before I put it up for review, since I've lost context in the meantime.

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

Nick, I'm specifically interested in the ability to show or hide the what's new page when performing an update. This is part of the silent update work. (See [1].) When do you expect that you can complete this work providing the show/hide the what's new page capability?

[1]https://wiki.mozilla.org/Silent_Update_whatsnew

Comment 6

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

Lawrence, the short answer is that we should have that soon, within the next week or so. We will set 'actions="silent"' in the update.xml to suppress loading the What's New page.

The changes to the update server (bug 459972) are being tested in staging, prior to going live in production. I need to finish up the code changes here, get review and land them, so that we can generate snippets which set an action (amongst other things). Hopefully we can use that for 8.0b3.  Note that we would set the same action on *all* the releases in a given sequence. So for the release channel sequence that starts at 4.0, going through 5.0*, 6.0*, 7.0*, all those releases could be configured to show what's new (or not).

Comment 7

[LegNeato]

Yep, perfect. The plan of record for Fx8 is in bug 689679 comment 5. Please let me know if you think these changes won't be implemented by then and we can take other mitigation steps.

Comment 8

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

Attachment: [cvs] patcher code changes

This is pretty much the same as the earlier patch, only with more accurate name than LOCALIZABLE_URLS and a comments sprinkled on. Oh, and a patch actually against CVS tip. This would become UPDATE_PACKAGING_R15.

Comment 9

[Rail Aliiev [:rail]]

Comment on attachment 567400 [details] [diff] [review]
[cvs] patcher code changes

Stamp! Looks good.

Comment 10

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

Attachment: [tools] Config bumper/creater should have configurable schema

Instead of hardcoding 'schema   1' in release blocks this makes it configurable, and defaults to the new way ('schema   2'). 

The tests in these files are broken beyond redemption, so I haven't bothered to fix and update them. But I did confirm that redoing the bumps/creations for
 * 3.6.23
 * 3.6.23 -> 7.0.1 MU
 * 8.0b2
worked properly, provided I added '-s 1' to the first two, and expected the <8.0b2> block to change scheme for the last.

Comment 11

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

Attachment: [buildbotcustom] Pass schema arg from release configs around

This lets us set a snippetSchema variable in release configs to override the new default of schema=2 in the config bumper/creator. Eg Firefox 3.6 (plain and major update), Thunderbird & SeaMonkey until they have support for the new attributes.

Comment 12

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

Attachment: [buildbot-configs] release config changes

Release config chagnes to force schema=1 for our 3.6 updates (since they don't support the new attributes), and swap to the (still to be created) UPDATE_PACKAGING_R15 tag for patcher on beta & release.

Testing notes:
* For 3.6 (plain and major updates) I have verified that the combination of the buildbot and tools changes reproduce the patcher config we used for 3.6.23 and a MU from 3.6.23 -> 7.0.1. Since we'll keep using the old UPDATE_PACKAGING_R11_1{,_MU} tags for patcher I didn't go any further

* For 8.0b4 I redid the whole updates step with all the changes on this bug, and then compared the snippets & partials against the actual ones we had for realz. The snippets had changes like this:

--- old/Firefox-8.0b4-build1/Firefox/4.0b10/Darwin_x86-gcc3-u-i386-x86_64/20110121161344/af/beta/complete.txt	2011-10-20 13:53:12.000000000 +1300
+++ new/aus2/Firefox/4.0b10/Darwin_x86-gcc3-u-i386-x86_64/20110121161344/af/beta/complete.txt	2011-10-25 22:07:33.000000000 +1300
@@ -1,10 +1,12 @@
-version=1
+version=2
 type=complete
 url=http://download.mozilla.org/?product=firefox-8.0b4-complete&os=osx&lang=af
 hashFunction=SHA512
 hashValue=422ed8ab87b11e04146cb1e2ab7c55ab868901cc6382582ebfceea1e245ceceef0ec0e7ed585abc93c8bb0774a662cc55e3be554681583ae24909dfc3c032a60
 size=30308910
 build=20111019081014
-appv=8.0 Beta
-extv=8.0
+displayVersion=8.0 Beta
+appVersion=8.0
+platformVersion=8.0
 detailsUrl=https://www.mozilla.com/af/firefox/8.0/releasenotes/
+actions=silent

which is all expected. The partials were the same.

Comment 13

[Rail Aliiev [:rail]]

Comment on attachment 569316 [details] [diff] [review]
[buildbotcustom] Pass schema arg from release configs around

Review of attachment 569316 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/release.py
@@ +1127,1 @@
>          )

Can you add a trailing coma here, so similar future diffs would look as one liner?

@@ +1331,1 @@
>          )

The same here.

Comment 14

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

Comment on attachment 569320 [details] [diff] [review]
[buildbot-configs] release config changes

Landed on default:
http://hg.mozilla.org/build/buildbot-configs/rev/d7dd54c8d7f3

Comment 15

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

Comment on attachment 569316 [details] [diff] [review]
[buildbotcustom] Pass schema arg from release configs around

Landed on default (with trailing commas):
http://hg.mozilla.org/build/buildbotcustom/rev/bbfcb08d514f

Comment 16

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

Comment on attachment 567662 [details] [diff] [review]
[tools] Config bumper/creater should have configurable schema

http://hg.mozilla.org/build/tools/rev/340672de3ae2

Comment 17

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

Comment on attachment 567400 [details] [diff] [review]
[cvs] patcher code changes

Landed on HEAD:
$ cvs ci -m "Bug 682805, Generate snippets using new attributes, r=rail"
Checking in MozAUSConfig.pm;
/cvsroot/mozilla/tools/patcher/MozAUSConfig.pm,v  <--  MozAUSConfig.pm
new revision: 1.20; previous revision: 1.19
done
Checking in patcher2.pl;
/cvsroot/mozilla/tools/patcher/patcher2.pl,v  <--  patcher2.pl
new revision: 1.44; previous revision: 1.43
done

Create new tag:
$ ~/mozilla/tools/patcher $ cvs tag UPDATE_PACKAGING_R15
T MozAUSConfig.pm
T MozAUSLib.pm
T README
T patcher2.cfg
T patcher2.pl

Create new tag on support libraries:
$ cd ~/mozilla/tools/release
$ cvs up -AC
$ cvs diff -r UPDATE_PACKAGING_R14    # HEAD == UPDATE_PACKAGING_R14 ??
# some changes to bin/backupsnip-nightly and bin/check-sync which aren't used for
# generating updates
$ cvs tag UPDATE_PACKAGING_R15

Create new tag in main code repos:
$ cd ~/mercurial/releases/mozilla-beta
$ hg up -C
$ hg pull -u
$ hg diff -r UPDATE_PACKAGING_R14: other-licenses/bsdiff/ modules/lib{bz2,mar} tools/update-packaging/
# no diff found
$ hg tag -m "Bug 682805, add tag UPDATE_PACKAGING_R15 (on R14), a=legneato DONTBUILD" -r UPDATE_PACKAGING_R14 UPDATE_PACKAGING_R15
$ hg out
$ hg push

Repeated for mozilla-central & mozilla-aurora so that it already exists when we do code migrations (m-beta takes care of m-release).

Comment 18

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

Attachment: [cvs] Suppress What's New page on mozilla-beta and mozilla-release updates

This implements bug 689679 comment #5 (plus later discussion confirming it). With r+ it should be landed prior to 8.0b5 updates running, in order to save your self yourself some work redoing.

Comment 19

[u279076]

qa- as I'm assuming there is nothing for QA to do to verify this fix. Please set to qa+ if there is something for QA to verify.

Comment 20

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

Comment on attachment 569544 [details] [diff] [review]
[cvs] Suppress What's New page on mozilla-beta and mozilla-release updates

Checking in mozBeta-branch-patcher2.cfg;
/cvsroot/mozilla/tools/patcher-configs/mozBeta-branch-patcher2.cfg,v  <--  mozBeta-branch-patcher2.cfg
new revision: 1.33; previous revision: 1.32
done
Checking in mozRelease-branch-patcher2.cfg;
/cvsroot/mozilla/tools/patcher-configs/mozRelease-branch-patcher2.cfg,v  <--  mozRelease-branch-patcher2.cfg
new revision: 1.16; previous revision: 1.15
done

Comment 21

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

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #19)
> qa- as I'm assuming there is nothing for QA to do to verify this fix. Please
> set to qa+ if there is something for QA to verify.

FTR, bug 697030 is where we'll test this.

I'm going to tempt fate and say we're done here. Followup bugs would probably be best for any regressions.