releasenotes url should be overridable

RESOLVED FIXED

Status

Release Engineering
General
P3
normal
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: bhearsum, Assigned: rail)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 7 obsolete attachments)

936 bytes, patch
bhearsum
: review+
Details | Diff | Splinter Review
3.96 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
7.31 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
12.24 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
From now on we're going to be using http://www.mozilla.org/projects/devpreview/releasenotes/ for the devpreview releases, we need to update the patcher configs to reflect this.
Assignee: nobody → rail
Created attachment 433318 [details] [diff] [review]
Proposed fix
Attachment #433318 - Flags: review?(bhearsum)
Also need to update
 http://hg.mozilla.org/build/tools/file/default/release/patcher-config-bump.pl
or it'll be reset at the next release.
Created attachment 433516 [details] [diff] [review]
patcher-config-bump.pl.diff
Attachment #433516 - Flags: review?(bhearsum)
Created attachment 433517 [details] [diff] [review]
patcher-config-bump.pl.diff

typo
Attachment #433516 - Attachment is obsolete: true
Attachment #433517 - Flags: review?(bhearsum)
Attachment #433516 - Flags: review?(bhearsum)
(Reporter)

Comment 5

8 years ago
Comment on attachment 433318 [details] [diff] [review]
Proposed fix

this is fine
Attachment #433318 - Flags: review?(bhearsum) → review+
(Reporter)

Comment 6

8 years ago
Comment on attachment 433517 [details] [diff] [review]
patcher-config-bump.pl.diff

Rail, this patch isn't going to work, unfortunately. We need to be able to support alphas which are "devpreview" style and ones that aren't.

Nick threw out the idea of getting a redirect put in from .com -> .org rather than us hacking around it. I'll look into doing that tomorrow.
Attachment #433517 - Flags: review?(bhearsum) → review-
(Reporter)

Comment 7

8 years ago
So, I didn't talk to drivers but given that we're going to be doing at least one 3.7 alpha which is not a devpreview, and we don't know which one it will be, I don't think a redirect is going to fly.

Unless someone has a better idea, I think we need to move the url into the release config. Incidentally, I think we might need something special for the lorentz beta(s) too, which may be another use case for it. Nick or Catlee, any better ideas?
Assignee: rail → marcia
Component: Release Engineering → Account Request: Hg
QA Contact: release → hg-acct-req
(Reporter)

Comment 8

8 years ago
sorry, bugzilla fail -- didn't mean to move this.
Assignee: marcia → nobody
Component: Account Request: Hg → Release Engineering
QA Contact: hg-acct-req → release

Comment 9

8 years ago
Assigning to Rail based on WIP.
Assignee: nobody → rail
Priority: -- → P3
Created attachment 437253 [details] [diff] [review]
patcher-config-bump.pl changes
Attachment #433517 - Attachment is obsolete: true
Attachment #437253 - Flags: review?(bhearsum)
Created attachment 437254 [details] [diff] [review]
buildbotcustom changes
Attachment #437254 - Flags: review?(bhearsum)
Created attachment 437255 [details] [diff] [review]
buildbot-configs changes
Attachment #437255 - Flags: review?(bhearsum)
(Reporter)

Comment 13

8 years ago
Comment on attachment 437253 [details] [diff] [review]
patcher-config-bump.pl changes

