468197 - Simplify / automate the addition of the pre-release suffix string (e.g. Alpha 1, Beta 2, etc.)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Robert Strong (they/them - no direct email)]

Talked this over briefly with beltzner and joduinn... I've got most of this done already

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch in progress rev1

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1 (doesn't include the automation bits for releases)

It might be nice to get this in for Beta 3 since we have some time until the release... it makes it so we can set the pre-release suffix in one file and have it set the value in the places we need it for pre-releases. I'll add release automation so this value can be easily set by the release team at a later time.

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 362093 [details] [diff] [review]
patch rev1 (doesn't include the automation bits for releases)

bah... copy paste error... new patch coming up

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: patch rev2

This has been tested building both the en-US and de Win32 installer and I also verified that browser.xul gets the proper values... all were tested for the empty file and the beta string cases.

Comment 5

[Robert Strong (they/them - no direct email)]

Forgot to mention that both NO_INSTDIR_FROM_REG cases were tested with both en-US and de Win32 installers as well.

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: string removal patch for trunk (checked in)

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 362098 [details] [diff] [review]
patch rev2

I think I have a simpler way to automate this... patch coming up

Comment 8

[Robert Strong (they/them - no direct email)]

Attachment: patch rev3 (checked in)

This automates the addition of Alpha X and Beta X naming for the installer and browser.xul as well as setting the NO_INSTDIR_FROM_REG define as appropriate.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 362183 [details] [diff] [review]
patch rev3 (checked in)

Cool beans, looks great.

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 362183 [details] [diff] [review]
patch rev3 (checked in)

Drivers, I'd like to use this method for changing the Beta 3 string for Beta 3 but with the way the mozilla-central tinderbox has been the last week I have no idea when the tree will be in decent shape to check in there... so, I am asking for approval for this prior to landing it on mozilla-central.

Comment 11

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/8a3394e165a9

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Has this been tested? I'd like to do that before we land it on branch.

Comment 13

[Robert Strong (they/them - no direct email)]

I've tested this locally but I haven't figured out a way to automate the testing for this change. Then again, we don't have automated testing of the manual change either.

Comment 14

[Robert Strong (they/them - no direct email)]

Attachment: alternative approach (in progress)

Seems a bit overkill but this could be converted into python and ut could then be tested in the same way that a few of the python scripts are tested under config... thoughts?

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Sorry, I didn't mean automated tests. I mean, has someone built with the new mechanism to ensure that it, you know, works.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 362183 [details] [diff] [review]
patch rev3 (checked in)

a191=beltzner

Comment 17

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e5099792b0b9

Comment 19

[[e-mail] (:bhearsum)]

Wish I'd looked at this bug sooner. Rob, we have code to do this in http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/package-name.mk - MOZ_PKG_LONGVERSION will be, eg, '3.1 Beta 3' when MOZ_PKG_PRETTYNAMES is passed. We already pass this for packaging and could easily pass it to the build step too.

It doesn't matter much to me how we do it, but just wanted to mention it.

Comment 20

[Robert Strong (they/them - no direct email)]

Attachment: followup patch (checked in)

I thought the output should have been Beta 3 instead of 3.1 Beta 3 likely due to being ill this last week :( this patch fixes that.

Comment 21

[Robert Strong (they/them - no direct email)]

Pushed the followup to mozilla-1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/230e03ff70fc

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Carrying over blocking from the source dupe bug 475100.

So, what's left to do here on the build side after we tag? Just patch a single file? If so, we should get a P1 blocker on that, too, to make sure we don't forget.

Comment 23

[[e-mail] (:bhearsum)]

As I understand it there's nothing we have to do. Automation will bump version.txt to '3.1b3' and Rob's patch will do the rest.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #19)
> MOZ_PKG_LONGVERSION will be, eg, '3.1 Beta 3' when MOZ_PKG_PRETTYNAMES is
> passed. We already pass this for packaging and could easily pass it to the
> build step too.

Sounds like we should get a followup filed to investigate doing it this way, though.

Comment 25

[Robert Strong (they/them - no direct email)]

(In reply to comment #24)
> (In reply to comment #19)
> > MOZ_PKG_LONGVERSION will be, eg, '3.1 Beta 3' when MOZ_PKG_PRETTYNAMES is
> > passed. We already pass this for packaging and could easily pass it to the
> > build step too.
> 
> Sounds like we should get a followup filed to investigate doing it this way,
> though.
We probably could use the same sed statements but we would have to modify the results since packaging returns using "3.1 Beta 3" and we need " 3.1 Beta 3". The name returned for packaging can also be "3.1 RC 1" which we never want. This is why I didn't go with what packaging uses.

Comment 26

[[e-mail] (:bhearsum)]

(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #19)
> > > MOZ_PKG_LONGVERSION will be, eg, '3.1 Beta 3' when MOZ_PKG_PRETTYNAMES is
> > > passed. We already pass this for packaging and could easily pass it to the
> > > build step too.
> > 
> > Sounds like we should get a followup filed to investigate doing it this way,
> > though.
> We probably could use the same sed statements but we would have to modify the
> results since packaging returns using "3.1 Beta 3" and we need " 3.1 Beta 3".
> The name returned for packaging can also be "3.1 RC 1" which we never want.
> This is why I didn't go with what packaging uses.

wfm. it's already checked in and working, anyways, no need to rock the boat.

Comment 27

[Robert Strong (they/them - no direct email)]

(In reply to comment #22)
> Carrying over blocking from the source dupe bug 475100.
> 
> So, what's left to do here on the build side after we tag? Just patch a single
> file? If so, we should get a P1 blocker on that, too, to make sure we don't
> forget.
Just to confirm what bhearsum wrote, nothing needs to be done except the version change in version.txt which is done by build automation.

Comment 28

[Robert Strong (they/them - no direct email)]

Comment on attachment 362110 [details] [diff] [review]
string removal patch for trunk (checked in)

verbal r+ from mconnor

Comment 29

[Robert Strong (they/them - no direct email)]

Comment on attachment 362110 [details] [diff] [review]
string removal patch for trunk (checked in)

Pushed this last remaining patch to remove the unused string to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/47947c9ae4fc

Comment 30

[Philippe M. Chiasson (:gozer)]

I am trying to port this over to comm-central in bug 481374, and I just noticed something odd with sed on OS X which caused me some grief with the sed pattern:

$> echo 12345 | sed -e's/[0-9]\+/x/'
12345
$> echo 12345 | gsed -e's/[0-9]\+/x/'
x

Has this patch been verified working on OS X as it is ?

Comment 31

[Robert Strong (they/them - no direct email)]

Phil, take a look at bug 481703 for what is actually being used to generate the pre-release suffix