614254 - Make all builddir names really short to avoid dir name length compile fails

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [tested] makes really short builddir names

Bug 608456 is the most recent example of this, but also in my staging release testing last week my windows builds were failing compile due to directory name (and path) length. I tested the attached patch with my win32 rebuild and successfully compiled.  While we also need to encourage developers to not go down as many levels in order to keep path shorter and within win32's character limit, shortening our builddir names will buy us more wiggle room.

The attached patch makes builddir names like:

On Talos/Test masters:

R3MSL162swcldgttm64ms45                               tyw7tta11y
R3MSL162swcldgttm64ms55                               tyw7ttce

On Builder masters:

W52medgttw32dgms25                                maclmxdg
W52medgttw32dgms35                                maclw32

Unittest names had to use the name_prefix instead of builder_prefix to be sure of uniqueness.

Comment 1

[[e-mail] (:bhearsum)]

Can we use something a _little_ more readable? This is going to really painful and error prone when we have to hop onto slaves.

Comment 2

[Chris AtLee [:catlee]]

Comment on attachment 492653 [details] [diff] [review]
[tested] makes really short builddir names

So the transformation you're applying here is really opaque...is it supposed to be readable?

What is reallyShort supposed to be doing?

I also wouldn't be surprised if some of our code depends on the builddir being equal to the builder name for some stuff.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 492653 [details] [diff] [review]
[tested] makes really short builddir names

After discussing with Catlee, there are a few issues that need testing here:

* slaves can share the same builddir name (slavebuilddir) but the master needs a unique builddir name and our master's builddir names are relied upon by things like status_db
* would need to test with actual builds that our tools aren't broken by renaming the slave-side dir names (ie: clobber, purge_builds)
* the shortened names need to be more readable
* unittests could all go in the same dir since they are not used for anything and are deleted before every test run

Also note that this bug is similar (but not a dupe) to bug 586664

Comment 5

[Rail Aliiev [:rail]]

Long path names block win32 release build as well. :(

Comment 6

[Rail Aliiev [:rail]]

I also filed bug 616542.

Comment 7

[Rail Aliiev [:rail]]

Firefox 4.0b8 release blocked by too long path names. Set as a blocker.

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: simple mapping exercise

simple function that shortens names in way that yields human readable, yet still short.  Using a static mapping allows us to use a human to optimize the shortenings.  This should be able to integrate into buildbotcustom/misc.py the same way that lsblakk's patch did.

sample output:

$ ./name.py
cen-lnx-bld
cen-lnx-xr
tm-lnx-bld
mb-w32-bld
rel-w32-bld

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [needs testing] shorter and more readable builddir names, one 'test' dir for test builders

Used jhford's mapping, it works well and passes checkconfig and start on a local master.  Now I need to test it with slaves on test-master to be sure that the slaves work with one 'test' dir. Should also do a staging release run with this.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

I've run this on a staging tests-master and it's working with just the 'test' dir as a slavebuilddir.

Comment 12

[Rail Aliiev [:rail]]

Comment on attachment 496261 [details] [diff] [review]
[needs testing] shorter and more readable builddir names, one 'test' dir for test builders

> +    mappings = {
> +        'mozilla': None,

Is this some kind of place holder to prevent other people to replace 'mozilla' word?

>      def builderPrefix(s, platform=None):
>          if platform:
> -            return "release-%s-%s_%s" % (releaseConfig['sourceRepoName'], platform, s)
> +            return reallyShort("release-%s-%s_%s" % (releaseConfig['sourceRepoName'], platform, s))
>          else:
> -            return "release-%s-%s" % (releaseConfig['sourceRepoName'], s)
> +            return reallyShort("release-%s-%s" % (releaseConfig['sourceRepoName'], s))

This will change the builder names as well, what will make them less readable. 

Could you use the same approach here without toucing builderPrefix? Something like this:

'builddir': reallyShort(builderPrefix('%s_mu_verify' % platform)),

Comment 13

[Rail Aliiev [:rail]]

> +        'release': 'rel',

Don't forget that this also requires changes in clobberer and ignore_dirs in factory.py.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [partially tested] shortens builddir name, puts all tests in one slavebuilddir, without affecting release builder names

So this works so far on a test-master, also the mapping of 'mozilla': 'None' is to remove 'mozilla' from the title of the dir, not just to reserve it. That takes away a decent amount of length and doesn't obscure what the dir is.  Same for 'desktop'.

Also, I did what you suggested and moved reallyShort() to the builddirs on release.py instead of in builderPrefix.  I personally don't find the names that unreadable, and I liked being able to cover everything in once place but if others are concerned about waterfall builder names then this way will work best for not changing those.

ignore_dirs is updated in this patch.  I didn't see anything in index.php for clobberer to update a RELEASE_PREFIX - so clobberer will need to be tested to see if adjustments will be necessary to pick up the new builddir names.

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: update for short names in clobberer

and this is what you get when you update your local repo :)

Comment 16

[Rail Aliiev [:rail]]

Comment on attachment 496332 [details] [diff] [review]
[partially tested] shortens builddir name, puts all tests in one slavebuilddir, without affecting release builder names

Looks good.

A nit:

>                      'properties': {
>                          'branch': branch,
>                          'platform': slave_platform,
>                          'build_platform': platform,
>                          'builddir': builddir,
> +                        'slavebuilddir': slavebuilddir,
>                          },

Is it necessary to add this property?

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to comment #16)
> Comment on attachment 496332 [details] [diff] [review]
> [partially tested] shortens builddir name, puts all tests in one slavebuilddir,
> without affecting release builder names
> 
> Looks good.
> 
> A nit:
> 
> >                      'properties': {
> >                          'branch': branch,
> >                          'platform': slave_platform,
> >                          'build_platform': platform,
> >                          'builddir': builddir,
> > +                        'slavebuilddir': slavebuilddir,
> >                          },
> 
> Is it necessary to add this property?

yes - so that anything run on talos machines is run in the same 'test' dir.

Comment 18

[Rail Aliiev [:rail]]

Comment on attachment 496336 [details] [diff] [review]
update for short names in clobberer

    /@
    \ \
  ___> \
 (__O)  \
(____@)  \
(____@)   \
 (__o)_    \
       \    \

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: [tested] makes really short builddir names in slavebuilddir only

So after running this on staging it was discovered that changing the builddir names on the master is something that requires a restart whereas with a slavebuilddir, the change is created on the buildslave without issue.  Now that I've tested this and know it to make good, short names I believe this will give us the short slavebuilddir names we need to stop failing compile without requiring a restart of all the masters.

Comment 20

[Rail Aliiev [:rail]]

Comment on attachment 497002 [details] [diff] [review]
[tested] makes really short builddir names in slavebuilddir only

Looks good to me.

Comment 21

[Robert Strong (they/them - no direct email)]

Looks like this is happening on mozilla-1.9.2 as well
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6/1292358596.1292366930.8046.gz

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 497002 [details] [diff] [review]
[tested] makes really short builddir names in slavebuilddir only

http://hg.mozilla.org/build/buildbotcustom/rev/c28f34b98879

Comment 23

[Chris AtLee [:catlee]]

Attachment: Bustage fix for try logs

Comment 24

[Chris AtLee [:catlee]]

Comment on attachment 497718 [details] [diff] [review]
Bustage fix for try logs

landed on default and production-0.8

Comment 25

[[e-mail] (:bhearsum)]

I think this bug is all done now.

Comment 26

[Marco Castelluccio [:marco]]

There is a blocking bug (https://bugzilla.mozilla.org/show_bug.cgi?id=616542) before this can be marked as fixed, I think.