This is mostly OK, but there's some more work to do:
* I like that the existing defaults have been kept because it lets us not explicitly specify the URL in many cases
* I think the %product% and %appVersion% interpolation is overkill. If it's being overridden, let's just be explicit about it. Please drop this logic.
* Release::Patcher::Config needs to be updated too (http://hg.mozilla.org/build/tools/file/22d2297dab86/lib/perl/Release/Patcher/Config.pm#l11). This library is already used by patcher-config-creator.pl, which is used in the major update generation. It's probably worthwhile to get patcher-config-bump.pl using that library, too. You may need to get ReleaseUpdatesFactory to set PERL5LIB to make that work.
Attachment #437253 - Flags: review?(bhearsum) → review-
(Reporter)

Updated

8 years ago
Attachment #437254 - Flags: review?(bhearsum) → review-
(Reporter)

Comment 14

8 years ago
Comment on attachment 437254 [details] [diff] [review]
buildbotcustom changes

The bits you have here are fine, but MajorUpdateFactory needs updating as well. Note that in the configs we'll need to have majorUpdateReleasenotesURL for branches in which majorUpdateRepo != None.
(Reporter)

Comment 15

8 years ago
Comment on attachment 437255 [details] [diff] [review]
buildbot-configs changes

It sucks that we need to use releasenotesURL = None to say "use the default", but it's o.k. overall.

Nits:
* releaseNotesUrl insetad of releasenotesURL
* Move it up above useBetaChannel
* For branches which have a major update (eg, 1.9.1), majorUpdateReleaseNotesUrl needs to be added, and passed to MajorUpdateFactory appropriately.
Attachment #437255 - Flags: review?(bhearsum) → review-
Just to don't forget, (In reply to comment #15)
> * For branches which have a major update (eg, 1.9.1),
> majorUpdateReleaseNotesUrl needs to be added, and passed to MajorUpdateFactory
> appropriately.

Good idea. In this case bug 556941 will be fixed as well.
Status: NEW → ASSIGNED
Some comments for the patches to be attached

(In reply to comment #13)
> * I think the %product% and %appVersion% interpolation is overkill. If it's
> being overridden, let's just be explicit about it. Please drop this logic.

Done.

> * Release::Patcher::Config needs to be updated too
> (http://hg.mozilla.org/build/tools/file/22d2297dab86/lib/perl/Release/Patcher/Config.pm#l11).
> This library is already used by patcher-config-creator.pl, which is used in the
> major update generation. It's probably worthwhile to get patcher-config-bump.pl
> using that library, too. You may need to get ReleaseUpdatesFactory to set
> PERL5LIB to make that work.

patcher-config-bump.pl now uses the same subroutine:

   $currentUpdateObj->{'details'} = $releaseNotesUrl ||
       GetProductDetails(product => $product, appVersion => $appVersion,
                         updateType => 'minor');

PERL5LIB env added as well.

(In reply to comment #14)
> The bits you have here are fine, but MajorUpdateFactory needs updating as well.
> Note that in the configs we'll need to have majorUpdateReleasenotesURL for
> branches in which majorUpdateRepo != None.

Done.

(In reply to comment #15)
> * releaseNotesUrl insetad of releasenotesURL
> * Move it up above useBetaChannel
> * For branches which have a major update (eg, 1.9.1),
> majorUpdateReleaseNotesUrl needs to be added, and passed to MajorUpdateFactory
> appropriately.

Done.

I tested the patches in staging using 1.9.1 (to have MU)  with the following results.

1. patcher-config-bump.pl changes

* update builder, bump step ran as

    perl ../tools/release/patcher-config-bump.pl -p firefox -r Firefox -v 3.5.9 -a 3.5.9 -o 3.5.8 -b 1 -c patcher-configs/moz191-branch-patcher2.cfg -t staging-stage.build.mozilla.org -f ftp.mozilla.org -d download.mozilla.org -l shipped-locales -u

* diff_patcher_config  output contains +- for betatest-url and buildids, details URL is still the same:
    
  details   http://www.mozilla.com/%locale%/firefox/3.5.9/releasenotes/

2. patcher-config-creator.pl changes

create_config step ran as
 perl ../tools/release/patcher-config-creator.pl -p firefox -r Firefox -v 3.6rc2 -a 3.6 -o 3.5.9 --old-app-version=3.5.9 -b 1 --old-build-number=1 -c patcher-configs/moz191-branch-major-update-patcher2.cfg -t staging-stage.build.mozilla.org -f ftp.mozilla.org -d download.mozilla.org -l shipped-locales --old-shipped-locales=old-shipped-locales --update-type=major -u -n http://www.mozilla.com/%locale%/firefox/3.6/details/index.html

diff_patcher_config (for moz191-branch-major-update-patcher2.cfg) output contains some staging related changes, buildid changes and finally

-  details   http://www.mozilla.com/%locale%/firefox/3.6.3/details/index.html
+ details   http://www.mozilla.com/%locale%/firefox/3.6/details/index.html

which is a good sign for bug 556941. :)

Maybe it is an overkill to test such changes running a full release process, but it was very educational for me. :)
Created attachment 437797 [details] [diff] [review]
tools changes
Attachment #437253 - Attachment is obsolete: true
Attachment #437797 - Flags: review?(bhearsum)
Created attachment 437798 [details] [diff] [review]
buildbotcustom changes
Attachment #437254 - Attachment is obsolete: true
Attachment #437798 - Flags: review?(bhearsum)
Created attachment 437799 [details] [diff] [review]
buildbot-configs changes
Attachment #437255 - Attachment is obsolete: true
Attachment #437799 - Flags: review?(bhearsum)
(Reporter)

Updated

8 years ago
Attachment #437797 - Flags: review?(bhearsum) → review+
(Reporter)

Comment 21

8 years ago
Comment on attachment 437797 [details] [diff] [review]
tools changes

This looks fine, other than my incredibly tiny nit:
releasenotes-url should be before help/run-tests in the options. r+ w/ that changed.
Attachment #437799 - Flags: review?(bhearsum) → review-
(Reporter)

Comment 22

8 years ago
Comment on attachment 437799 [details] [diff] [review]
buildbot-configs changes

You missed updating MajorUpdateFactory on the production side -- looks fine otherwise.
(Reporter)

Updated

8 years ago
Attachment #437798 - Flags: review?(bhearsum) → review+
Created attachment 438049 [details] [diff] [review]
tools changes

(In reply to comment #21)
> releasenotes-url should be before help/run-tests in the options. r+ w/ that
> changed.

Done.
Attachment #437797 - Attachment is obsolete: true
Attachment #438049 - Flags: review?(bhearsum)
Created attachment 438051 [details] [diff] [review]
buildbot-configs changes

(In reply to comment #22)
> You missed updating MajorUpdateFactory on the production side -- looks fine
> otherwise.

My fault. Fixed.
Attachment #437799 - Attachment is obsolete: true
Attachment #438051 - Flags: review?(bhearsum)
I tested major_update builder staging and it ran without any failure.
major_update_verify builders failed because they were unable to find updates for "mn" (new in 3.6.x afaik):

Using  http://staging-stage.build.mozilla.org/update/1/Firefox/3.5.9/20100407041608/Linux_x86-gcc3/mn/betatest/update.xml?force=1
FAIL: no partial update found for http://staging-stage.build.mozilla.org/update/1/Firefox/3.5.9/20100407041608/Linux_x86-gcc3/mn/betatest/update.xml?force=1
FAIL: download_mars returned non-zero exit code: 1
Using  http://staging-stage.build.mozilla.org/update/1/Firefox/3.5.9/20100407041608/Linux_x86-gcc3/mn/betatest/update.xml?force=1
FAIL: no complete update found for http://staging-stage.build.mozilla.org/update/1/Firefox/3.5.9/20100407041608/Linux_x86-gcc3/mn/betatest/update.xml?force=1
FAIL: download_mars returned non-zero exit code: 1
(Reporter)

Comment 26

8 years ago
(In reply to comment #25)
> I tested major_update builder staging and it ran without any failure.
> major_update_verify builders failed because they were unable to find updates
> for "mn" (new in 3.6.x afaik):

Almost right. "mn" was dropped in 3.6. Because we use the "from" release's list of locales to test, we fail on any locales which are dropped.

I had a look over the snippets and they look fine. This is ready to land, as far as I'm concerned.
(Reporter)

Updated

8 years ago
Duplicate of this bug: 556941
(Reporter)

Updated

8 years ago
Summary: 1.9.3 patcher config detailsURL needs updating → releasenotes url should be overridable
(Reporter)

Comment 28

8 years ago
Comment on attachment 437798 [details] [diff] [review]
buildbotcustom changes

changeset:   697:670969d5525c
Attachment #437798 - Flags: checked-in+
(Reporter)

Comment 29

8 years ago
Comment on attachment 438049 [details] [diff] [review]
tools changes

changeset:   575:330b61674a6d
Attachment #438049 - Flags: review?(bhearsum)
Attachment #438049 - Flags: review+
Attachment #438049 - Flags: checked-in+
(Reporter)

Updated

8 years ago
Attachment #438051 - Flags: review?(bhearsum)
Attachment #438051 - Flags: review+
Attachment #438051 - Flags: checked-in+
(Reporter)

Updated

8 years ago
Blocks: 557819
(Reporter)

Comment 30

8 years ago
This tested out fine in my staging run.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.