perf-o-matic should store chromium svn revision

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Right now, the graph server only stores WebKit's revision. However, for chromium port, it'll be also useful to store chromium revision.

This is also a requirement if we were to use the graph server to view results for Chromium perf bots.

[Ryosuke Niwa]

Created attachment 125294 [details]
Adds the feature

[Dirk Pranke]

Comment on attachment 125294 [details]
Adds the feature

looks good to me. adam, eric, or ojan might want to glance at it as well.

[Eric Seidel (no email)]

Comment on attachment 125294 [details]
Adds the feature

View in context: https://bugs.webkit.org/attachment.cgi?id=125294&action=review

> Tools/Scripts/webkitpy/common/checkout/scm/scm.py:188
> +    def svn_revision(self, path):
>          self._subclass_must_implement()

Feels a little odd that this doesn't use any self state.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:906
> +    def repository_paths(self):
> +        """Returns a list of (repository_name, repository_path) tuples of its depending code base.
> +        By default it returns a list that only contains a ('webkit', <webkitRepossitoryPath>) tuple."""
> +        return [('webkit', self.webkit_base())]

Odd that this belongs on base, since multi-checkouts are a chromium-only concept.

[Dirk Pranke]

Comment on attachment 125294 [details]
Adds the feature

View in context: https://bugs.webkit.org/attachment.cgi?id=125294&action=review

>> Tools/Scripts/webkitpy/layout_tests/port/base.py:906
>> +        return [('webkit', self.webkit_base())]
> 
> Odd that this belongs on base, since multi-checkouts are a chromium-only concept.

Seems like returning a list is a generic concept since the caller doesn't know if the port uses one or more checkouts. Maybe I'm not following you?

[Ryosuke Niwa]

Comment on attachment 125294 [details]
Adds the feature

View in context: https://bugs.webkit.org/attachment.cgi?id=125294&action=review

> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:340
> +        repos.append(('chromium', self.path_from_chromium_base()))

Oops, a slight fix needed here. Because Source/WebKit/chromium is still in the WebKit repository, it won't work inside WebKit checkout (i.e. chromium-revision will be same as webkit-revision). I'll use self.path_from_chromium_base('build') instead.

[Ryosuke Niwa]

Committed r106687: <http://trac.webkit.org/changeset/106687>

[Ryosuke Niwa]

Build fix landed in http://trac.webkit.org/changeset/107064.