541582 - New packaged tests builds don't report "moz:" rev

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

Old monolithic builds waterfall:
{
rev:ca986f28d2c1
moz:6e60470b04ab
}

New packaged builds:
{
rev:ca986f28d2c1
}

NB: Filed in SeaMonkey, but may be a (RelEng) buildbotcustom issue?

Comment 1

[Robert Kaiser]

Right, I've seen this, I'm not 100% sure what to do about it - we're using the generic factories here, and need no single change anywhere right now, but for adding this, we would. I'm not yet sure neither if that's good or how to reasonably achieve it.

Comment 2

[Robert Kaiser]

Attachment: add a printMozillaRev option that prints the Mozilla/platform revision

OK, I figured out a patch, though it needs this additional option to the factory, but with that one, we just parse platform.ini as well and get the Mozilla revision from there to print it.

This patch is currently in use in the SeaMonkey buildmaster.

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 423193 [details] [diff] [review]
add a printMozillaRev option that prints the Mozilla/platform revision

>diff --git a/process/factory.py b/process/factory.py
>+        if self.printMozillaRev:
>+            # Figure out platform/Mozilla revision
>+            def get_moz_build_info(rc, stdout, stderr):
>+                retval = {}
>+                stdout = "\n".join([stdout, stderr])
>+                m = re.search("^BuildID=(\w+)", stdout, re.M)
>+                if m:
>+                    retval['moz_buildid'] = m.group(1)
>+                m = re.search("^SourceStamp=(\w+)", stdout, re.M)
>+                if m:
>+                    retval['moz_revision'] = m.group(1)
>+                m = re.search("^SourceRepository=(\S+)", stdout, re.M)
>+                if m:
>+                    retval['moz_repo_path'] = m.group(1)
>+                return retval

Is there any reason for buildid to be here?

Also, I'm not sure it makes sense for this to be in the base class...it makes sense in the SeaMonkey or Thunderbird context, but flipping it on for Firefox would just cause us to print the same thing twice. Can you subclass for this?

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> Is there any reason for buildid to be here?

Not much, it can probably just be left out.

> Also, I'm not sure it makes sense for this to be in the base class...it makes
> sense in the SeaMonkey or Thunderbird context, but flipping it on for Firefox
> would just cause us to print the same thing twice. Can you subclass for this?

It's not on by default, as you might have seen. Having this rather minimal piece of code available doesn't mean it needs to be turned on and used everywhere.
For how this class is written, subclassing is vastly difficult, and this printing thing is the only difference we need at all, subclassing just for that seems like overdoing things for me. Also, AFAIK, mobile uses two different repos, couldn't this be helpful there as well?

Comment 5

[Robert Kaiser]

Attachment: [tools] add a new script to print build data

Here's a script for build tools to print out properties of a build in a certain directory.

Comment 6

[Robert Kaiser]

Attachment: [buildbotcustom] use tools script for printing data

This is the patch I imagine for buildbotcustom to use the script from the other attachment - I couldn't test this yet because I have no way of running buildbot with this as long as the script is not in tools.

The script for tools has been tested from the commandline.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 424807 [details] [diff] [review]
[tools] add a new script to print build data

>diff --git a/buildfarm/utils/printbuildrev.py b/buildfarm/utils/printbuildrev.py
>new file mode 100644
>--- /dev/null
>+++ b/buildfarm/utils/printbuildrev.py
>@@ -0,0 +1,31 @@
>+# print build ID and to stdout

Nit: no 'and'.

>+# and print revision(s) for tinderbox scraping

Comment 8

[Robert Kaiser]

Attachment: [checked in] [buildbotcustom] use tools script for printing data

I had a slight glitch in the call in the last version, this version is now running on the SeaMonkey buildmaster for testing, and I'm using a clone of build/tools with the other patch on the slaves now.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 424807 [details] [diff] [review]
[tools] add a new script to print build data

The contents of the script are fine. r+ with a nit fixed:
Update the example usage to say /path/to/appdir or something similar - /path/to/dist/bin doesn't seem to be accurate.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 425136 [details] [diff] [review]
[checked in] [buildbotcustom] use tools script for printing data

I ran this through staging, and it looks good to me.

Comment 11

[Robert Kaiser]

(In reply to comment #9)
> (From update of attachment 424807 [details] [diff] [review])
> The contents of the script are fine. r+ with a nit fixed:
> Update the example usage to say /path/to/appdir or something similar -
> /path/to/dist/bin doesn't seem to be accurate.

Landed the script as http://hg.mozilla.org/build/tools/rev/611e60035b1b

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 425136 [details] [diff] [review]
[checked in] [buildbotcustom] use tools script for printing data

changeset:   598:b0d6f82693da

Comment 13

[Robert Kaiser]

That makes this bug fixed, actually ;-)