Closed Bug 1567326 Opened 5 years ago Closed 4 years ago

Thunderbird merge script updates for ESR

Categories

(Thunderbird :: Build Config, task)

task
Not set
normal

Tracking

(thunderbird_esr78 unaffected)

RESOLVED FIXED
Thunderbird 79.0
Tracking Status
thunderbird_esr78 --- unaffected

People

(Reporter: rjl, Assigned: rjl)

Details

Attachments

(2 files, 1 obsolete file)

A couple of changes to make to the Thunderbird merge scripts sometime before ESR76 based on feedback from the creation of comm-esr68.

  1. merge-release-2-esr.sh - Update .gecko_rev.yml to also drop any existing GECKO_HEAD_REF/GECKO_HEAD_REV fields and set GECKO_HEAD_REF: default
    ESR68 pointed to an old beta changeset.

  2. merge-beta-2-release.sh - Similar to where merge-central-2-beta.sh updates version_display.txt (substr a1 to b1), the bX needs to be dropped from version_display.txt when going to release. Alternatively this could go in merge-beta-2-esr.sh if dropping the beta designation here causes problems for Seamonkey.

  3. Check the commit messages for updating .gecko_rev.yml in all of the scripts. Looks like some refer to "client.py" and does not reflect the reality of what is being changed in the era of Taskcluster.

