Closed Bug 488099 Opened 13 years ago Closed 13 years ago

Expose build properties of l10n repacks via TinderboxPrint for scraping

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Pike, Assigned: coop)

References

Details

(Whiteboard: [l10n])

Attachments

(3 files, 2 obsolete files)

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.
Whiteboard: [l10n]
Covered in l10n meeting this morning; coop will deal with this later this week.
Assignee: nobody → ccooper
Priority: -- → P3
I assume we'd want these output at the front end of the build to make diagnosing failures a little easier?
Status: NEW → ASSIGNED
Priority: P3 → P2
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.
(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?
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.
(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.
Attachment #374287 - Flags: review?(l10n)
Attachment #374287 - Flags: review?(l10n) → review+
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.
I have a TinderboxPrint patch running on staging-master right now.
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.
(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?
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.
(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?
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.
Attachment #377419 - Flags: review?(l10n)
Attached patch Add l10n_tree var to config.py (obsolete) — Splinter Review
Attachment #374287 - Attachment is obsolete: true
Attachment #377422 - Flags: review?(l10n)
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.
(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.
Attachment #377422 - Attachment is obsolete: true
Attachment #377445 - Flags: review?(l10n)
Attachment #377422 - Flags: review?(l10n)
Attachment #377445 - Flags: review?(l10n) → review+
Attachment #377419 - Flags: review?(l10n) → review+
Comment on attachment 377419 [details] [diff] [review]
Expose build properties via TinderboxPrint

changeset:   307:36736618d5bc
Attachment #377419 - Flags: checked‑in+
Comment on attachment 377445 [details] [diff] [review]
Add l10n_tree var to config.py, v2

changeset:   1158:41313c7b6207
Attachment #377445 - Flags: checked‑in+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Blocks: 494367
Depends on: 494582
Reopening, we shouldn't default tree to None when it hangs slaves.
Status: RESOLVED → REOPENED
No longer depends on: 494582
Resolution: FIXED → ---
Depends on: 494582
One attempt to fix the default behaviour is attachment 379382 [details] [diff] [review], but that has some issues, see bug 494582 comment 3.
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.
Status: REOPENED → ASSIGNED
Attachment #379571 - Flags: review+
Attachment #379571 - Flags: review?(nthomas)
Comment on attachment 379571 [details] [diff] [review]
Use "notset" as the default for the tree property.

changeset:   313:3acbbda0603b
Attachment #379571 - Flags: checked‑in+
production-master reconfig-ed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.