[Webkit-unassigned] [Bug 50098] New webkit-patch rebaseline2 command.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50098





--- Comment #14 from James Kozianski <koz at chromium.org>  2010-12-01 19:51:43 PST ---
(In reply to comment #10)
> > > > > WebKitTools/Scripts/webkitpy/layout_tests/rebaseline/rebaseliner.py:61
> > > > > +    def _make_bucket_tree(self):
> > > > > +        b = bucket.BucketTree()
> > > > > +        b.b(None,
> > > > > +            b.b('mac',
> > > > > +                b.b('mac-snowleopard',
> > > > > +                    b.b('mac-leopard',
> > > > > +                        b.b('mac-tiger'))),
> > > > > +                b.b('win',
> > > > > +                    b.b('chromium',
> > > > > +                        b.b('chromium-mac'),
> 
> It's a little more complicated than this. chromium-mac falls back to chromium, then does the same fallback order as upstream mac (i.e. !win). See http://codesearch.google.com/codesearch/p#OAMlx_jo-ck/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py&l=52&exact_package=chromium.

Yeah, this tree was a bit of a simplification. I was wondering if it was possible / a good idea to change our test fallback order so that it does make a tree? In the likely case that it isn't I could change my code to handle DAGs... I think it should be possible to extend the algorithm I use here.

> > > > > WebKitTools/Scripts/webkitpy/layout_tests/rebaseline/rebaseliner.py:63
> > > > > +        AGGREGATE_BUCKETS = ['chromium', 'chromium-win', 'win', 'mac', 'qt', 'chromium-gpu']
> > > > 
> > > > Why is this magic list here?  We shouldn't have magic constants like this in the code.
> > > 
> > > These are the names of platforms that aren't targeted by any builders because they don't denote real platforms, merely convenient fallback targets. This is significant in deduping: if we have the same expectations in mac-leopard, mac-snowleopard and mac-tiger, but a different result in mac, then we can clobber the result in mac and delete the others. We couldn't do that if mac was a platform in itself, because then it having different expectations would be valid and not a duplication.
> > > 
> > > I've added a clarifying comment to explain the meaning of the list.
> > 
> > Ok, but how does that scale when we add more ports / platforms to the project?  Will someone have to hunt around all of webkitpy to find all the port-specific logic and update it?  It seems better to centralize that knowledge in the port abstraction.
> 
> I agree that this logic should be in shared code, probably in the port abstraction. I like this better than what we have in port right now though. In order to understand each platform's fallback rules, you need to look in a bunch of different files and it's very complicated. Having the data in one file would be great. That said, this could just be the underlying data used by the existing port code. For example, the base port class could have this mapping and the baseline_search_path implementations could just index into the mapping. 
> 
> What we lose here is that each port file completely encapsulates the data for that port, but the increased code clarity is worth it IMO.
> 
> In either case, that seems like it could easily be a separate patch. In the interested of avoiding too much yak shaving, how about you rewrite this patch to use baseline_search_path and leave refactoring this stuff as a FIXME?

I'm not sure it's easy to use baseline_search_path() instead of the current code - if I just use one port's baseline_search_path() then it won't be broad enough to be useful[1], and if combine all the baseline_search_path()s then it doesn't form a tree. I'll investigate making my deduping algorithm work for DAGs, and then I can generate the DAG from the baseline_search_paths(), and then as a next step replace the baseline_search_path() implementations with a single one that walks the DAG to calculate its fallback path.

[1] eg: I'm on snowleopard now, so my baseline_search_path() is [mac-snowleopard, mac] which should never cause deduping because we don't know anything about mac-leopard or the other paths that fallback onto mac.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.