Sheriff policy is to only make builds visible if they run on all trees feeding into mozilla-central. Spidermonkey builds, especially the ggc (generational garbage collector) build, want to be visible (because they keep breaking, and we need them green in order to be able to turn on GGC.) Currently, it appears that it is assumed by default that all branches feed into mozilla-central unless specifically excluded. But spidermonkey builds need a configuration wad per branch, and it seems odd to me that a config file implicitly assumes that sort of thing, when exceptions exist.

This patch allows you to mark branches with tags, so that you can select sets of branches by tag. It only uses the tags for spidermonkey builds, for now. I've seeded it with two possible tags: 'mozilla-central', meaning something that feeds into mozilla-central, and 'inbound', for builds that you only want on a limited number of trees that you're checking manually. ('inbound' is intended for hidden builders, to match the current setup of spidermonkey warnings-as-errors and dtrace builds.) It should perhaps have a more specific name, like 'tress-that-spidermonkey-peers-watch'. I'm not sure about the 'select_tags' key name. Perhaps 'branch_tags' or 'select_branch_tags' or something?

And here's the implementation of the tag-matching stuff for spidermonkey objects. Note that I had to change how spidermonkey builders get their branch config info.

(In reply to Steve Fink [:sfink] from comment #0) > Currently, it appears that it is assumed by default that all branches feed > into mozilla-central unless specifically excluded. Sortof. When talking about Firefox, branches in project_branches.py inherit most things from mozilla-central. Branches only defined config.py don't inherit from anything. > But spidermonkey builds > need a configuration wad per branch, and it seems odd to me that a config > file implicitly assumes that sort of thing, when exceptions exist. I don't think Spidermonkey is unique in this sense. I think you're hitting the upper limits of the original structure of the "projects" support. I don't really like the idea of the current patch here...it's one of those things that makes it even harder to reason about our configs. Moving this logic to buildbotcustom also means that running "config.py" to spit out configuration is incomplete, because buildbotcustom deals with applying the tags. It seems like a simpler way of accomplishing this while still reducing duplication is to do something similar to what project_branches.py does for Firefox. Eg, define mozilla-central as your root, define the branches that are based on it, override where necessary. As a tangent, we want to move scheduling into the tree in the medium term future. This would make this problem moot, as changes to which builders are run will ride trains and come along in merges.

(In reply to Ben Hearsum [:bhearsum] from comment #3) > (In reply to Steve Fink [:sfink] from comment #0) > > Currently, it appears that it is assumed by default that all branches feed > > into mozilla-central unless specifically excluded. > > Sortof. When talking about Firefox, branches in project_branches.py inherit > most things from mozilla-central. Branches only defined config.py don't > inherit from anything. > > > But spidermonkey builds > > need a configuration wad per branch, and it seems odd to me that a config > > file implicitly assumes that sort of thing, when exceptions exist. > > I don't think Spidermonkey is unique in this sense. I think you're hitting > the upper limits of the original structure of the "projects" support. > > I don't really like the idea of the current patch here...it's one of those > things that makes it even harder to reason about our configs. Moving this > logic to buildbotcustom also means that running "config.py" to spit out > configuration is incomplete, because buildbotcustom deals with applying the > tags. Does it? The logic in buildbotcustom is create PROJECTS config-wads, not full BRANCHES values. Do people typically compare the PROJECTS data from running config.py? (It would still all be there, just implicit in the tags.) Either way, I guess it's a good property to have as much of the config data as possible laid out in a simple and flat manner after config.py runs. I've fiddled with this a bit more since posting these patches. I agree with the problems of putting this logic into buildbotcustom, so my thought was to do this branch expansion in config.py instead and use the same mechanism for non-spidermonkey config changes. At an abstract level, a 'tag' mechanism makes way more sense to me than explicitly enumerating all of the config changes for each branch -- that's just taking a meaningful concept like "all branches feeding into mozilla-central should have config option X set to False" and expanding it out into a flat set of config settings directly in the code (well, configuration), making you either infer or overcomment what the original reason for each setting was. Much harder to reason about and update. But when I went to implement it, I found that the only things that really match this pattern are the config changes that ride the release trains. None of the other cross-branch config changes really correspond to a meaningful tag name (tagging them with "turn-off-option-X" defeats the point.) And the release-riding ones really want to numerically compare gecko versions, not just check for tags, so you need a different mechanism anyway. I have a patch that does that part, btw, and I think it's in improvement on the mass of MERGE DAY branch lists. At the moment, though, it is applied on top of the patches in this bug, so I'll need to disentangle it. Oh, and it *does* depend on tagging stuff in project_branches.py as 'feeds-mozilla-central' (but the tag is applied in config.py, and if this is the only use of tagging, it shouldn't be called a tag. { inherit_gecko_version: 'mozilla-central' }, perhaps.) Perhaps there is other configuration that is more taggable and is currently implicit; I don't know enough to tease it out. I do know that when I read lines like BRANCHES['mozilla-aurora']['enable_mac_a11y'] = True I wonder "why? what does that mean? What is it about the mozilla-aurora branch that wants that particular config setting? What other branches want that setting and what do they have in common? If I'm making a new project branch, how do I decide whether I want it?" With a descriptive tag name like "used-by-hyperventilating-frogs", you immediately know "anything used by hyperventilating frogs needs enable_mac_a11y set to True", or even a whole set of interdependent config settings. > It seems like a simpler way of accomplishing this while still reducing > duplication is to do something similar to what project_branches.py does for > Firefox. Eg, define mozilla-central as your root, define the branches that > are based on it, override where necessary. What do you mean by "define mozilla-central as your root"? I didn't see anything in config.py or project_branches.py that treats mozilla-central specially or as a basis for other stuff. There's the section commented with # Copy project branches into BRANCHES keys and the one with # Copy global vars in first, then platform vars but neither of those seems to copy from BRANCHES['mozilla-central'] anywhere. They copy from GLOBAL_VARS and PLATFORM_VARS, which I'm all for. Then there's a big long section starting with ######## generic branch configs that creates a BRANCHES entry from each ACTIVE_PROJECT_BRANCHES -- that must be what you're referring to? It's really the defaults in that section that everything inherits from. I'd be happier if those defaults came from BRANCHES['mozilla-central'], or even BRANCH_DEFAULTS would be fine. (Hm... come to think of it, isn't that the same as GLOBAL_VARS? Why are the hardcoded defaults needed?) Also, my understand of "branches" in the config (BRANCHES, PROJECT_BRANCHES, etc.) was that they were 1-1 to an hg repo. So creating spidermonkey branches seems wrong; spidermonkey stuff is just builds (and attendant schedulers). So I'm not sure exactly what you mean. Oh, I guess you just mean the PROJECTS config wads? The problem with that approach is that I'd still have to explicitly enumerate one for each project branch, and if you add a new project branch the spidermonkey stuff would get out of date. (Also, with the current setup it seems like I have to repeat these N config keys in {staging,preproduction,production}_config.py, though perhaps that's just me doing something silly.) How about... uh, putting a list of PROJECTS key names into GLOBAL_VARS or the hardcoded defaults section ("generic branch configs")? The buildbotcustom stuff would end up looking nearly the same as in the patches I currently have attached, which would have the same problem as above unless I moved the expansion logic into config.py. (Which totally makes sense.) > As a tangent, we want to move scheduling into the tree in the medium term > future. This would make this problem moot, as changes to which builders are > run will ride trains and come along in merges. Hrm... that would fix this stuff, you're right. Though I'm not sure about the other ramifications. Aren't there trees that only do a subset of builds even though they feed into mozilla-central, because all of the changes are known to be restricted to a subset of the platforms or something? Like, when merging from a b2g-only branch you'd have to keep re-adding all of the non-b2g platform builders? Eg, it looks like birch only runs pgo builds on desktop platforms. Oh well, I won't worry about it for now!

(In reply to Steve Fink [:sfink] from comment #4) > (In reply to Ben Hearsum [:bhearsum] from comment #3) > > (In reply to Steve Fink [:sfink] from comment #0) > > > Currently, it appears that it is assumed by default that all branches feed > > > into mozilla-central unless specifically excluded. > > > > Sortof. When talking about Firefox, branches in project_branches.py inherit > > most things from mozilla-central. Branches only defined config.py don't > > inherit from anything. > > > > > But spidermonkey builds > > > need a configuration wad per branch, and it seems odd to me that a config > > > file implicitly assumes that sort of thing, when exceptions exist. > > > > I don't think Spidermonkey is unique in this sense. I think you're hitting > > the upper limits of the original structure of the "projects" support. > > > > I don't really like the idea of the current patch here...it's one of those > > things that makes it even harder to reason about our configs. Moving this > > logic to buildbotcustom also means that running "config.py" to spit out > > configuration is incomplete, because buildbotcustom deals with applying the > > tags. > > Does it? The logic in buildbotcustom is create PROJECTS config-wads, not > full BRANCHES values. Do people typically compare the PROJECTS data from > running config.py? (It would still all be there, just implicit in the tags.) > Either way, I guess it's a good property to have as much of the config data > as possible laid out in a simple and flat manner after config.py runs. > > I've fiddled with this a bit more since posting these patches. I agree with > the problems of putting this logic into buildbotcustom, so my thought was to > do this branch expansion in config.py instead and use the same mechanism for > non-spidermonkey config changes. At an abstract level, a 'tag' mechanism > makes way more sense to me than explicitly enumerating all of the config > changes for each branch -- that's just taking a meaningful concept like "all > branches feeding into mozilla-central should have config option X set to > False" and expanding it out into a flat set of config settings directly in > the code (well, configuration), making you either infer or overcomment what > the original reason for each setting was. Much harder to reason about and > update. The problem with these sorts of assumptions is that they don't hold for very long. The customizations of project branches in project_branches.py has examples of this. > > But when I went to implement it, I found that the only things that really > match this pattern are the config changes that ride the release trains. None > of the other cross-branch config changes really correspond to a meaningful > tag name (tagging them with "turn-off-option-X" defeats the point.) And the > release-riding ones really want to numerically compare gecko versions, not > just check for tags, so you need a different mechanism anyway. I have a > patch that does that part, btw, and I think it's in improvement on the mass > of MERGE DAY branch lists. And this isn't always true either. Sometimes we have things that hold on mozilla-central (or aurora) indefinitely. Overall, trying to be smarter and deduplicate a lot of this information has made _more_ pain for us, not less. > > It seems like a simpler way of accomplishing this while still reducing > > duplication is to do something similar to what project_branches.py does for > > Firefox. Eg, define mozilla-central as your root, define the branches that > > are based on it, override where necessary. > > What do you mean by "define mozilla-central as your root"? I didn't see > anything in config.py or project_branches.py that treats mozilla-central > specially or as a basis for other stuff. There's the section commented with Sorry - you're right. BRANCHES/PLATFORMS define root stuff, not mozilla-central. My head got all mixed up. > ######## generic branch configs > > that creates a BRANCHES entry from each ACTIVE_PROJECT_BRANCHES -- that must > be what you're referring to? It's really the defaults in that section that > everything inherits from. I'd be happier if those defaults came from > BRANCHES['mozilla-central'], or even BRANCH_DEFAULTS would be fine. (Hm... > come to think of it, isn't that the same as GLOBAL_VARS? Why are the > hardcoded defaults needed?) > > Also, my understand of "branches" in the config (BRANCHES, PROJECT_BRANCHES, > etc.) was that they were 1-1 to an hg repo. So creating spidermonkey > branches seems wrong; spidermonkey stuff is just builds (and attendant > schedulers). So I'm not sure exactly what you mean. This is another example of spidermonkey outgrowing PROJECTS. PROJECTS was originally designed to be out-of-band from Gecko branches and builds. It started off with fuzzing, which gets run during idle time. Having spidermonkey builds live in PROJECTS and be scheduled as part of Gecko pushes is a hack IMO. It seems with Spidermonkey builds getting so branchey, that we might be better off putting them in the existing BRANCHES constructs. The difficult part there is that every "platform" in a branch is assumed to be a Gecko-style build. So you'd to have some new substructure underneath branches for spidermonkey stuff. Frankly, I think we're better off staying flatter and more duplicated until we have in-tree scheduling. > Oh, I guess you just mean the PROJECTS config wads? The problem with that > approach is that I'd still have to explicitly enumerate one for each project > branch, and if you add a new project branch the spidermonkey stuff would get > out of date. (Also, with the current setup it seems like I have to repeat > these N config keys in {staging,preproduction,production}_config.py, though > perhaps that's just me doing something silly.) This stuff would be simpler if Spidermonkey (and maybe other projects) inherited things like tools_repo from GLOBAL_VARS. What a mess. > > How about... uh, putting a list of PROJECTS key names into GLOBAL_VARS or > the hardcoded defaults section ("generic branch configs")? The > buildbotcustom stuff would end up looking nearly the same as in the patches > I currently have attached, which would have the same problem as above unless > I moved the expansion logic into config.py. (Which totally makes sense.) I think this is similar to what I'm suggesting a couple paragraphs up....

(In reply to Ben Hearsum [:bhearsum] from comment #5) > (In reply to Steve Fink [:sfink] from comment #4) > > (In reply to Ben Hearsum [:bhearsum] from comment #3) > > > (In reply to Steve Fink [:sfink] from comment #0) > > > > Currently, it appears that it is assumed by default that all branches feed > > > > into mozilla-central unless specifically excluded. > > > > > > Sortof. When talking about Firefox, branches in project_branches.py inherit > > > most things from mozilla-central. Branches only defined config.py don't > > > inherit from anything. > > > > > > > But spidermonkey builds > > > > need a configuration wad per branch, and it seems odd to me that a config > > > > file implicitly assumes that sort of thing, when exceptions exist. > > > > > > I don't think Spidermonkey is unique in this sense. I think you're hitting > > > the upper limits of the original structure of the "projects" support. > > > > > > I don't really like the idea of the current patch here...it's one of those > > > things that makes it even harder to reason about our configs. Moving this > > > logic to buildbotcustom also means that running "config.py" to spit out > > > configuration is incomplete, because buildbotcustom deals with applying the > > > tags. > > > > Does it? The logic in buildbotcustom is create PROJECTS config-wads, not > > full BRANCHES values. Do people typically compare the PROJECTS data from > > running config.py? (It would still all be there, just implicit in the tags.) > > Either way, I guess it's a good property to have as much of the config data > > as possible laid out in a simple and flat manner after config.py runs. > > > > I've fiddled with this a bit more since posting these patches. I agree with > > the problems of putting this logic into buildbotcustom, so my thought was to > > do this branch expansion in config.py instead and use the same mechanism for > > non-spidermonkey config changes. At an abstract level, a 'tag' mechanism > > makes way more sense to me than explicitly enumerating all of the config > > changes for each branch -- that's just taking a meaningful concept like "all > > branches feeding into mozilla-central should have config option X set to > > False" and expanding it out into a flat set of config settings directly in > > the code (well, configuration), making you either infer or overcomment what > > the original reason for each setting was. Much harder to reason about and > > update. > > The problem with these sorts of assumptions is that they don't hold for very > long. The customizations of project branches in project_branches.py has > examples of this. Well, sure. I'm fine leaving lots of room for exceptions and customization. It just makes me nervous when the same customization settings occur repeatedly. It feels like commonality got lost, which opens up the possibility of inconsistency and makes it harder for newbies like me to figure out what's going on. > > But when I went to implement it, I found that the only things that really > > match this pattern are the config changes that ride the release trains. None > > of the other cross-branch config changes really correspond to a meaningful > > tag name (tagging them with "turn-off-option-X" defeats the point.) And the > > release-riding ones really want to numerically compare gecko versions, not > > just check for tags, so you need a different mechanism anyway. I have a > > patch that does that part, btw, and I think it's in improvement on the mass > > of MERGE DAY branch lists. > > And this isn't always true either. Sometimes we have things that hold on > mozilla-central (or aurora) indefinitely. No reason to disallow explicit enumeration of branches when appropriate. It still seems like it would be more robust to start out optimistic ("these settings are totally dependent on gecko 71") and fall back to explicit expansion when needed. It gives an explicit decision where you can declare something to be following a new rule, or is truly an exception and just needs to be laid out flat. > Overall, trying to be smarter and deduplicate a lot of this information has > made _more_ pain for us, not less. I have to bow to your experience, especially since it sounds like there's some history there. In the absence of evidence, I would claim that problems are often a result of going too far one way or the other (requiring everything to be 100% rule based is at least as bad as expanding everything out to a totally flat JSON file), or of having the wrong data model so that you end up being both too expressive and not expressive enough at the same time. Then again, in the absence of evidence the proper thing to do is to shut up. :-) Being improper, I would say that philosophically I see (1) the mental model of the coder (configure-er) leading to (2) the configuration and logic placed into the actual .py/.cfg files to generate (3) the in-memory data model that feeds (4) the actual builds and actions that are performed. It's very nice to have #3 be as flat and simple as possible -- easier to compare changes, simpler to debug problems. Releng seems to place great value on making #2 and #3 be as similar as possible, for mostly the same reasons. I assert that it's more important to make #1 and #2 be as similar as possible. It makes it easier to make changes that are likely to be correct in all situations, and when you find problems in #4 you can map it back through the layers to figure out what's *really* wrong, and not just find where to tweak a value to fix the immediate symptom. Having an easily-accessible #3 gives you most of the benefits of a structurally simple #2. Admittedly, at least from the outside you guys don't seem to do much whack-a-mole fixing. But I don't know if I should credit that to having the right configuration syntax, or to engineering discipline combined with familiarity with the current setup combined with overdeveloped frontal neocortexes. > > Also, my understand of "branches" in the config (BRANCHES, PROJECT_BRANCHES, > > etc.) was that they were 1-1 to an hg repo. So creating spidermonkey > > branches seems wrong; spidermonkey stuff is just builds (and attendant > > schedulers). So I'm not sure exactly what you mean. > > This is another example of spidermonkey outgrowing PROJECTS. PROJECTS was > originally designed to be out-of-band from Gecko branches and builds. It > started off with fuzzing, which gets run during idle time. Having > spidermonkey builds live in PROJECTS and be scheduled as part of Gecko > pushes is a hack IMO. That's food for thought. In a way, the spidermonkey builds really belong to a sub-view of a gecko tree. They only apply to a portion of the tree, both with respect to when they should be triggered and what gets built. Really, going back to a separate spidermonkey branch that merges in seems more correct from a purist standpoint, but we really like relying on sheriffs' work. Soon, we'll probably have some spidermonkey-related builds that do need to be tied to every Gecko push, as we move the ggc-related API changes into the whole browser. In fact, we want this already for our static rooting analyis. I need some more time to set that one up. And these are different animals. > It seems with Spidermonkey builds getting so branchey, that we might be > better off putting them in the existing BRANCHES constructs. The difficult > part there is that every "platform" in a branch is assumed to be a > Gecko-style build. So you'd to have some new substructure underneath > branches for spidermonkey stuff. Frankly, I think we're better off staying > flatter and more duplicated until we have in-tree scheduling. I don't think I really grasped this the first time I read it. Yes, what you said. 100% agree. I just deleted 2 paragraphs where I painfully came to the same conclusion because I hadn't read carefully enough. > > Oh, I guess you just mean the PROJECTS config wads? The problem with that > > approach is that I'd still have to explicitly enumerate one for each project > > branch, and if you add a new project branch the spidermonkey stuff would get > > out of date. (Also, with the current setup it seems like I have to repeat > > these N config keys in {staging,preproduction,production}_config.py, though > > perhaps that's just me doing something silly.) > > This stuff would be simpler if Spidermonkey (and maybe other projects) > inherited things like tools_repo from GLOBAL_VARS. What a mess. Yeah. Embedding within branches would fix this. > > How about... uh, putting a list of PROJECTS key names into GLOBAL_VARS or > > the hardcoded defaults section ("generic branch configs")? The > > buildbotcustom stuff would end up looking nearly the same as in the patches > > I currently have attached, which would have the same problem as above unless > > I moved the expansion logic into config.py. (Which totally makes sense.) > > I think this is similar to what I'm suggesting a couple paragraphs up.... Yes. Yes, it is. And if I'd actually read it properly, I would have known that already.

Ok, I ended up making all of the spidermonkey builds use a new BRANCH_PROJECTS mechanism and diffed the set of builders generated before and after, at least for master_localconfig.py pointing to {try,build}_localconfig.py, and they are identical. With this final patch, the rootanalysis and generational builds move out of the minimalistic inbound set ({mozilla-inbound,birch,ionmonkey}) and instead switch to a 'spidermonkey_tier_1' set, which is everything feeding into mozilla-central. I separated it out just to make it closer to a pure refactoring when introducing the BRANCH_PROJECTS thing. Some builder names changed slightly, but not in a way that should affect the tbpl regexes.

Comment on attachment 765013 [details] [diff] [review] Simplify spidermonkey build objects by using branch configuration directly and handling multiple variants this one looks ok to me, but I have some questions regarding configs (incoming)

Comment on attachment 765014 [details] [diff] [review] Add a new BRANCH_PROJECTS section for project jobs associated with Gecko branches Review of attachment 765014 [details] [diff] [review]: ----------------------------------------------------------------- It looks great in overall. Thanks a lot for the patch. See some questions inline. ::: mozilla/config.py @@ +1252,5 @@ > +for k, v in localconfig.PROJECTS.items(): > + if k not in PROJECTS: > + PROJECTS[k] = {} > + for k1, v1 in v.items(): > + PROJECTS[k][k1] = v1 Can you add some comments here. I spend some time before I understand that these loops just copy local config. :) @@ +1288,5 @@ > +BRANCH_PROJECTS = { > + # Builds that should trigger backouts if they break. Should be on all trees > + # feeding into mozilla-central. > + 'spidermonkey_tier_1': { > + 'project_name': 'spidermonkey_%(branch)s', Is it important to use %(branch)s here? The pretty builder names look a little bit ugly: "Linux x86-64 oak leak test spidermonkey_oak-generational build" (branch listed twice). @@ +1529,5 @@ > +# Expand out the branch_projects into a full PROJECT object per branch > +for b, branch in BRANCHES.items(): > + for project in branch.get('branch_projects', {}): > + assert project in BRANCH_PROJECTS > + info = BRANCH_PROJECTS[project].copy() You may want to use deepcopy instead.

(In reply to Rail Aliiev [:rail] from comment #11) > Comment on attachment 765014 [details] [diff] [review] > Add a new BRANCH_PROJECTS section for project jobs associated with Gecko > branches > > Review of attachment 765014 [details] [diff] [review]: > ----------------------------------------------------------------- > > It looks great in overall. Thanks a lot for the patch. See some questions > inline. > > ::: mozilla/config.py > @@ +1252,5 @@ > > +for k, v in localconfig.PROJECTS.items(): > > + if k not in PROJECTS: > > + PROJECTS[k] = {} > > + for k1, v1 in v.items(): > > + PROJECTS[k][k1] = v1 > > Can you add some comments here. I spend some time before I understand that > these loops just copy local config. :) Sure. But note that this isn't my code -- the diff screwed it up, but this whole part was pre-existing code that got caught up with my reshuffling of the PROJECTS keys. I cut & paste it for my new BRANCH_PROJECTS stuff. But I had the same problem; I spent some time understanding exactly the same thing. Given that, I definitely agree they should be commented! As long as I'm looking at this code, do you prefer the original implementation, or what I think is the equivalent: for k, v in localconfig.PROJECTS.items(): PROJECTS.setdefault(k, {}).update(v) ? It's denser, but it does map a little more directly to what's happening. Actually, maybe the clearest would be for k, v in localconfig.PROJECTS.items(): if k not in PROJECTS: PROJECTS[k] = {} PROJECTS[k].update(v) > @@ +1288,5 @@ > > +BRANCH_PROJECTS = { > > + # Builds that should trigger backouts if they break. Should be on all trees > > + # feeding into mozilla-central. > > + 'spidermonkey_tier_1': { > > + 'project_name': 'spidermonkey_%(branch)s', > > Is it important to use %(branch)s here? The pretty builder names look a > little bit ugly: > "Linux x86-64 oak leak test spidermonkey_oak-generational build" (branch > listed twice). Well, sorta. I need to generate a unique name for the per-BRANCH PROJECTS table. Then I reuse that name to form a guaranteed-unique builder name, which then redundantly adds in a %(branch)s of its own. Ok, let me see what it looks like to stop overloading it like that... well, it's pretty straightforward. I had to modify the misc.py patch slightly to use the new project_name instead of the PROJECTS key, but that's just: - 'project': project, + 'project': config['project_name'], This results in "Linux x86-64 oak leak test spidermonkey_tier_1-generational build", which lists the branch only once, but includes the full PROJECT_BRANCH key ("spidermonkey_tier_1") in the name. Do you think that's ok? I could make it customizable too, so that it's just "Linux x86-64 oak leak test spidermonkey_tier_1-generational build", but then you'd have to be careful to not end up with duplicate names (by eg requesting the same variant in two different PROJECT_BRANCHes where you use the same name stem.) But that's an automatable check. > @@ +1529,5 @@ > > +# Expand out the branch_projects into a full PROJECT object per branch > > +for b, branch in BRANCHES.items(): > > + for project in branch.get('branch_projects', {}): > > + assert project in BRANCH_PROJECTS > > + info = BRANCH_PROJECTS[project].copy() > > You may want to use deepcopy instead. ok

Comment on attachment 765987 [details] [diff] [review] Add a new BRANCH_PROJECTS section for project jobs associated with Gecko branches Many are probably not you, but... >>> check_call(['pep8', dir, '--diff', '--max-line-length=159', '--show-source'], stdin=f) /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:13:13: E126 continuation line over-indented for hanging indent WebStatus(http_port=master_config['http_port'], allowForce=True)) ^ /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:18:13: E126 continuation line over-indented for hanging indent "tcp:%(ssh_port)i:interface=127.0.0.1" % master_config, ^ /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:62:20: E201 whitespace after '[' ACTIVE_PROJECTS = [ p for p in ACTIVE_PROJECTS if not PROJECTS[p].get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:62:84: E202 whitespace before ']' ACTIVE_PROJECTS = [ p for p in ACTIVE_PROJECTS if not PROJECTS[p].get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:68:27: E201 whitespace after '[' ACTIVE_BRANCH_PROJECTS = [ p for p in ACTIVE_BRANCH_PROJECTS if not BRANCH_PROJECTS[p].get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/build_localconfig.py:68:105: E202 whitespace before ']' ACTIVE_BRANCH_PROJECTS = [ p for p in ACTIVE_BRANCH_PROJECTS if not BRANCH_PROJECTS[p].get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/config.py:89:6: E123 closing bracket does not match indentation of opening bracket's line }, ^ /tmp/tmp.W8808L2Fn4/mozilla/config.py:100:13: E126 continuation line over-indented for hanging indent 'browser', ^ /tmp/tmp.W8808L2Fn4/mozilla/config.py:1253:1: E302 expected 2 blank lines, found 1 def apply_localconfig(config, local): ^ /tmp/tmp.W8808L2Fn4/mozilla/config.py:1319:33: E261 at least two spaces before inline comment 'linux64-debug': {}, # Filled in with branch-specific values below ^ /tmp/tmp.W8808L2Fn4/mozilla/config.py:1522:37: W601 .has_key() is deprecated, use 'in' if BRANCHES[branch]['platforms'].has_key('win64') and branch not in ('try', 'mozilla-central'): ^ /tmp/tmp.W8808L2Fn4/mozilla/scheduler_localconfig.py:11:13: E126 continuation line over-indented for hanging inde nt "tcp:%(ssh_port)i:interface=127.0.0.1" % master_config, ^ /tmp/tmp.W8808L2Fn4/mozilla/scheduler_localconfig.py:21:27: E201 whitespace after '[' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if not v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/scheduler_localconfig.py:21:35: E231 missing whitespace after ',' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if not v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/scheduler_localconfig.py:21:91: E202 whitespace before ']' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if not v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:13:13: E126 continuation line over-indented for hanging indent WebStatus(http_port=master_config['http_port'], allowForce=True)) ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:18:13: E126 continuation line over-indented for hanging indent "tcp:%(ssh_port)i:interface=127.0.0.1" % master_config, ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:28:20: E201 whitespace after '[' ACTIVE_PROJECTS = [ k for k,v in PROJECTS.items() if v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:28:28: E231 missing whitespace after ',' ACTIVE_PROJECTS = [ k for k,v in PROJECTS.items() if v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:28:73: E202 whitespace before ']' ACTIVE_PROJECTS = [ k for k,v in PROJECTS.items() if v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:29:27: E201 whitespace after '[' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:29:35: E231 missing whitespace after ',' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if v.get('enable_try') ] ^ /tmp/tmp.W8808L2Fn4/mozilla/try_localconfig.py:29:87: E202 whitespace before ']' ACTIVE_BRANCH_PROJECTS = [ k for k,v in BRANCH_PROJECTS.items() if v.get('enable_try') ] ^

Comment on attachment 765013 [details] [diff] [review] Simplify spidermonkey build objects by using branch configuration directly and handling multiple variants more pep8 enforcing for this one: /tmp/tmp.LyZbWTWofl/common.py:139:32: E261 at least two spaces before inline comment ('%s' % word) + # Contain the word we're looking ^ /tmp/tmp.LyZbWTWofl/misc.py:2390:22: E261 at least two spaces before inline comment builderNames = {} # Map(variant => builder names) ^ /tmp/tmp.LyZbWTWofl/misc.py:2428:26: E201 whitespace after '{' name_info = { 'base_name': pf['base_name'] % config, ^ /tmp/tmp.LyZbWTWofl/misc.py:2431:43: E202 whitespace before '}' 'branch': branch } ^

I've fixed all the pep8 violations in my local copies.

Comment on attachment 765987 [details] [diff] [review] Add a new BRANCH_PROJECTS section for project jobs associated with Gecko branches lgtm

Comment on attachment 765021 [details] [diff] [review] Run spidermonkey rootanalysis and generational jobs on all trees feeding into mozilla-central The second hunk needa a refresh

http://hg.mozilla.org/build/buildbot-configs/rev/23d7c82e6480 http://hg.mozilla.org/build/buildbot-configs/rev/67d926ddd43e http://hg.mozilla.org/build/buildbotcustom/rev/597ce2a340d6

In production

Backed out because the new jobs are not showing up, and the updated existing jobs are locked forever at pending. http://hg.mozilla.org/build/buildbot-configs/rev/3d794ce72f75 - backed out 67d926ddd43e http://hg.mozilla.org/build/buildbot-configs/rev/9da2b0713338 - backed out 23d7c82e6480 http://hg.mozilla.org/build/buildbotcustom/rev/208373b9848a - backed out 597ce2a340d6

In production

The new jobs were not created because I was not merging the branch_projects from GLOBAL_VARS and project_branches.py, so when I set eg mozilla-inbound to do both the 'spidermonkey_tier_1' and 'spidermonkey_inbound' projects, spidermonkey_inbound won. This patch merges in the project_branches values, and does a minor cleanup by setting PROJECTS' branchconfig value only for branch_projects instead of for all PROJECTS. The latter shouldn't have any effect either way. That said, this only explains the missing new jobs. I tested this on my own master, and it began running all of the jobs I expected. This stuff renames the existing builders, which is somewhat antisocial but shouldn't make the builders pend forever. I'd like to get a twistd.log for the time leading up to 2013-Jun-26 14:39PDT to try to diagnose. (Or reconfig with the darn thing again, and grab a log then. And pray that it magically works this time.)

I'm getting this error, trying to verify configs: ValueError: Cannot shorten "mozilla-b2g18_v1_1_0_hd_linux64-debug_spidermonkey-generational" to maximum length (30). Got to: m-b18I0_hd_l64-d_sm-generational Sounds like you need to update the builbotcustom patch to shorten "generational".

(In reply to Rail Aliiev [:rail] from comment #23) > I'm getting this error, trying to verify configs: > > ValueError: Cannot shorten > "mozilla-b2g18_v1_1_0_hd_linux64-debug_spidermonkey-generational" to maximum > length (30). Got to: m-b18I0_hd_l64-d_sm-generational > > Sounds like you need to update the builbotcustom patch to shorten > "generational". Yeah, sorry. I didn't upload that change. (That's the only thing I changed in buildbotcustom since the posted version.)

https://hg.mozilla.org/build/buildbotcustom/rev/5ad19343465f http://hg.mozilla.org/build/buildbot-configs/rev/26e81670747d http://hg.mozilla.org/build/buildbot-configs/rev/e71457cf2e60

Hopefully you won't get tired of rereviewing this patch anytime soon. My latest problem was, mainly, that I'm an idiot and forgot to rerun test-masters.sh after the latest change. But more specifically, this BRANCH_PROJECTS stuff generates BRANCHES objects that are not subject to ACTIVE_BRANCHES and ACTIVE_PROJECTS. (There is an ACTIVE_BRANCH_PROJECTS, but that's not fine enough granularity here.) So a non-try master was getting try-only branches. I've updated the patch to only expand BRANCH_PROJECTS when the 'enable_try' value of the branch and branch_project match. You could argue that we should never have a mismatched BRANCHES[branch]['project_branches'] value in the first place. For example, bm12 was failing because BRANCHES['try']['project_branches'] was ['spidermonkey_tier_1', 'spidermonkey_try'], which doesn't make much sense because 'spidermonkey_tier_1' isn't intended for use on try (and it does not have enable_try set to True.) It's getting in there because 'spidermonkey_tier_1' is in GLOBAL_VARS['project_branches'], since it's kind of the only place to put it that will get picked up by all mozilla-central-based branches, and I did not add a mechanism for a project_branch to be *removed* from BRANCHES[x]['project_branches']. I could add that, but it doesn't seem to fit in how other similar things are done. For example, 'platforms' have a separate 'lock_platforms' key that you can use for things like this, but having the user enumerate all of the branch_projects would kind of defeat the purpose of having the cartesian product autogenerated (so that they can't be missed.) Right now, the only need for an exclusion list is this try vs non-try thing, and that's easily fixed by the explicit check in this updated patch. If you want, I can do the exclusion logic, or make 'project_branches' values be a tuple of try vs non-try lists of project_branches. Here's the interdiff, inline because it's short: diff --git a/mozilla/config.py b/mozilla/config.py --- a/mozilla/config.py +++ b/mozilla/config.py @@ -1522,16 +1522,19 @@ for branch in BRANCHES.keys(): # Expand out the branch_projects into a full PROJECT object per branch for b, branch in BRANCHES.items(): - for project in branch.get('branch_projects', {}): - info = deepcopy(BRANCH_PROJECTS[project]) - info['project_name'] = project + for name, branch_project in branch.get('branch_projects', {}).items(): + if branch.get('enable_try', False) == branch_project.get('enable_try', False): + continue + + info = deepcopy(branch_project) + info['project_name'] = name info['branch'] = b info['branchconfig'] = branch - branch_project_name = '%s__%s' % (project, b) + branch_project_name = '%s__%s' % (name, b) assert branch_project_name not in PROJECTS, '%s already in PROJECTS' % project_name PROJECTS[branch_project_name] = info ######## mozilla-central

...and back out again (the patch I just attached fixes the failure this is a backout for): http://hg.mozilla.org/build/buildbotcustom/rev/e5eb01c8eb53 - backout http://hg.mozilla.org/build/buildbot-configs/rev/bf338f13bb7b - backout http://hg.mozilla.org/build/buildbot-configs/rev/e83947ada8fb - backout

Can you refresh attachment 765021 [details] [diff] [review], please? It fails to apply.

Rebased buildbotcustom patch

Rebased

Rebased

Argh! I totally broke it with that last update, and my testing didn't show it. I finally figured out why: .pyc files. I've been testing in a single master directory, pushing and popping my patches and repointing the symlinks, and the .pyc files keep giving me old configs. :( Anyway, the fix here is in two parts: first, I needed to compare 'enable_try' values with != instead of == (doh!), and second, BRANCHES[b]['enable_try'] wasn't even set because the BRANCH_PROJECTS handling was too early in config.py. If you compare the set of builders before and after just this patch, you should only see a bunch of renames of the form: -OS X 10.7 cypress spidermonkey_cypress-warnaserr build +OS X 10.7 cypress spidermonkey_inbound-warnaserr build With the next patch applied on top, you should also see a bunch of +Linux x86-64 build-system leak test spidermonkey_tier_1-generational build +Linux x86-64 build-system leak test spidermonkey_tier_1-rootanalysis build for the various branches from project_branches.py that previously did not have those builds, as well as some renames from the previous patch like -Linux x86-64 cypress leak test spidermonkey_inbound-generational build -Linux x86-64 cypress leak test spidermonkey_inbound-rootanalysis build +Linux x86-64 cypress leak test spidermonkey_tier_1-generational build +Linux x86-64 cypress leak test spidermonkey_tier_1-rootanalysis build

Oh, right. I forgot to mention that I changed the structure of the 'branch_projects' values in that previous patch -- I was using BRANCHES[branch]['branch_projects'] = { 'projname1': {}, 'projname2': {} } and now just have BRANCHES[branch]['branch_projects'] = [ 'projname1', 'projname2' ] because I realized that I'm never using the config information through BRANCHES. That change required updating this spidermonkey_tier_1 addition in this patch.

Comment on attachment 769724 [details] [diff] [review] Simplify spidermonkey build objects by using branch configuration directly and handling multiple variants, builder list diff and config dump diff look good, test-masters.sh passes. \o/

...and another attempt: https://hg.mozilla.org/build/buildbotcustom/rev/86a41c139bad https://hg.mozilla.org/build/buildbot-configs/rev/86285ce4c3dc https://hg.mozilla.org/build/buildbot-configs/rev/d9c1a10729d9

in production

Net result seems to be that now absolutely no spidermonkey builds run on inbound, and between the time they got disappeared and the time the first merge from inbound to central happened, someone landed ggc bustage on inbound, so now we don't have it running where we actually need it, and do have it running busted elsewhere.

This needs backout and reconfig asap.

This may be useful to sfink for debugging. It's the list of actual builders on a build master after two reconfigs (07:19 and 19:09 PDT). Builders like Linux cypress leak test spidermonkey_inbound-warnaserrdebug build look a little odd (but I didn't read all of this bug). We have six pending builds on inbound's 8e1f9400edde: Linux mozilla-inbound leak test spidermonkey_mozilla-inbound-warnaserrdebug build Linux mozilla-inbound spidermonkey_mozilla-inbound-warnaserr build Linux x86-64 mozilla-inbound leak test spidermonkey_mozilla-inbound-generational build Linux x86-64 mozilla-inbound leak test spidermonkey_mozilla-inbound-rootanalysis build Linux x86-64 mozilla-inbound leak test spidermonkey_mozilla-inbound-warnaserrdebug build Linux x86-64 mozilla-inbound spidermonkey_mozilla-inbound-warnaserr build which were scheduled at 07:15 when that revision landed. None of those builders exist any more, but perhaps that's expected if the names changed.

Comment on attachment 769724 [details] [diff] [review] Simplify spidermonkey build objects by using branch configuration directly and handling multiple variants, Backouts: http://hg.mozilla.org/build/buildbot-configs/rev/ee17088d21d1 http://hg.mozilla.org/build/buildbotcustom/rev/df4cd3b0fdf2

Merge and reconfig complete - out of production.

(In reply to Nick Thomas [:nthomas] from comment #41) > Merge and reconfig complete - out of production. Thank you :-)

Updated with latest round of flailing. Renamed spidermonkey_inbound to spidermonkey_info.

This is the diff of dump_master.py on all masters between the previous configuration and the one with just the buildbotcustom and first patch applied ("Add a new BRANCH_PROJECTS section for project jobs associated with Gecko branches"). It mostly shows how I now have new schedulers for each branch_project x branch.

This one is the additional delta on top of the first diff, showing all of the added builds for project_branches.

I am attempting to test this patch now.

Re-uploading the same patch because firefox won't let me cut & paste. My tests all passed, once I fixed up my sendchange lines. One mystery: when I mimicked non-hgpoller builds, I was not able to get them to fire off even before my patch. I haven't dug into that yet. Here's my full test matrix: Good push: C=611624b7ddb4; buildbot sendchange --master localhost:9900 -u sfink@mozilla.com -b integration/mozilla-inbound -r $C -l "http://hg.mozilla.org/integration/mozilla-inbound/rev/$C" --comments "Bug 0: fix stuff" -P firefox js/src/squeezebot.cpp results: bad -> good (builds, but no sm builds -> all builds including sm builds) Not hgpoller: C=611624b7ddb4; buildbot sendchange --master localhost:9900 -u sfink@mozilla.com -b integration/mozilla-inbound -r $C --comments "Bug 0: fix stuff" -P firefox js/src/squeezebot.cpp results: good -> good (no builds -> no builds)???! Good push with no js/src changes: C=611624b7ddb4; buildbot sendchange --master localhost:9900 -u sfink@mozilla.com -b integration/mozilla-inbound -r $C -l "http://hg.mozilla.org/integration/mozilla-inbound/rev/$C" --comments "Bug 0: fix stuff" -P firefox def/nerf/gloof.cpp results: good -> good (builds but no sm -> builds but no sm) Push to wrong branch: C=611624b7ddb4; buildbot sendchange --master localhost:9900 -u sfink@mozilla.com -b mozilla-inbound -r $C -l "http://hg.mozilla.org/integration/mozilla-inbound/rev/$C" --comments "Bug 0: fix stuff" -P firefox js/src/squeezebot.cpp results: bad -> good (all builds -> no builds)

Uh, this is the patch I meant to upload.

Fixup patch landed: https://hg.mozilla.org/build/buildbotcustom/rev/2fa94595d686

in production

Appears to be working. \o/