Closed Bug 767501 Opened 12 years ago Closed 12 years ago

Try chooser doesn't work with new b2g builds

Categories

(Release Engineering :: General, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: bhearsum, Assigned: catlee)

References

Details

(Whiteboard: [trychooser][b2g])

We enabled new b2g builds today that were done in the style of the Thunderbird configs. That is, a separate "namespace" inside of our buildbot-configs rather than hacking them in as additional "platforms" in config.py. While testing, I noticed that try chooser appears to have no effect on them: both builds get triggered regardless of the syntax used.

This isn't a blocking issue, as the slaves they build on are currently underutilized. We need to get this figured out though.
AFAIK the new builders are already hooked up to the same Schedulers as Desktop Firefox try builds, so they *should* be getting processed for try syntax. Lukas tells me we need to update:
https://github.com/mozilla/buildbotcustom/blob/master/try_parser.py - the chooser code itself
https://github.com/mozilla/build-tools/tree/master/trychooser - the syntax builder

Given that we're treating B2G as its own Product in the buildbot-configs, I think we should try to add a new grouping to try chooser, such that you can do something like: "try: --product: b2g,firefox". I'm not sure how to handle specifying different platform options for different products in the same push. We might be able to punt on that and say "one product per push" for now.

Going to try looking at this next week.
Assignee: nobody → bhearsum
It looks the current behaviour is different than I thought. Some pushes seem to be getting b2g builds, eg: https://tbpl.mozilla.org/?tree=Try&rev=7a4009aef509

Some don't, eg: https://tbpl.mozilla.org/?tree=Try&rev=933c0451e1a6

