Bug 76356 - webkitpy: clean up chromium port code in preparation for static port names
Summary: webkitpy: clean up chromium port code in preparation for static port names
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks: 76215 76357
  Show dependency treegraph
 
Reported: 2012-01-15 20:00 PST by Dirk Pranke
Modified: 2012-01-17 11:25 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.86 KB, patch)
2012-01-15 20:05 PST, Dirk Pranke
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2012-01-15 20:00:05 PST
webkitpy: clean up port code in preparation for static port names
Comment 1 Dirk Pranke 2012-01-15 20:05:30 PST
Created attachment 122585 [details]
Patch
Comment 2 Adam Barth 2012-01-17 01:58:22 PST
Comment on attachment 122585 [details]
Patch

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?
Comment 3 Dirk Pranke 2012-01-17 11:18:50 PST
(In reply to comment #2)
> (From update of attachment 122585 [details])
> 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.
> 

Yup. I'm open to suggestions :).

> > 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?

We have to add it to the list, unfortunately. I should circle back w/ Tony in a separate patch and see if we can remove some of these things now, but I don't know how to eliminate this completely.
Comment 4 Dirk Pranke 2012-01-17 11:25:13 PST
Committed r105177: <http://trac.webkit.org/changeset/105177>