New issue
Advanced search Search tips

Issue 778870 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

OWNERS checking fails with file move

Project Member Reported by sadrul@chromium.org, Oct 27 2017

Issue description

Example CL: https://chromium-review.googlesource.com/c/chromium/src/+/740341/2

Summary:
 1. services/ui/public/cpp/gpu/OWNERS refers to file://services/ui/gpu/OWNERS
 2. The CL deletes services/ui/gpu/OWNERS
 3. The CL also updates services/ui/public/cpp/gpu/OWNERS to refer to file://components/viz/OWNERS (which does exist).

Pre-submit step fails with the following error:

DEBUG:root:git show 8dcef1a6328360813e6eb5d5614d40cec7e5dffb:services/ui/gpu/OWNERS;  cwd=/b/build/slave/linux/build/src
DEBUG:root:git show 8dcef1a6328360813e6eb5d5614d40cec7e5dffb:services/ui/gpu/interfaces/OWNERS;  cwd=/b/build/slave/linux/build/src
DEBUG:root:git show 8dcef1a6328360813e6eb5d5614d40cec7e5dffb:services/ui/public/cpp/gpu/OWNERS;  cwd=/b/build/slave/linux/build/src
DEBUG:PRESUBMIT:owner: sadrul@chromium.org; approvals given by: danakj@chromium.org
Traceback (most recent call last):
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1633, in <module>
    sys.exit(main())
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1617, in main
    options.dry_run)
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1360, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1270, in ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "<string>", line 2750, in CheckChangeOnCommit
  File "<string>", line 2424, in _CommonChecks
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 1063, in PanProjectChecks
    input_api, output_api, source_file_filter=None))
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 876, in CheckOwners
    reviewers_plus_owner)
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/owners.py", line 175, in files_not_covered_by
    self.load_data_needed_for(files)
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/owners.py", line 216, in load_data_needed_for
    self._read_owners(self.os_path.join(dirpath, 'OWNERS'))
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/owners.py", line 305, in _read_owners
    ' '.join(comment))
  File "/b/build/scripts/slave/.recipe_deps/depot_tools/owners.py", line 357, in _add_entry
    ('%s does not refer to an existing file.' % directive[5:]))
owners.SyntaxErrorInOwnersFile: /b/build/slave/linux/build/src/services/ui/public/cpp/gpu/OWNERS:1 syntax error: //services/ui/gpu/OWNERS does not refer to an existing file.
step returned non-zero exit code: 1

(complete log: https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Fchromium_presubmit%2F575649%2F%2B%2Frecipes%2Fsteps%2Fpresubmit%2F0%2Fstdout )
 

Comment 1 by sadrul@chromium.org, Oct 27 2017

Labels: -Pri-2 Pri-1

Comment 2 by sadrul@chromium.org, Oct 27 2017

Cc: estaab@chromium.org
Not really sure who I should send to for triage. /cc'ing estaab@ maybe you know?
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/195a4bf5c6e8d7237f22dc3cba8b59598f5c1190

commit 195a4bf5c6e8d7237f22dc3cba8b59598f5c1190
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Oct 27 17:00:43 2017

viz: Remove the broken OWNERS file for now.

Will land a separate CL with the updated path, because simply updating
the file with the right path still fails presubmit.

BUG= 719931 ,  778870 

Change-Id: I0f1e44fd1aa90c98b8597a347abe51fa573d5c21
Reviewed-on: https://chromium-review.googlesource.com/740978
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512215}
[delete] https://crrev.com/63d08bd8c480ed251038a0b8626e986e54d80d26/services/ui/public/cpp/gpu/OWNERS

Comment 4 by no...@chromium.org, Oct 27 2017

Cc: aga...@chromium.org
Components: -Infra>Platform>Config Infra>SDK
Does jparent@'s team now owns depot_tools?

Comment 5 by aga...@chromium.org, Oct 27 2017

Owner: jochen@chromium.org
Status: Assigned (was: Untriaged)
Assigning to jochen, because he wrote the code which causes PRESUBMIT to use the old version of a modified OWNERS file (generally a good thing) which leads to this failure (because the old version refers to a file which is now deleted, so maybe it should get the old version of that file too?).
Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 6 2017

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

commit e3991bc4d8b1f24a5acb314ffa97dadc68cfc28f
Author: Jochen Eisinger <jochen@chromium.org>
Date: Mon Nov 06 17:25:28 2017

Also check override files before checking the fs

A CL might delete an OWNERS file which we still need to read for the OWNERS
check.

BUG= 778870 
R=agable@chromium.org

Change-Id: I25636ff36228a1afb3c10edf5c2419773a4d057e
Reviewed-on: https://chromium-review.googlesource.com/754623
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>

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

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a08a42b60ef5f773241be18c69c3165ad0a27887

commit a08a42b60ef5f773241be18c69c3165ad0a27887
Author: depot-tools-roller@chromium.org <depot-tools-roller@chromium.org>
Date: Mon Nov 06 20:33:51 2017

Roll src/third_party/depot_tools/ 92b8b9906..bb915e00a (3 commits; 1 trivial rolls)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/92b8b99069f4..bb915e00ae3c

$ git log 92b8b9906..bb915e00a --date=short --no-merges --format='%ad %ae %s'
2017-11-05 jochen Also check override files before checking the fs
2017-10-26 whesse Dart: Change url of Dart repository in gclient recipe module Dart config.

Created with:
  roll-dep src/third_party/depot_tools
BUG= 778870 


The AutoRoll server is located here: https://depot-tools-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=phajdan.jr@chromium.org

Change-Id: Ic95da29070866451141497eba5f3fdae335ff8c0
Reviewed-on: https://chromium-review.googlesource.com/755335
Reviewed-by: depot-tools-roller . <depot-tools-roller@chromium.org>
Commit-Queue: depot-tools-roller . <depot-tools-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514234}
[modify] https://crrev.com/a08a42b60ef5f773241be18c69c3165ad0a27887/DEPS

Sign in to add a comment