I guess it's hitting some existing syntax or something....
Looks like without -p or with -p all you'll get b2g builds. Additionally, -b o/d works to control opt/debug....
(In reply to Ben Hearsum [:bhearsum] from comment #1)
> Given that we're treating B2G as its own Product in the buildbot-configs, I
> think we should try to add a new grouping to try chooser, such that you can
> do something like: "try: --product: b2g,firefox". I'm not sure how to handle
> specifying different platform options for different products in the same
> push. We might be able to punt on that and say "one product per push" for
> now.

Thinking about this more, it _really_ sucks to make people push twice to test different products. How about a format like this for being able to do both in one push?

try: --project b2g -b do -p gb_armv7a_gecko; --project firefox

A different separator could be used if desired. Implementation-wise, this would probably mean adjusting the current consumers of TryParser to call an intermediary instead, which would split up the syntax and call different TryParser's for each.
Catlee is going to take over here.
Assignee: bhearsum → catlee
another option is something like

try: -p win32 -t all (normal try syntax - gets you firefox builds)
try-b2g: -p armv7a (new syntax - gets you b2g builds)
Whiteboard: [trychooser]
I presume this is the same problem...

This push didn't get any builds at all:
https://tbpl.mozilla.org/?tree=Try&rev=9e944e26c03d
 try: -b do -f -p b2g -u none -t none 

And self-serve gives a "msg": "Revision 9e944e26c03d not found on branch try"
(In reply to Ed Morley [:edmorley] from comment #7)
> I presume this is the same problem...
> 
> This push didn't get any builds at all:
> https://tbpl.mozilla.org/?tree=Try&rev=9e944e26c03d
>  try: -b do -f -p b2g -u none -t none 
> 
> And self-serve gives a "msg": "Revision 9e944e26c03d not found on branch try"

This is because b2g is not a "platform" to buildbot, it's implemented as an entirely parallel product. "ics_armv7a_gecko" is a platform within the b2g product. Unfortunately, try chooser was designed with a single product in mind, which is why this bug isn't trivial to resolve.
Ah, that makes sense - thank you :-)

In the meantime, perhaps if we were just to remove the b2g option from trychooser, it would avoid confusion? (eg Marshall tried twice to get builds and thought he was just having to wait ages for the Try backlog, not knowing that nothing was ever going to be built).
I didn't realize there was a b2g option...yes we should definitely remove that. I'm not around until Wednesday but if someone wants to, the source is here: https://hg.mozilla.org/build/tools/file/default/trychooser/index.html.

After review/landing I think anyone from RelEng can update the try chooser code.
QA Contact: lsblakk → hwine
Priority: -- → P3
Whiteboard: [trychooser] → [trychooser][b2g]
Depends on: 791426
Removed from the platforms list in bug 791426.
Can we please reprioritize this?  Currently the top try users seem to be b2g devs, and that makes everyone's lives harder.
This is probably #3 or #4 on my list, behind a bunch of other busted things that need fixing.
(In reply to Ben Hearsum [:bhearsum] from comment #8)
> (In reply to Ed Morley [:edmorley] from comment #7)
> > I presume this is the same problem...
> > 
> > This push didn't get any builds at all:
> > https://tbpl.mozilla.org/?tree=Try&rev=9e944e26c03d
> >  try: -b do -f -p b2g -u none -t none 
> > 
> > And self-serve gives a "msg": "Revision 9e944e26c03d not found on branch try"
> 
> This is because b2g is not a "platform" to buildbot, it's implemented as an
> entirely parallel product. "ics_armv7a_gecko" is a platform within the b2g
> product. Unfortunately, try chooser was designed with a single product in
> mind, which is why this bug isn't trivial to resolve.

I guess I should have looked for this bug sooner.

ics_armv7a_gecko is indeed a platform, which means that -p ics_armv7a_gecko works. But it works in a kind of hacky way: all of the products are passed through the try parser and are filtered by the same syntax string. So -p all will give you b2g builds on all available platforms as well as the full set of firefox builds, and -p linux,ics_armv7a_gecko would give you firefox builds on linux and b2g builds on ics_armv7a_gecko.

Which is totally usable for now, since the platforms don't overlap and b2g doesn't have any tests defined. As soon as either of those changes, it'll be a mess and we'll have to do something.

The try chooser gets a buildbotBranch parameter that, assuming it is set correctly, could be used to distinguish projects. (Is the name "buildbotBranch" meaningful? I have no idea what it's supposed to mean. If we end up using it for this, I'd be inclined to change it to "product" or something.) Then any of these options would be straightforward to implement:

  try-b2g: ...usual syntax...
  try(b2g): ...usual syntax...
  try: --product b2g ...usual syntax...
  try: b2g(...usual syntax...) firefox(...usual syntax...)

and semicolon-separated combinations of any of the above (well, the try* ones wouldn't require semicolons.) Though do we even need to support multiple products in one push?

Or there's

  try: -p b2g[ics_armv7a_gecko],firefox[linux,macosx] ...

if you think of -p as either 'platform' *or* 'product'. (I'm thinking no product defaults to either firefox or any product.) Not sure if I'm kidding with this one or not.
(In reply to Steve Fink [:sfink] from comment #15)
> (In reply to Ben Hearsum [:bhearsum] from comment #8)
> > (In reply to Ed Morley [:edmorley] from comment #7)
> > > I presume this is the same problem...
> > > 
> > > This push didn't get any builds at all:
> > > https://tbpl.mozilla.org/?tree=Try&rev=9e944e26c03d
> > >  try: -b do -f -p b2g -u none -t none 
> > > 
> > > And self-serve gives a "msg": "Revision 9e944e26c03d not found on branch try"
> > 
> > This is because b2g is not a "platform" to buildbot, it's implemented as an
> > entirely parallel product. "ics_armv7a_gecko" is a platform within the b2g
> > product. Unfortunately, try chooser was designed with a single product in
> > mind, which is why this bug isn't trivial to resolve.
> 
> I guess I should have looked for this bug sooner.
> 
> ics_armv7a_gecko is indeed a platform, which means that -p ics_armv7a_gecko
> works. But it works in a kind of hacky way: all of the products are passed
> through the try parser and are filtered by the same syntax string. So -p all
> will give you b2g builds on all available platforms as well as the full set
> of firefox builds, and -p linux,ics_armv7a_gecko would give you firefox
> builds on linux and b2g builds on ics_armv7a_gecko.
> 
> Which is totally usable for now, since the platforms don't overlap and b2g
> doesn't have any tests defined. As soon as either of those changes, it'll be
> a mess and we'll have to do something.

Ah, yes. This does sound OK for now.

> The try chooser gets a buildbotBranch parameter that, assuming it is set
> correctly, could be used to distinguish projects. (Is the name
> "buildbotBranch" meaningful? I have no idea what it's supposed to mean. If
> we end up using it for this, I'd be inclined to change it to "product" or
> something.) Then any of these options would be straightforward to implement:
>   try-b2g: ...usual syntax...
>   try(b2g): ...usual syntax...
>   try: --product b2g ...usual syntax...
>   try: b2g(...usual syntax...) firefox(...usual syntax...)
> 
> and semicolon-separated combinations of any of the above (well, the try*
> ones wouldn't require semicolons.) 

I would _assume_ buildbot branch would be "mozilla-central" or anything else listed in https://github.com/mozilla/build-buildbot-configs/blob/master/mozilla/b2g_config.py#L318. (Or in the config.py equivalent.) It's possible that we could change these without breaking anything, but I wouldn't bet on it. Additionally, you can have multiple branches for one project (eg, mozilla-central, aurora, beta, and release are all part of the conceptual "Firefox/Fennec" project).

> Though do we even need to support
> multiple products in one push?

Depends who you ask. It's not a blocker in my mind, but I certainly think it would become an annoyance.

> Or there's
> 
>   try: -p b2g[ics_armv7a_gecko],firefox[linux,macosx] ...
> 
> if you think of -p as either 'platform' *or* 'product'. (I'm thinking no
> product defaults to either firefox or any product.) Not sure if I'm kidding
> with this one or not.

While this is clever, I will hazard to guess that it will become confusing for users and maintainers :).
(In reply to Ben Hearsum [:bhearsum] from comment #16)
> (In reply to Steve Fink [:sfink] from comment #15)
> > The try chooser gets a buildbotBranch parameter that, assuming it is set
> > correctly, could be used to distinguish projects. (Is the name
> > "buildbotBranch" meaningful? I have no idea what it's supposed to mean. If
> > we end up using it for this, I'd be inclined to change it to "product" or
> > something.) Then any of these options would be straightforward to implement:
> >   try-b2g: ...usual syntax...
> >   try(b2g): ...usual syntax...
> >   try: --product b2g ...usual syntax...
> >   try: b2g(...usual syntax...) firefox(...usual syntax...)
> > 
> > and semicolon-separated combinations of any of the above (well, the try*
> > ones wouldn't require semicolons.) 
> 
> I would _assume_ buildbot branch would be "mozilla-central" or anything else
> listed in
> https://github.com/mozilla/build-buildbot-configs/blob/master/mozilla/
> b2g_config.py#L318. (Or in the config.py equivalent.) It's possible that we
> could change these without breaking anything, but I wouldn't bet on it.
> Additionally, you can have multiple branches for one project (eg,
> mozilla-central, aurora, beta, and release are all part of the conceptual
> "Firefox/Fennec" project).

Oh, sorry, you're right. I was looking at builder['name'] for some reason, which is things like 'try' and 'b2g_try'. In the relevant cases here, buildbotBranch will always be 'try', so it's useless.

So I'd be inclined to pass an additional parameter through, grabbing the value from BRANCHES[branch]['product_name']. That seems to be set to either 'b2g' or 'linux'. (Other values used: 'sunbird', 'thunderbird', 'fennec', 'seamonkey'. All of which sound pretty good to me.)

> > Though do we even need to support
> > multiple products in one push?
> 
> Depends who you ask. It's not a blocker in my mind, but I certainly think it
> would become an annoyance.

Yeah, I guess lots of core code gets pulled into both firefox and b2g, and pushers really ought to have an easy way to build everything affected.

Is b2g planning on growing any tests soon?

> > Or there's
> > 
> >   try: -p b2g[ics_armv7a_gecko],firefox[linux,macosx] ...
> > 
> > if you think of -p as either 'platform' *or* 'product'. (I'm thinking no
> > product defaults to either firefox or any product.) Not sure if I'm kidding
> > with this one or not.
> 
> While this is clever, I will hazard to guess that it will become confusing
> for users and maintainers :).

Yeah, it's a little silly. Having completely separate sections (semicolon-separated?) does seem more straightforward.
(In reply to Steve Fink [:sfink] from comment #17)
> > I would _assume_ buildbot branch would be "mozilla-central" or anything else
> > listed in
> > https://github.com/mozilla/build-buildbot-configs/blob/master/mozilla/
> > b2g_config.py#L318. (Or in the config.py equivalent.) It's possible that we
> > could change these without breaking anything, but I wouldn't bet on it.
> > Additionally, you can have multiple branches for one project (eg,
> > mozilla-central, aurora, beta, and release are all part of the conceptual
> > "Firefox/Fennec" project).
> 
> Oh, sorry, you're right. I was looking at builder['name'] for some reason,
> which is things like 'try' and 'b2g_try'. In the relevant cases here,
> buildbotBranch will always be 'try', so it's useless.
> 
> So I'd be inclined to pass an additional parameter through, grabbing the
> value from BRANCHES[branch]['product_name']. That seems to be set to either
> 'b2g' or 'linux'. (Other values used: 'sunbird', 'thunderbird', 'fennec',
> 'seamonkey'. All of which sound pretty good to me.)

I think this can work for now.....we may change the way we define what a "project" is when we redesign these configs, but that shouldn't stop you from making this work now.

> Is b2g planning on growing any tests soon?

Yup. We just turned some on in bug 800025, in fact!
Whoops, as bhearsum noticed, I meant "...set to either 'b2g' or 'firefox'...", not "...'b2g' or 'linux'..."

(In reply to Ben Hearsum [:bhearsum] from comment #18)
> I think this can work for now.....we may change the way we define what a
> "project" is when we redesign these configs, but that shouldn't stop you
> from making this work now.

Ooh, someone's redesigning these configs? They're pretty snarled up right now. Hopefully the redesign is for less snarl... ;-)

I'm not that fond of the current set of parameters passed into tryChooser, btw. TryChooser ends up inferring whether we're talking about builds vs tests by the data structure in prettyNames and the presence or absence of unittestPrettyNames. I see why it was done that way, but it'd be a nice thing to clean up too. Preferably eliminating the builder double-naming at the same time.
Oh, and I meant to mention here where nobody will see it: I've also added a 'b2g' checkbox to https://bitbucket.org/sfink/trychooser/ that will do the -p ics_armv7a_gecko option. Now I just need to evangelize that tool.
Thanks a ton Steve!
WFM I think?
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
I still think we should redesign the syntax to more clearly support different "projects", but this bug is fixed AFAICT.
Product: mozilla.org → Release Engineering
Component: Tools → General
You need to log in before you can comment on or make changes to this bug.