Bad OWNERS file syntax passes and then breaks presubmit |
||||||||||
Issue descriptionr520291 made an OWNERS file change that depot_tools/owners.py does not recognize. Whether depot_tools/owners.py should have recognized or not is certainly up for debate. As is, all the try bots passed, the CL landed, and then presubmit breaks: Traceback (most recent call last): File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1686, in <module> sys.exit(main()) File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1670, in main options.dry_run) File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1412, in DoPresubmitChecks results += executer.ExecPresubmitScript(presubmit_script, filename) File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_support.py", line 1319, in ExecPresubmitScript result = eval(function_name + '(*__args)', context) File "<string>", line 1, in <module> File "<string>", line 2774, in CheckChangeOnCommit File "<string>", line 2453, in _CommonChecks File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 1161, in PanProjectChecks input_api, output_api, source_file_filter=None)) File "/b/build/scripts/slave/.recipe_deps/depot_tools/presubmit_canned_checks.py", line 974, 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 295, in _read_owners lineno, '\n'.join(comment)) File "/b/build/scripts/slave/.recipe_deps/depot_tools/owners.py", line 372, in _add_entry 'or an email address.' % (directive,))) owners.SyntaxErrorInOwnersFile: /b/build/slave/linux/build/src/chrome/app/OWNERS:29 syntax error: "rsesek@chromium.org # For crash keys migration https://crbug.com/598854 ." is not a "set noparent", file include, "*", or an email address. step returned non-zero exit code: 1 So I'm going to adjust chrome/app/OWNERS to unbreak the presubmit bot. I would like it if something would sanity check OWNERS files and prevent future CLs from landing if they break owners.py.
,
Nov 30 2017
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7bb8170a1ff6ba07e46032a5c1e8bbb7305bf0b commit e7bb8170a1ff6ba07e46032a5c1e8bbb7305bf0b Author: Lei Zhang <thestig@chromium.org> Date: Thu Nov 30 01:08:48 2017 Fix OWNERS file syntax error from r520291. TBR=rsesek@chromium.org BUG= 789773 Change-Id: I4888b747adac3a2c1c1f156b9255103b2cac4599 Reviewed-on: https://chromium-review.googlesource.com/798400 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#520362} [modify] https://crrev.com/e7bb8170a1ff6ba07e46032a5c1e8bbb7305bf0b/chrome/app/OWNERS
,
Nov 30 2017
Well, given the documented file format in owners.py, that should've been a syntax error, but this is dumb and we should allow end-of-line comments as well, I think.
,
Dec 6 2017
,
Feb 2 2018
I'm not likely to look at this any time soon.
,
Feb 2 2018
,
Feb 9 2018
There are really two problems here: 1) the OWNERS syntax doesn't support end-of-line comments 2) the presubmit bot uses the old version of OWNERS files to compute ownership (a very good thing), so it doesn't try to parse the new version, so it doesn't realize it is broken. I'm also unlikely to get to this any time soon, but it's good to have this on file.
,
Apr 18 2018
Just tripped over this again: https://chromium-review.googlesource.com/c/chromium/src/+/1016674
,
Apr 18 2018
@ehmaldonaldo - want to take a stab at this? Should be pretty easy.
,
Apr 18 2018
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/5c62ed59d2a3901931d01b27388d7c0037178a8e commit 5c62ed59d2a3901931d01b27388d7c0037178a8e Author: Edward Lesmes <ehmaldonado@chromium.org> Date: Thu Apr 19 20:52:34 2018 owners: Add support for comments at the end of the line. Bug: 789773 Change-Id: Ic6fff7e4eba5e86dc77cf2b8e213228ec394dccf Reviewed-on: https://chromium-review.googlesource.com/1019612 Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> [modify] https://crrev.com/5c62ed59d2a3901931d01b27388d7c0037178a8e/owners.py [modify] https://crrev.com/5c62ed59d2a3901931d01b27388d7c0037178a8e/tests/owners_unittest.py
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/067ef5d8941a59ca176f5ee63bb02d0e3df947c4 commit 067ef5d8941a59ca176f5ee63bb02d0e3df947c4 Author: Edward Lesmes <ehmaldonado@chromium.org> Date: Thu Apr 19 22:03:24 2018 Add support for checking that OWNERS files are correctly formatted. Bug: 789773 Change-Id: I4c6c676a821fad33a34ef6c46468af95cdf6c0ec Reviewed-on: https://chromium-review.googlesource.com/1020073 Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> [modify] https://crrev.com/067ef5d8941a59ca176f5ee63bb02d0e3df947c4/presubmit_canned_checks.py [modify] https://crrev.com/067ef5d8941a59ca176f5ee63bb02d0e3df947c4/tests/presubmit_unittest.py
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6459fb329fa5e59d1bc0bed92b1a3ac4c71ec38c commit 6459fb329fa5e59d1bc0bed92b1a3ac4c71ec38c Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Thu Apr 19 22:50:50 2018 Roll src/third_party/depot_tools/ 161623779..5c62ed59d (2 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/161623779eaf..5c62ed59d2a3 $ git log 161623779..5c62ed59d --date=short --no-merges --format='%ad %ae %s' 2018-04-19 ehmaldonado owners: Add support for comments at the end of the line. 2018-04-19 tandrii [git-cl] make more informative error message. Created with: roll-dep src/third_party/depot_tools BUG= chromium:789773 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=agable@chromium.org Change-Id: I9c55e9864502a0ec1895124e9ed8c4421d73e3f3 Reviewed-on: https://chromium-review.googlesource.com/1020234 Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#552191} [modify] https://crrev.com/6459fb329fa5e59d1bc0bed92b1a3ac4c71ec38c/DEPS
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/cb62e48b5424c4bc36b919cc7ed61484599bbc70 commit cb62e48b5424c4bc36b919cc7ed61484599bbc70 Author: Edward Lesmes <ehmaldonado@chromium.org> Date: Fri Apr 20 01:59:24 2018 Check format of OWNERS files in PRESUBMIT and PanProjectChecks. Bug: 789773 Change-Id: Iaa721c9971d1834dd796a9cd28a28a83b6482e05 Reviewed-on: https://chromium-review.googlesource.com/1019975 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org> [modify] https://crrev.com/cb62e48b5424c4bc36b919cc7ed61484599bbc70/presubmit_canned_checks.py [modify] https://crrev.com/cb62e48b5424c4bc36b919cc7ed61484599bbc70/PRESUBMIT.py
,
Apr 20 2018
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b338af18b682f2a071ba4e7e259fa6908226f486 commit b338af18b682f2a071ba4e7e259fa6908226f486 Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Fri Apr 20 02:48:22 2018 Roll src/third_party/depot_tools/ 5c62ed59d..067ef5d89 (4 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/5c62ed59d2a3..067ef5d8941a $ git log 5c62ed59d..067ef5d89 --date=short --no-merges --format='%ad %ae %s' 2018-04-19 ehmaldonado Add support for checking that OWNERS files are correctly formatted. 2018-04-19 tandrii [auth] cleanup old code.google.com oauth scope usage. 2018-04-19 tandrii [auth] cache LUCI_CONTEXT local_auth parameters. 2018-04-19 tandrii [gerrit_util] learn about and use LUCI_CONTEXT when available. Created with: roll-dep src/third_party/depot_tools BUG= chromium:789773 ,chromium:834536,chromium:834536 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=agable@chromium.org Change-Id: I8680161b25310e23648fe1034c1afa0268116326 Reviewed-on: https://chromium-review.googlesource.com/1019731 Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#552243} [modify] https://crrev.com/b338af18b682f2a071ba4e7e259fa6908226f486/DEPS
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/29bf9f2810e083930364abc1c23d575c73e1bf0b commit 29bf9f2810e083930364abc1c23d575c73e1bf0b Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Fri Apr 20 04:31:00 2018 Roll src/third_party/depot_tools/ 067ef5d89..cb62e48b5 (2 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/067ef5d8941a..cb62e48b5424 $ git log 067ef5d89..cb62e48b5 --date=short --no-merges --format='%ad %ae %s' 2018-04-19 ehmaldonado Check format of OWNERS files in PRESUBMIT and PanProjectChecks. 2018-04-19 tandrii [auth] Fix typo introduced in https://crrev.com/c/1018488/13/auth.py#514 Created with: roll-dep src/third_party/depot_tools BUG= chromium:789773 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=agable@chromium.org Change-Id: If44ddda88be63088f48046e8ef753b130fe9aa54 Reviewed-on: https://chromium-review.googlesource.com/1020723 Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#552261} [modify] https://crrev.com/29bf9f2810e083930364abc1c23d575c73e1bf0b/DEPS |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by thestig@chromium.org
, Nov 30 2017