New issue
Advanced search Search tips

Issue 697156 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

presubmit incorrectly rejects CL

Project Member Reported by thakis@chromium.org, Feb 28 2017

Issue description

The CQ doesn't want to land https://codereview.chromium.org/2703633003/ It says: http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/374106

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    third_party/WebKit/Source/BUILD.gn

Missing LGTM from an OWNER for these files:
    third_party/WebKit/Source/BUILD.gn

Yet I lgtm'd that CL, and I'm in WebKit/OWNERS here https://cs.chromium.org/chromium/src/third_party/WebKit/OWNERS?type=cs&q=webkit/owners+package:%5Echromium$&l=6 and WebKit/Source/OWNERS doesn't exist.

It's also suspicious that the message gets printed twice.
 
Owner: dpranke@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Restrict-View-Google -Infra-Troopers
There's a bug in the way we're handling per-file rules and the API_OWNERS file. Working on it now ...

Comment 4 by efoo@google.com, Mar 1 2017

Components: -Infra Infra>CQ

Comment 5 by yutak@chromium.org, Mar 2 2017

FYI, when I tried "git cl owners", it correctly suggested thakis@ as a potential
reviewer. I thought that was funny because it recognized my CL lacked OWNERS
approval but it also recognized thakis@ (who already had given an LGTM) would
cover the approval...

Status: Started (was: Assigned)
Proposed fix uploaded to https://chromium-review.googlesource.com/c/447962/ .
Labels: OS-All
Components: -Infra>CQ Infra>SDK
Labels: Build-OWNERS
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/4dc849f802c527ba3b2ba8fd11c91687794692ba

commit 4dc849f802c527ba3b2ba8fd11c91687794692ba
Author: Dirk Pranke <dpranke@chromium.org>
Date: Tue Mar 07 00:26:06 2017

Fix a bug with handling file:// entries in owners.

Previously, if an OWNERS file included //foo/API_OWNERS, then the
code would get confused and think that it had already read and processed
//foo/OWNERS, transferring the contents of the former to the latter.
This was wrong. This change fixes the attribution and adds tests to make
sure we catch this in the future.

R=thakis@chromium.org
BUG= 697156 

Change-Id: I1f1b846cafac2ad6d792d2dccfce94911e9d15c3
Reviewed-on: https://chromium-review.googlesource.com/447962
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>

[modify] https://crrev.com/4dc849f802c527ba3b2ba8fd11c91687794692ba/owners.py
[modify] https://crrev.com/4dc849f802c527ba3b2ba8fd11c91687794692ba/tests/owners_unittest.py

Status: Fixed (was: Started)

Sign in to add a comment