Closed Bug 1203838 Opened 4 years ago Closed 4 years ago

Thunderbird release builds don't update update gecko to the release tag


(Thunderbird :: Build Config, defect)

Not set


(thunderbird42 wontfix, thunderbird43 fixed, thunderbird44 fixed, thunderbird_esr38 fixed)

Thunderbird 44.0
Tracking Status
thunderbird42 --- wontfix
thunderbird43 --- fixed
thunderbird44 --- fixed
thunderbird_esr38 --- fixed


(Reporter: nthomas, Assigned: Fallen)



(1 file)

tl;dr - on comm-esr38 we get the tip of the verbranch, which is missing the version bump. On comm-beta might easily get extra gecko commits which weren't intended (but this isn't checked yet).

Long form -

* buildbot pulls comm-esr38 and updates to tag correctly
/usr/local/bin/hg clone --verbose --noupdate build
/usr/local/bin/hg update --clean --repository build --rev default
hg up -C -r THUNDERBIRD_38_2_0_RELEASE

* the mozconfig has
mk_add_options CLIENT_PY_ARGS="$([ -f $topsrcdir/build/ ] && cat $topsrcdir/build/"
mk_add_options ALWAYS_RUN_CLIENT_PY=1
which hard codes the mozilla-rev.

* buildbot calls 'make -f build' in comm-esr38 checkout, with |COMM_REV="THUNDERBIRD_38_2_0_RELEASE"| and |MOZILLA_REV="THUNDERBIRD_38_2_0_RELEASE"| in the environment. These will be ignored later.

* the call looks like this
python2.7 /builds/slave/tb-rel-c-esr38-l64_bld-0000000/build/ checkout --hg-options='--time' --hgtool=../tools/buildfarm/utils/ --hgtool1=../scripts/buildfarm/utils/ --skip-chatzilla --skip-comm --skip-inspector --tinderbox-print --mozilla-repo= --mozilla-rev=THUNDERBIRD_38_VERBRANCH

* the hgtool command is 
['python', '../tools/buildfarm/utils/', '', '/builds/slave/tb-rel-c-esr38-l64_bld-0000000/build/mozilla']
and we end up with the tip of repo from an |hg up -C|, namely 3f64f554dd43

* does a |hg update -r THUNDERBIRD_38_VERBRANCH|, and we end up at 5f77862a6e66

This happens to be the tip of the verbranch, but is vulnerable to other changes landing. And actually we wanted e72e9054d9eb, which is the relbranch landing which bumps the gecko version from 38.2.0esrpre to 38.2.0.
This affects esr38 builds, both en-US and l10n, but not comm-beta. 

It probably stems from this difference in build/
  --mozilla-repo= --mozilla-rev=THUNDERBIRD_38_VERBRANCH
Looks like we can fix this by preferring MOZILLA_REV from the environment in and ignoring the --mozilla-rev argument. This might be different than other mozilla tools that use the env as default and then use the args to overwrite, but it would solve the problem mentioned. Does this sound sensible to you?

I checked the logs, the onpush builds don't have MOZILLA_REV set and will therefore use THUNDERBIRD_38_VERBRANCH:

The candidate builds do have MOZILLA_REV set, to e.g. THUNDERBIRD_38_2_0_RELEASE, which contains the version bumps.

Sound good?
Flags: needinfo?(nthomas)
That would work I think, although is a little counter-intuitive. What if we put this in
   ...  --mozilla-rev=${MOZILLA_REV:=THUNDERBIRD_38_VERBRANCH}

Would the shell have a chance to expand that ?
Flags: needinfo?(nthomas)
Unfortunately not at the moment, right now the line that parsers is here:

this is passed to here:

I'd have to eval the string at some point, I guess in
Ok, the solution I found to expand variables is also not very intuitive. The problem is that CLIENT_PY_ARGS is loaded via mozconfig's mk_add_options, which makes it a recursive make variable and inner variables are automatically expanded. The $ in the above will be interpreted as a make variable and with the {} it gets swallowed. The only way to make it work is using two dollar signs:

 ...  --mozilla-rev=$${MOZILLA_REV:-THUNDERBIRD_38_VERBRANCH}

Would you prefer this option, or the patch?
Flags: needinfo?(nthomas)
Also seems fragile, passing so many expansions before it's used. I'd probably prefer the over that, especially if it emits messages/warnings.
Flags: needinfo?(nthomas)
Attached patch Fix - v1 β€” β€” Splinter Review
Ok, then this patch should do it. Warnings and messages are shown.
Assignee: nobody → philipp
Attachment #8662914 - Flags: review?(bugspam.Callek)
Comment on attachment 8662914 [details] [diff] [review]
Fix - v1

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

stamp, I also grant a+ for whatever uplifts you need. (I can't officially set those a+'s in this BMO product, but I still grant it)
Attachment #8662914 - Flags: review?(bugspam.Callek) → review+
When will we land this ?
Moving this to product Thunderbird and setting checkin-needed so it will be in our bug queries. c-c looks a bit broken right now, but by me we can land this any time.
Component: Release Automation → Build Config
Keywords: checkin-needed
Product: Release Engineering → Thunderbird
QA Contact: bhearsum
Comment on attachment 8662914 [details] [diff] [review]
Fix - v1

Setting a+ from Callek
Attachment #8662914 - Flags: approval-comm-esr38+
Attachment #8662914 - Flags: approval-comm-beta+
Attachment #8662914 - Flags: approval-comm-aurora+
Philipp, I assume you will do the actual checkins when appropriate.
I'll keep an eye on this, but anyone processing checkin-needed can do so.
Bug 1203838 - Thunderbird release builds don't update update gecko to the release tag. r=Callek a=aleth CLOSED TREE
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 44.0
You need to log in before you can comment on or make changes to this bug.