630929 - Update generateCCBranchObjects to match interim changes in the main Mozilla version

Status: RESOLVED FIXED

(SeaMonkey :: Release Engineering, defect)

Description

[Justin Wood (:Callek)]

Attachment: WIP 1

generateBranchObjects in buildbotcustom has changed enough that it is worth going through it with a comb to try and match what we can/should... I am doing this now.

Attaching my WIP, and requesting feedback incase KaiRo has any early comments.

Comment 1

[Robert Kaiser]

Comment on attachment 509157 [details] [diff] [review]
WIP 1

Hrm, I thought I had synced everything needed when I did the buildbot 0.8 migration...

>+    # Try Server notifier

Doesn't hurt and I added some try stuff at other places, but in the end, SeaMonkey isn't running try anyhow, and won't for the near future due to lack of hardware. I guess it's OK to have this in here, even if it's unused on our side.

>-            builderNames=coverageBuilders + weeklyBuilders,
>+            builderNames=weeklyBuilders,

And where do coverageBuilders end up now?

>+            mozconfig=pf['mozconfig_dep'], # XXXCallek do we really need _dep

Now that we switched branch to do static builds for both dep and nightly, we probably can actually use the same for both like Firefox does. Doesn't change much, but aligns us better.

>+            branchName=name, # XXXCallek Needed?

I think so, yes, else IIRC it doesn't get to know about "comm-central-trunk".


The rest is cosmetic mostly, but I guess sync doesn't do any harm in the end, but makes keeping things in sync easier :)

Comment 2

[Justin Wood (:Callek)]

(In reply to comment #1)
> Comment on attachment 509157 [details] [diff] [review]
> WIP 1

> >-            builderNames=coverageBuilders + weeklyBuilders,
> >+            builderNames=weeklyBuilders,
> 
> And where do coverageBuilders end up now?

Hrm, it actually looks like this merge with the main side was a bad idea; good catch. It seems dustin accidentally patched our end here, and not his own side, and thus left them without a real builder for it. I mentioned this on the bug which changed it (Bug 437482)

Comment 3

[Justin Wood (:Callek)]

Attachment: doit

Comment 4

[Robert Kaiser]

Comment on attachment 509342 [details] [diff] [review]
doit

>-    # Separate notifier for unittests, since they need to be run through
>-    # the unittest errorparser

BTW, I seem to remember that there might be a bug out there that wanted us to switch dep/nightly builders to the unittest parser, which you are doing here. Could you look for that and mark it to reflect you're fixing it here?

>-                    'builddir': reallyShort('%s-%s-shark' % (name, platform)),
>+                    'slavebuilddir': reallyShort('%s-%s-shark' % (name, platform)),

Haha, I guess it's good we don't actually run shark builds for SeaMonkey. ;-)

>-                brandName=config['brand_name'],

Are you really sure we can drop this one? It might not generate an error here, as we generate the brandName by first-letter-capitalizing the lowercase var (productName?), but SeaMonkey has another capital letter in the middle of the brand name and therefore we might end up with something wrong.

>+            # This would only update m-c blocklist as it stands, see Bug 630526

Er, the comment is suboptimal, as there is no "m-c blocklist", there is one per product, so I guess you mean the "Firefox blocklist". ;-)


r=me with the nits investigated/fixed.

Comment 5

[Justin Wood (:Callek)]

(In reply to comment #4)
> Comment on attachment 509342 [details] [diff] [review]
> doit
> 
> BTW, I seem to remember that there might be a bug out there that wanted us to
> switch dep/nightly builders to the unittest parser, which you are doing here.
> Could you look for that and mark it to reflect you're fixing it here?

Done [Bug 621345]

> >-                brandName=config['brand_name'],
> 
> Are you really sure we can drop this one? It might not generate an error here,
> as we generate the brandName by first-letter-capitalizing the lowercase var
> (productName?), but SeaMonkey has another capital letter in the middle of the
> brand name and therefore we might end up with something wrong.

Whops, you are right. I thought I looked into this before removing it too.

> >+            # This would only update m-c blocklist as it stands, see Bug 630526
> 
> Er, the comment is suboptimal, as there is no "m-c blocklist", there is one per
> product, so I guess you mean the "Firefox blocklist". ;-)

Fixed with: "This would only update Firefox blocklist as it stands, see Bug 630526"

https://hg.mozilla.org/build/buildbotcustom/rev/b1ea5695e062
[followup] https://hg.mozilla.org/build/buildbotcustom/rev/50da0c39c8a2

And also reconfiged the buildmaster.