981225 - Hazard builds uploading to wrong FTP dir

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Steve Fink [:sfink] [:s:]]

I just noticed that hazard builds upload to a directory name containing the full hash, whereas other builds use the short hash. For example,

https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1decc1b983197f97861b834797a1078d77f3647d/

instead of

https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1decc1b98319/

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Use short revisions to fix try upload dir path

Sorry jlund for making you my reviewmonkey while aki's out. :-)

This turns out to be a little more important, because I'm going to need to infer the hazard output directory from the log directory, and it would be nice if they were the same.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Pass through the build ID to mozharness builds

Not really the same problem, but related, and it sort of matches the bug title: non-try builds do not pass through the buildid, which means that when the builds want to upload something, they have to generate their own buildid (based on the current time). I need the buildid to be predictable (specifically from information in a pulse message). Or rather, it's much better if it is predictable so I don't have to grovel through the logs.

So just pass it through. b2g_build.py grabs it out of BuildID and so will be unaffected by this change. spidermonkey_build.py and hazard_build.py both need to switch from using buildtime to buildid, and then they'll get it from here.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Switch from using buildtime to buildid

here's the switch to buildid

I could do this in buildb2gbase.py instead, but I don't know if maybe b2g_build.py prefers to get it from platform.ini. I could see an argument for continuing to use that.

Comment 4

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

Oops, name collision. All 3 patches are still valid.

Comment 5

[Jordan Lund (:jlund)]

Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

Review of attachment 8450636 [details] [diff] [review]:
-----------------------------------------------------------------

in-line need-info before I finish review.

::: misc.py
@@ +1579,5 @@
>                  # finished all the builders needed for this platform and
>                  # there is nothing left to do
> +                factory = makeMHFactory(config, pf,
> +                                        signingServers=secrets.get(pf.get('dep_signing_servers')),
> +                                        copy_properties=['buildid', 'builduid'],

so are we sure we want this here? Looking at copy_properties, I think what it will do is pass the same buildid and builduid to any builds that are triggered from this, e.g. nightly builds that trigger l10n schedulers: http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#6248

AFAIK, you should get the buildid and builduid passed on to spider/hazard builds if it exists. If I look at say this job: https://tbpl.mozilla.org/php/getParsedLog.php?id=43212519&full=1&branch=mozilla-central and then check out the properties in buildprops.json, the buildid and builduid are already there. Sorry if I'm mistaken, just sanity checking.

Comment 6

[Jordan Lund (:jlund)]

Comment on attachment 8450445 [details] [diff] [review]
Use short revisions to fix try upload dir path

Review of attachment 8450445 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm. once all the build things land in mozharness, it will be nice to clean up duplicate code, e.g. http://mxr.mozilla.org/build/search?string=def%20query_revision

::: scripts/spidermonkey_build.py
@@ +207,2 @@
>  
> +        return revision[0:12] if revision else None

I think I am going to need this splice in my BuildScript.query_revision() too. thanks for discovering this for me :)

Comment 7

[Jordan Lund (:jlund)]

Comment on attachment 8450637 [details] [diff] [review]
Switch from using buildtime to buildid

Review of attachment 8450637 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 8

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

(In reply to Jordan Lund (:jlund) from comment #5)
> Comment on attachment 8450636 [details] [diff] [review]
> Pass through the build ID to mozharness builds
> 
> Review of attachment 8450636 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> in-line need-info before I finish review.
> 
> ::: misc.py
> @@ +1579,5 @@
> >                  # finished all the builders needed for this platform and
> >                  # there is nothing left to do
> > +                factory = makeMHFactory(config, pf,
> > +                                        signingServers=secrets.get(pf.get('dep_signing_servers')),
> > +                                        copy_properties=['buildid', 'builduid'],
> 
> so are we sure we want this here?

Not sure at all.

> Looking at copy_properties, I think what
> it will do is pass the same buildid and builduid to any builds that are
> triggered from this, e.g. nightly builds that trigger l10n schedulers:
> http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#6248
> 
> AFAIK, you should get the buildid and builduid passed on to spider/hazard
> builds if it exists. If I look at say this job:
> https://tbpl.mozilla.org/php/getParsedLog.
> php?id=43212519&full=1&branch=mozilla-central and then check out the
> properties in buildprops.json, the buildid and builduid are already there.
> Sorry if I'm mistaken, just sanity checking.

I was totally cargo-culting. I searched for 'buildid' and copied everything I found without bothering to check whether I needed it. Thank you very much for the catch!

Comment 9

[Steve Fink [:sfink] [:s:]]

(In reply to Jordan Lund (:jlund) from comment #6)
> Comment on attachment 8450445 [details] [diff] [review]
> Use short revisions to fix try upload dir path
> 
> Review of attachment 8450445 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm. once all the build things land in mozharness, it will be nice to clean
> up duplicate code, e.g.
> http://mxr.mozilla.org/build/search?string=def%20query_revision

spidermonkey_build.py is badly named and hazard_build.py is totally cut & paste from it. It's a total mess, but it was hard enough getting it working that I wanted to do it separately, without worrying about generalizing things to work for the existing hazard builds too. So yes, you're absolutely right, as soon as all of this stuff works it'll be time to merge it all together.

Comment 10

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/build/mozharness/rev/c6dd0055188d
http://hg.mozilla.org/build/mozharness/rev/081bbea1a00b

Comment 11

[Pete Moore [:pmoore][:pete]]

About to roll this out to production...

Comment 12

[Pete Moore [:pmoore][:pete]]

In production