827306 - ensure that non-release build dirs are the same length as release build dirs

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[[e-mail] (:bhearsum)]

bug 827305 tracks an issue where we hit the maximum path length problem on Windows during a release, but not during dep builds. This happened because the release build dir is 8 characters longer than the dep build dir ("tb-c-esr17-w32" vs "tb-rel-c-esr17-w32-bld"). Aki suggested that we could avoid hitting this problem at release time by ensuring that both directories are the same length (probably should make sure the nightly build dir is the same length, too). Ideally we'd do this by shortening the release build dir, not by lengthening the dep build one.

Comment 1

[[e-mail] (:bhearsum)]

I just realized that we could potentially bust at merge time if, for example, the aurora build dir name is longer than the mozilla-central build dir name. Maybe we should make all build directories for the same type of build the same length?

Comment 2

[[e-mail] (:bhearsum)]

OK, here's a concrete proposal:
* Figure out the maximum build directory length that we need.
* Change reallyShort (and probably rename it) to give names between a min and max size (configurable, but defaulting to what we want for dep/nightly builds).
* Any directories that are shorter than desired after substitution will be padded at the end with 'z' characters until they reach the desired length.
* Any builders that can't be shortened enough will raise exceptions, which will cause errors at checkconfig/reconfig time.

Comment 3

[Aki Sasaki (not active)]

We could simplify the logic if we:

a) prepend release dirs with rel- (already done)
b) prepend dep and nightly dirs with something the same length or longer (depend- and nightly- or dep- and ntl- or something).

But yeah, m-c should be the same length as m-a as m-b as m-r, so that's a good approach as well.

Comment 4

[[e-mail] (:bhearsum)]

I'll take care of this after the dependent bug is fixed.

Comment 5

[[e-mail] (:bhearsum)]

Attachment: add tests; refactor

This patch is a no-op change that does a few things:
* Renames reallyShort to normalizeName (because that's what it will do in the next patch)
* Adds a few tests
* Refactors the code to use regexes. I think it's much more readable this way.

I dumped out all the builder names before and after this patch and all of them are exactly the same.

Comment 6

[[e-mail] (:bhearsum)]

Attachment: add min+max support, sync up ALL of the lengths (except l10n)

Comment 7

[[e-mail] (:bhearsum)]

Attachment: changed directories

Comment 8

[[e-mail] (:bhearsum)]

A couple of notes from Callek via e-mail:
* IIRC we break clobberer when we change build directories but leave buildernames alone, so we'll need to plan to fix that.
* Current patch doesn't take into account prefix (tb-) for the length calc, I commented in bug on this one though.

Comment 9

[[e-mail] (:bhearsum)]

Attachment: address some comments/bugs

* Add a delimeter between the actually build dir and the padding
* Use 0 as the padding char because it's a bit less ugly.
* Take prefix into account before padding

Comment 10

[[e-mail] (:bhearsum)]

First patch is in production.

Comment 12

[[e-mail] (:bhearsum)]

12:25 < catlee-lunch> bhearsum: did you see that we need to add an import/reload of buildbotcustom.common?
12:25 < bhearsum> nope
12:25 < bhearsum> scary
12:27 < catlee-lunch> yeah, busted the reconfigs last night
12:27 < bhearsum> sorry about that :(
12:28 < bhearsum> so i need to add that as part of my next patch to that file, right?
12:28 < catlee-lunch> hmm
12:28 -!- catlee-lunch is now known as catlee
12:28 < catlee> process/factory.py reloads it
12:28 < catlee> but misc.py imports it first
12:29 < catlee> evil
12:29 < bhearsum> oh boy
12:29 < bhearsum> i'm going to have to touch reload order, aren't i?
12:29 -!- sfink [[e-mail]] has joined #releng

Comment 13

[[e-mail] (:bhearsum)]

Attachment: fix reloading

AFAICT we should be reloading in misc.py, because that's where we reload other things that are imported by it and factory.py (eg, misc_scheduler).

Comment 14

[[e-mail] (:bhearsum)]

Rough landing here -- it turns out that slavebuilddir is a shared reference. Meaning, updating it during a reconfig affects live builds. Eg, https://tbpl.mozilla.org/php/getParsedLog.php?id=19637638&tree=Try&full=1 had its build directory updated after compiling, which meant some subsequent steps failed because dependent files weren't in the new directory.

Comment 15

[[e-mail] (:bhearsum)]

Other than the builds in progress failing, this seems to have landed fine.

Comment 16

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #14)
> Rough landing here -- it turns out that slavebuilddir is a shared reference.
> Meaning, updating it during a reconfig affects live builds. Eg,
> https://tbpl.mozilla.org/php/getParsedLog.php?id=19637638&tree=Try&full=1
> had its build directory updated after compiling, which meant some subsequent
> steps failed because dependent files weren't in the new directory.

We hit more of these today during a reconfig. Turns out that not all masters had been reconfiged yesterday, according to the twistd.log's that I looked at. I'm going to make sure that everything is actually reconfiged with this change at this point.

Comment 17

[[e-mail] (:bhearsum)]

bug 840547 was filed to track the build-dir-changes-at-reconfig issue.
bug 840556 tracks a follow-up issue about not all of the expected builders getting padding.