(In reply to Rob Lemley [:rjl] from comment #0)

A couple of changes to make to the Thunderbird merge scripts sometime before ESR76 based on feedback from the creation of comm-esr68.

  1. merge-release-2-esr.sh - Update .gecko_rev.yml to also drop any existing GECKO_HEAD_REF/GECKO_HEAD_REV fields and set GECKO_HEAD_REF: default
    ESR68 pointed to an old beta changeset.

See https://hg.mozilla.org/releases/comm-esr68/rev/7e3d0416eab369a93faeab66b0cc483c17f02ce2

  1. merge-beta-2-release.sh - Similar to where merge-central-2-beta.sh updates version_display.txt (substr a1 to b1), the bX needs to be dropped from version_display.txt when going to release.

I had always thought this should be here, but someone asked me to remove it at some point... Adding this back is easy enough!

Alternatively this could go in merge-beta-2-esr.sh if dropping the beta designation here causes problems for Seamonkey.

merge-beta-2-esr.sh is not a thing. :) But this will be fine for Seamonkey (or I'll check with those people first before doing it).

  1. Check the commit messages for updating .gecko_rev.yml in all of the scripts. Looks like some refer to "client.py" and does not reflect the reality of what is being changed in the era of Taskcluster.

Yep! This definitely needs to get done. I keep forgetting to update them.

Assignee: nobody → rob
Attached patch gecko_rev.patchSplinter Review
This will take care of #3.

#1 can wait, and #2 maybe we don't change?

(In reply to Rob Lemley [:rjl] from comment #2)

This will take care of #3.

#1 can wait, and #2 maybe we don't change?

Oops! I just saw this. Looks like I made the exact same change though!

This is for item #1. With any luck these scripts won't be used by time
we are ready for it, but this way it won't be a forgotten fixme.
Attachment #9128705 - Flags: review?(clokep)
Comment on attachment 9128705 [details] [diff] [review]
[merge day] set GECKO_HEAD_REF to default for esr

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

Looks OK. I didn't test it though.

::: comm-merges/merge-release-2-esr.sh
@@ +22,5 @@
> +sed "/^GECKO_HEAD_REV/d" esr/.gecko_rev.yml > tmpfile
> +mv tmpfile esr/.gecko_rev.yml
> +sed "/^GECKO_HEAD_REF/d" esr/.gecko_rev.yml > tmpfile
> +mv tmpfile esr/.gecko_rev.yml
> +sed -e '/mozilla-esr[0-9][0-9]*/a\

Not to be pedantic, but why's this one have single quotes and the others are double quotes? Can we be consistent?

Waiting for a response from Rob, just setting needinfo so he knows. :)

Flags: needinfo?(rob)
Flags: needinfo?(rob)
Attachment #9128705 - Flags: review?(clokep)
Attached patch rel2esr78.patchSplinter Review

(In reply to Patrick Cloke [:clokep] from comment #6)

Comment on attachment 9128705 [details] [diff] [review]

Not to be pedantic, but why's this one have single quotes and the others are
double quotes? Can we be consistent?

My editor says it's an error if we do that in double quotes, but I tried it and it seems to work.

Attachment #9128705 - Attachment is obsolete: true
Attachment #9159029 - Flags: review?(clokep)

Maybe this is the wrong spot to comment, but I'd like to come back the the discussion of 9th July 2019, quoting from tb-drivers:

On 07/07/2019 22:17, Patrick Cloke wrote:

In the past we’ve always merged beta to release, then release to esr. This ensures all the proper changesets are there.

I don't see why we wouldn’t do that here.

Coming back to this.

You pushed about 2500 changesets onto that repository of which only the top 23 were relevant:
https://hg.mozilla.org/releases/comm-esr68/pushloghtml

At the very bottom of the push we have COMM_20081201_RELBRANCH from 2008.

I don't understand why those ESR repositories need to be a graveyard of old junk with 435 branches none of which are relevant in any way.

My suggestion was that I would move the 23 changeset that were effectively new on comm-beta since comm-esr68 was cloned from comm-beta. It's too late now, my suggestion was just overruled without discussion, but perhaps we can do it differently next time for comm-esr76.


So I understand comm-esr78 will be or has already been cloned from comm-beta. On merge day, it should receive the changeset pushed to comm-beta after the clone and nothing else IMHO.

I concur.

It should be enough to specify the revision to push to comm-esr78 after running merge-release-2-esr.sh.
Instead of:
hg -R esr push -f
run:
hg -R esr push -f -r .

Comment on attachment 9159029 [details] [diff] [review]
rel2esr78.patch

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

::: comm-merges/merge-release-2-esr.sh
@@ +25,5 @@
> +mv tmpfile esr/.gecko_rev.yml
> +sed -e "/mozilla-$ESR/a\
> +GECKO_HEAD_REF: default
> +" esr/.gecko_rev.yml > tmpfile
> +mv tmpfile esr/.gecko_rev.yml

I believe this is:
* Deleting the line that starts with `GECKO_HEAD_REV`
* Deleting the line that starts with `GECKO_HEAD_REF`
* Adding a line: `GECKO_HEAD_REF: default`

Can we just modify the `GECKO_HEAD_REF` line instead of deleting it and re-adding it?

I believe `sed "s/^GECKO_HEAD_REF: .*/GECKO_HEAD_REF: default/"` will do it and is a bit clearer IMO!
Attachment #9159029 - Flags: review?(clokep) → review+

(In reply to Jorg K (CEST = GMT+2) from comment #9)

So I understand comm-esr78 will be or has already been cloned from comm-beta. On merge day, it should receive the changeset pushed to comm-beta after the clone and nothing else IMHO.

I believe it is useful to have the full history. This is how all the Mozilla repositories are setup, as you go from central -> beta -> release -> esr you get additional changesets specific to those releases, but you never remove any of the history. Note that this is also true of the mozilla-esr78 repo -- that has every tag/branch that has ever existed since the Firefox code has been in Mercurial. At least that's how https://hg.mozilla.org/releases/mozilla-esr78/branches looks to me, but I might be mistaken!

What benefit is there in NOT pushing these changes?

I can do a separate push first that brings it up-to-date with comm-release before the merge if that's useful so that the final push only has the new 77 -> 78 changes?

I'm not sure what the issue with pushing all the changes brings though. Can you describe further why you think it is bad?

(In reply to Patrick Cloke [:clokep] from comment #11)

Comment on attachment 9159029 [details] [diff] [review]
rel2esr78.patch

Review of attachment 9159029 [details] [diff] [review]:

::: comm-merges/merge-release-2-esr.sh
@@ +25,5 @@

+mv tmpfile esr/.gecko_rev.yml
+sed -e "/mozilla-$ESR/a
+GECKO_HEAD_REF: default
+" esr/.gecko_rev.yml > tmpfile
+mv tmpfile esr/.gecko_rev.yml

I believe this is:

  • Deleting the line that starts with GECKO_HEAD_REV
  • Deleting the line that starts with GECKO_HEAD_REF
  • Adding a line: GECKO_HEAD_REF: default

Can we just modify the GECKO_HEAD_REF line instead of deleting it and
re-adding it?

I believe sed "s/^GECKO_HEAD_REF: .*/GECKO_HEAD_REF: default/" will do it
and is a bit clearer IMO!

Yeah that's sort of over-engineered. It's possible that no GECKO_HEAD_REF line is present to change in which case the substitution won't happen and you end up with an invalid file. Of course, it shouldn't happen because beta should have something like FIREFOX_78_0b5_RELEASE.

Considering that the resulting file is still going to be wrong, I'm okay with dropping this patch. Someone has to find the right M-esr78 rev and update HEAD_REF and add HEAD_REV again anyway.

https://hg.mozilla.org/users/bugzilla_standard8.plus.com/drivertools/rev/36cc980832b4263f9d1637e27d11d40d08c2cb0e

(In reply to Rob Lemley [:rjl] from comment #0)

  1. merge-beta-2-release.sh - Similar to where merge-central-2-beta.sh updates version_display.txt (substr a1 to b1), the bX needs to be dropped from version_display.txt when going to release. Alternatively this could go in merge-beta-2-esr.sh if dropping the beta designation here causes problems for Seamonkey.

I don't know if this change was ever made? But it seems that both 1 and 3 were done.

(In reply to Patrick Cloke [:clokep] from comment #14)

https://hg.mozilla.org/users/bugzilla_standard8.plus.com/drivertools/rev/36cc980832b4263f9d1637e27d11d40d08c2cb0e

(In reply to Rob Lemley [:rjl] from comment #0)

  1. merge-beta-2-release.sh - Similar to where merge-central-2-beta.sh updates version_display.txt (substr a1 to b1), the bX needs to be dropped from version_display.txt when going to release. Alternatively this could go in merge-beta-2-esr.sh if dropping the beta designation here causes problems for Seamonkey.

I don't know if this change was ever made? But it seems that both 1 and 3 were done.

I looked further and we already seem to do this, see https://hg.mozilla.org/users/bugzilla_standard8.plus.com/drivertools/file/36cc980832b4263f9d1637e27d11d40d08c2cb0e/comm-merges/merge-beta-2-release.sh#l28

I think this is all set.

From comment 12:

What benefit is there in NOT pushing these changes?

The only benefit I've heard is that TortoiseHg shows all branches by default, but there's an easy workaround of turning off "inactive branches".

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 79.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: