488099 - Expose build properties of l10n repacks via TinderboxPrint for scraping

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Axel Hecht [:Pike]]

We should expose a few build properties to scraping via TinderboxPrint for l10n builds:

locale, tree, buildnumber, repo revisions (l10n_revision and en_revision for fx, plus toolkit_revision for fennec).

It smells a bit over the top to call echo on the slave, but the other builders do that, so could probably do l10n. A custom step just on the master would do, too.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Covered in l10n meeting this morning; coop will deal with this later this week.

Comment 2

[Chris Cooper [:coop] (he/him)]

I assume we'd want these output at the front end of the build to make diagnosing failures a little easier?

Comment 3

[Axel Hecht [:Pike]]

Yeah. We have locale, tree, buildnumber as soon as we start the build, but we won't have the en-US repo revisions until we configured, wgot, unpacked and idented.

Might make sense to split that up into two.

Comment 4

[Chris Cooper [:coop] (he/him)]

(In reply to comment #3)
> Yeah. We have locale, tree, buildnumber as soon as we start the build, but we
> won't have the en-US repo revisions until we configured, wgot, unpacked and
> idented.

Axel: what piece of data does "tree" correspond to? Do you mean branch, or something else?

Comment 5

[Axel Hecht [:Pike]]

tree would be fx31x or fx35x, aka, the title of the section in l10nbuilds.ini. That should already be a build property.

We should rename the fx31x and fx32x to 35 and 36, too.

Comment 6

[Chris Cooper [:coop] (he/him)]

(In reply to comment #5)
> tree would be fx31x or fx35x, aka, the title of the section in l10nbuilds.ini.
> That should already be a build property.

OK, I was looking at a 1.9.0 CVS build which doesn't seem to have that property, but I see it now on m-c/1.9.1/fennec.

Comment 7

[Chris Cooper [:coop] (he/him)]

Attachment: Rename fx31x and fx32x to 35 and 36

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 374287 [details] [diff] [review]
Rename fx31x and fx32x to 35 and 36

r=me. For deployment, I'm pretty sure a reconfig should be good enough.

Comment 9

[Chris Cooper [:coop] (he/him)]

I have a TinderboxPrint patch running on staging-master right now.

Comment 10

[Chris Cooper [:coop] (he/him)]

Axel: running into a little problem here with 'tree' again. It appears that m-c builds have the property set but 1.9. builds do not, e.g.:

http://production-master.build.mozilla.org:8010/builders/Linux%20mozilla-1.9.1%20l10n/builds/2417

vs.

http://production-master.build.mozilla.org:8010/builders/Linux%20mozilla-central%20l10n/builds/2230

This is happening on all three platforms.

Comment 11

[Chris Cooper [:coop] (he/him)]

(In reply to comment #10)
> Axel: running into a little problem here with 'tree' again. It appears that m-c
> builds have the property set but 1.9. builds do not, e.g.:

My bad. Builds triggered by the nightly scheduler don't include the tree property, but builds triggered via locale changes do have the tree property.

Axel: how do you want me to proceed here re: displaying the tree info?

Comment 12

[Axel Hecht [:Pike]]

I expect us to get the tree back when we move over to triggering the nightlies, at least that's how it's wired up in my head.

Does the lack of a tree property error on WithProperties, or does it just come across empty? I don't recall.

If it doesn't error, I'd rather have it, but if we don't get that in, it's not a big deal either. It would just require that I hardcode downstream which builders correspond to which tree when incorporating the builds with the l10n dashboard.

Comment 13

[Chris Cooper [:coop] (he/him)]

(In reply to comment #12) 
> If it doesn't error, I'd rather have it, but if we don't get that in, it's not
> a big deal either. It would just require that I hardcode downstream which
> builders correspond to which tree when incorporating the builds with the l10n
> dashboard.

I'd prefer not to have you do a remote mapping. That's bound to get out of sync at some point. 

How about I look at the l10nbuild.ini file (via ConfigParser) to match the buildername and pull out the tree for you?

Comment 14

[Chris Cooper [:coop] (he/him)]

Attachment: Expose build properties via TinderboxPrint

Decided that the easiest way to get tree info for all the builds was to add the tree info to config.py and set it for all factories at creation time.

Axel: is there a reason why you need to maintain a separate l10nbuilds.ini file? This could be the first step towards folding the contents of l10nbuilds.ini into config.py. We wouldn't have to sync builder names, etc. if we had a single source.

config.py patch coming shortly.

Comment 15

[Chris Cooper [:coop] (he/him)]

Attachment: Add l10n_tree var to config.py

Comment 16

[Axel Hecht [:Pike]]

Comment on attachment 377422 [details] [diff] [review]
Add l10n_tree var to config.py

This patch looks incomplete. I'd expect a second chunk for the l10nbuilds.ini in mozilla2, and changes in both master.cfg to actually pass the tree over to the NightlyRepackFactory.

Regarding dropping l10n_builds.ini in favour of config.py, not sure. I find config.py hard, and wouldn't know how to map that to my setup. I'm not so fond of the resulting code with all details passed down in every call and constructor one by one. Not that I wouldn't see how to incrementally get into that situation.

Comment 17

[Chris Cooper [:coop] (he/him)]

(In reply to comment #16)
> (From update of attachment 377422 [details] [diff] [review])
> This patch looks incomplete. I'd expect a second chunk for the l10nbuilds.ini
> in mozilla2, and changes in both master.cfg to actually pass the tree over to
> the NightlyRepackFactory.

Yeah, my mistake. I'm working with some local copies and some symlinks here and the local copies didn't get picked up in the diff. I'll report shortly.

Comment 18

[Chris Cooper [:coop] (he/him)]

Attachment: Add l10n_tree var to config.py, v2

Comment 19

[Chris Cooper [:coop] (he/him)]

Comment on attachment 377419 [details] [diff] [review]
Expose build properties via TinderboxPrint

changeset:   307:36736618d5bc

Comment 20

[Chris Cooper [:coop] (he/him)]

Comment on attachment 377445 [details] [diff] [review]
Add l10n_tree var to config.py, v2

changeset:   1158:41313c7b6207

Comment 21

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

Reopening, we shouldn't default tree to None when it hangs slaves.

Comment 22

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

One attempt to fix the default behaviour is attachment 379382 [details] [diff] [review], but that has some issues, see bug 494582 comment 3.

Comment 23

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

From bug 494582 comment 6:

PS: The right place to set the default would have been in DependentL10n,
setting it to "notset" or something instead of None.

Comment 24

[Chris Cooper [:coop] (he/him)]

Attachment: Use "notset" as the default for the tree property.

Comment 25

[Chris Cooper [:coop] (he/him)]

Comment on attachment 379571 [details] [diff] [review]
Use "notset" as the default for the tree property.

changeset:   313:3acbbda0603b

Comment 26

[Chris Cooper [:coop] (he/him)]

production-master reconfig-ed.