[webkit-reviews] review granted: [Bug 76356] webkitpy: clean up chromium port code in preparation for static port names : [Attachment 122585] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 76356: webkitpy: clean up chromium port code in preparation for static port
names
https://bugs.webkit.org/show_bug.cgi?id=76356

Attachment 122585: Patch
https://bugs.webkit.org/attachment.cgi?id=122585&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=122585&action=review


Some of this code is pretty gross, but I see that it was here already and that
you're just moving it around.

> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:97
> +    @classmethod
> +    @memoized
> +    def _chromium_base_dir(cls, filesystem):
> +	   module_path = filesystem.path_to_module(cls.__module__)
> +	   offset = module_path.find('third_party')
> +	   if offset == -1:
> +	       return filesystem.join(module_path[0:module_path.find('Tools')],
'Source', 'WebKit', 'chromium')
> +	   else:
> +	       return module_path[0:offset]

I see.	That's an interesting way of detecting this condition, but I guess
that's what we've been doing for a whioe.

> Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py:78
> +    @staticmethod
> +    def _static_build_path(filesystem, build_directory, chromium_base,
webkit_base, *comps):
> +	   if build_directory:
> +	       return filesystem.join(build_directory, *comps)
> +	   if filesystem.exists(filesystem.join(chromium_base, 'sconsbuild')):
> +	       return filesystem.join(chromium_base, 'sconsbuild', *comps)
> +	   if filesystem.exists(filesystem.join(chromium_base, 'out', *comps)):

> +	       return filesystem.join(chromium_base, 'out', *comps)
> +	   if filesystem.exists(filesystem.join(webkit_base, 'sconsbuild')):
> +	       return filesystem.join(webkit_base, 'sconsbuild', *comps)
> +	   return filesystem.join(webkit_base, 'out', *comps)

This is kind of gross.	What if we add a new output directory?