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

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

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

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 -
eg http://archive.mozilla.org/pub/mozilla.org/thunderbird/candidates/38.2.0-candidates/build1/logs/release-comm-esr38-linux64_build-bm73-build1-build0.txt.gz

* buildbot pulls comm-esr38 and updates to tag correctly
/usr/local/bin/hg clone --verbose --noupdate https://hg.mozilla.org/releases/comm-esr38 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/client.py-args ] && cat $topsrcdir/build/client.py-args)"
mk_add_options ALWAYS_RUN_CLIENT_PY=1
which hard codes the mozilla-rev.

* buildbot calls 'make -f client.mk 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 client.py call looks like this
python2.7 /builds/slave/tb-rel-c-esr38-l64_bld-0000000/build/client.py checkout --hg-options='--time' --hgtool=../tools/buildfarm/utils/hgtool.py --hgtool1=../scripts/buildfarm/utils/hgtool.py --skip-chatzilla --skip-comm --skip-inspector --tinderbox-print --mozilla-repo=http://hg.mozilla.org/releases/mozilla-esr38 --mozilla-rev=THUNDERBIRD_38_VERBRANCH

* the hgtool command is 
['python', '../tools/buildfarm/utils/hgtool.py', 'http://hg.mozilla.org/releases/mozilla-esr38', '/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

* client.py 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.

Comment 1

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

This affects esr38 builds, both en-US and l10n, but not comm-beta. 

It probably stems from this difference in build/client.py-args
  --mozilla-repo=http://hg.mozilla.org/releases/mozilla-esr38 --mozilla-rev=THUNDERBIRD_38_VERBRANCH

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Looks like we can fix this by preferring MOZILLA_REV from the environment in client.py 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:
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/comm-esr38-linux/latest/comm-esr38-linux-nightly-bm72-build1-build10.txt.gz

The candidate builds do have MOZILLA_REV set, to e.g. THUNDERBIRD_38_2_0_RELEASE, which contains the version bumps.
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/candidates/38.2.0-candidates/build1/logs/release-comm-esr38-linux64_build-bm73-build1-build0.txt.gz

Sound good?

Comment 3

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

That would work I think, although is a little counter-intuitive. What if we put this in client.py-args:
   ...  --mozilla-rev=${MOZILLA_REV:=THUNDERBIRD_38_VERBRANCH}

Would the shell have a chance to expand that ?

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Unfortunately not at the moment, right now the line that parsers client.py-args is here:

https://dxr.mozilla.org/comm-central/source/mail/config/mozconfigs/linux32/release#26

this is passed to client.py here:

https://dxr.mozilla.org/comm-central/source/client.mk#468

I'd have to eval the string at some point, I guess in client.mk.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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 client.py-args 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 client.py patch?

Comment 6

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

Also seems fragile, passing so many expansions before it's used. I'd probably prefer the client.py over that, especially if it emits messages/warnings.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

Ok, then this patch should do it. Warnings and messages are shown.

Comment 8

[Justin Wood (: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)

Comment 9

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

When will we land this ?

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8662914 [details] [diff] [review]
Fix - v1

Setting a+ from Callek

Comment 12

[Kent James (:rkent)]

Philipp, I assume you will do the actual checkins when appropriate.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I'll keep an eye on this, but anyone processing checkin-needed can do so.

Comment 14

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/e0033c7a85976a17355ecae6988c71a94942ab52
Bug 1203838 - Thunderbird release builds don't update update gecko to the release tag. r=Callek a=aleth CLOSED TREE

Comment 15

[Kent James (:rkent)]

Comment on attachment 8662914 [details] [diff] [review]
Fix - v1

Pushed to beta (gecko 43) https://hg.mozilla.org/releases/comm-beta/rev/7c8712edcb1b

Comment 16

[Kent James (:rkent)]

Comment on attachment 8662914 [details] [diff] [review]
Fix - v1

https://hg.mozilla.org/releases/comm-esr38/rev/748fe457467d