New issue
Advanced search Search tips

Issue 702283 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 679390



Sign in to add a comment

[WPT Sync] Ignore OWNERS files during both import and export.

Project Member Reported by qyears...@chromium.org, Mar 16 2017

Issue description

Background: in Chromium, we want to keep track of Chromium owners, teams and components for layout test directories, including wpt.

However, wpt has its own set of OWNERS files, where owners are specified as GitHub usernames, and Chromium teams/components are not relevant.

From email thread: sergiyb@:

> IMHO a simpler solution would be to not sync OWNERS files at all and let files in Chromium only contain team/component tags while files in GitHub contain @username entries. To avoid confusion we can add a presubmit check forbidding adding owners to Chromium repo unless it's a team/component tag.

This would mean treating OWNERS files similar to the way that baselines (-expected.txt files) are treated now -- not exportable, and not deleted upon import.

Currently, the importer already doesn't import OWNERS files[1].

So the changes that will need to be made are:

 - Import: do not delete OWNERS files on import[2].
 - Export: do not consider OWNERS files as exportable[3].

 [1] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py#148
 [2] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py#239
 [3] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit.py#86
 
Cc: -serg...@chromium.org
Owner: serg...@chromium.org
Status: Assigned (was: Available)
Blocking: 679390
Labels: -Pri-2 Pri-1
P1 since it's blocking another P1.

Comment 4 by foolip@chromium.org, Mar 20 2017

Not captured in the description is translating a GitHub @username to someone@chromium.org.

For example, I wouldn't mind seeing reviews in external/wpt/fullscreen, where I am in the upstream OWNERS file, and I'm not sure if it's possible to subscribe to components in reviews?
Since we currently don't sync OWNERS anyway, I think adding a sync for individual OWNERS is a separate problem. It is also non-trivial since one would need to keep a mapping somewhere and make sure that it stays up-to-date with actual owners.

Therefore, I think this should not be implemented as part of this bug. If in the future we will decide to add support for individual OWNERS, we can then replace the presubmit check mentioned in the description with the one that will check the mapping before adding owners.

Comment 6 by rbyers@chromium.org, Mar 20 2017

Keeping the OWNERS files completely separate (no mapping between them) for now SGTM.  Note that the upstream files have an entirely different semantic from our notion of "OWNERS" - they're really a WATCHLIST file: https://github.com/w3c/web-platform-tests/blob/master/README.md#test-review.
Status: Started (was: Assigned)
CL: https://codereview.chromium.org/2779053002

Comment 8 by ojan@chromium.org, Mar 28 2017

So what's the resolution here? We have OWNERS files in the chromium repo that only contain teams/bug components and that we don't sync upstream? If so, that SGTM.
Yes, that's the resolution and the above CL will implement this. OWNERS files are NOT synchronized both ways.
CL has landed, but where is this service running? Do I need to deploy changes to it manually or will it update itself?
Cc: qyears...@chromium.org
The service consists of a couple builders running recipes on the chromium.infra.cron master:

 Import: https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller
 Export: https://build.chromium.org/p/chromium.infra.cron/builders/wpt-exporter

Currently, the import only happens once per day, and today is a special day since the csswg-test repo just got merged into web-platform-tests; export script is run continuously.

No need to deploy changes manually; since the recipes run scripts in chromium/src, and so the next time it starts the changes should be in effect :-)

This means we should be able to start adding OWNERS files into subdirectories of LayoutTests/external/wpt/, and neither the importer or exporter should touch  the new files.
Status: Fixed (was: Started)
Thanks for explaining. I've landed hundreds of OWNERS files here: http://crrev.com/2751113002 and looks like they are not exported, e.g. compare https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/LayoutTests/external/wpt/FileAPI/OWNERS and https://github.com/w3c/web-platform-tests/blob/master/FileAPI/OWNERS.
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment