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

commit f7e1e10db5f949dead75f907934aba0a4dfd8b3d
Author: Amin Hassani <ahassani@google.com>
Date: Thu Jun 21 20:13:30 2018

cpplint: Pull in upstream changes

The changes include:
- root flag now can process non-subdirectories
-- https://github.com/google/styleguide/commit/e7ddd2af622475b0c5e0f587d69fb061d2c10c46

- root flag can be configured with CPPLINT.cfg
-- https://github.com/google/styleguide/commit/2322e4ffaa19f4c55e5679102318ce67454fb7ed

- root setting is relative to the CPPLINT.cfg file
-- https://github.com/google/styleguide/commit/8a87a46cc7f8d6cca5c0203ddba5d81149db7695

- Cleans up header file detection for hpp and hxx

- Adds quite mode

Bug: 852898

Change-Id: Id44bbfadc913cc27192049ab9a222f22d0deab5d
Reviewed-on: https://chromium-review.googlesource.com/1108381
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/f7e1e10db5f949dead75f907934aba0a4dfd8b3d/cpplint.py

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

commit 1321b38de5a9837f5260ec9a76ef519d2385032f
Author: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 22 01:56:35 2018

Roll src/third_party/depot_tools 4099daa97b38..09098853e107 (8 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/4099daa97b38..09098853e107


git log 4099daa97b38..09098853e107 --date=short --no-merges --format='%ad %ae %s'
2018-06-21 vadimsh@chromium.org Demote linux-386 to "best effort support", just like e.g. linux-ppc64.
2018-06-21 recipe-roller@chromium.org Roll recipe dependencies (trivial).
2018-06-21 ehmaldonado@chromium.org gclient: Make gclient respect unmanaged dependencies when syncing.
2018-06-21 recipe-roller@chromium.org Roll recipe dependencies (trivial).
2018-06-21 vadimsh@chromium.org Stop checking CIPD packages exist on linux-386.
2018-06-21 ahassani@google.com cpplint: Pull in upstream changes
2018-06-21 tandrii@chromium.org bot_update: default to non-shallow checkouts.
2018-06-21 iannucci@chromium.org Fix minor regression in git_upstream_diff.


Created with:
  gclient setdep -r src/third_party/depot_tools@09098853e107

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.



BUG=chromium:854300,chromium:853032,chromium:854300,chromium:852898, chromium:855137 
TBR=agable@chromium.org

Change-Id: Id8a265a396d0d7fdbdad58688673e2acb30a3446
Reviewed-on: https://chromium-review.googlesource.com/1111137
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#569505}
[modify] https://crrev.com/1321b38de5a9837f5260ec9a76ef519d2385032f/DEPS

It looks like depot_tools commit [1] breaks V8 presubmit bot: https://ci.chromium.org/p/v8/builders/luci.v8.try/v8_presubmit

There are some cpplint errors in V8 codebase, e.g.
src/builtins/builtins-intl.cc:83: Add #include <memory> for unique_ptr<> [build/include_what_you_use] [4]

Before there errors were silently ignored, after commit [1] presubmit starts complaining without actually showing errors but with total errors found message:
Failed to process /usr/local/google/home/kozyatinskiy/code/v8/src/api.cc
Total errors found: 1

I am not sure where it should be fixed - on V8 side or on depot_tools. Please take a look.

[1] https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/f7e1e10db5f949dead75f907934aba0a4dfd8b3d

I guess that problem is that v8 presubmit.py [1] waiting for done, and cpplint stops emiting done in some cases:
https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1108381/6/cpplint.py#6118

[1] https://cs.chromium.org/chromium/src/v8/tools/presubmit.py?rcl=7a5731ed6129d32905fdef1149410134393c1a30&l=90

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

commit 7999d926809fdb560e58012dddd74b235ea1d99a
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Fri Jun 22 09:25:54 2018

Revert "cpplint: Pull in upstream changes"

This reverts commit f7e1e10db5f949dead75f907934aba0a4dfd8b3d.

Reason for revert: breaks V8 presubmit, e.g. https://ci.chromium.org/p/v8/builders/luci.v8.try/v8_presubmit/b8943030202460843984

Original change's description:
> cpplint: Pull in upstream changes
> 
> The changes include:
> - root flag now can process non-subdirectories
> -- https://github.com/google/styleguide/commit/e7ddd2af622475b0c5e0f587d69fb061d2c10c46
> 
> - root flag can be configured with CPPLINT.cfg
> -- https://github.com/google/styleguide/commit/2322e4ffaa19f4c55e5679102318ce67454fb7ed
> 
> - root setting is relative to the CPPLINT.cfg file
> -- https://github.com/google/styleguide/commit/8a87a46cc7f8d6cca5c0203ddba5d81149db7695
> 
> - Cleans up header file detection for hpp and hxx
> 
> - Adds quite mode
> 
> Bug: 852898
> 
> Change-Id: Id44bbfadc913cc27192049ab9a222f22d0deab5d
> Reviewed-on: https://chromium-review.googlesource.com/1108381
> Commit-Queue: Amin Hassani <ahassani@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>

TBR=agable@chromium.org,ahassani@chromium.org

Change-Id: I3d380ecf8cb80000d7f7dcc0a2c75bb8a12d2a51
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 852898
Reviewed-on: https://chromium-review.googlesource.com/1111797
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>

[modify] https://crrev.com/7999d926809fdb560e58012dddd74b235ea1d99a/cpplint.py

I've reverted the change since its was blocking everyone on V8 team. Will look into what caused it and comment here.

I see several issues here:
 1. New cpplint does not seem to print an actual error, just the number of errors found, e.g. [1] - they need to be printed to be actionable by developers.
 2. V8 had some cpplint failures that were silently ignore before - this will need to be fixed before rolling out new cpplint.
 3. Based on code in [2], it seems v8_presubmit counted "Done processing" lines as errors (although I can't find any cpplint failures to verify that), whereas new cpplint seems to produce "Failed to process" for errors and prints "Done processing" for all files. We'll need to update this code before rolling out new cpplint.
 4. In general, it seems we need to decouple rolling new cpplint into depot_tools from rolling it into V8, e.g. we can use V8's pinned depot_tools [3]. Unfortunately, presubmit recipe module hard-codes usage of the version from depot_tools in [4]. IMHO, this needs to change, e.g. by adding a way to configure run_presubmit.py recipe to use a custom depot_tools location.

Amin, can you please address issues 1 and  4  and then land your change again? We'll then be able to address issues  2  and  3  when our DEPS roller will fail to roll new depot_tools into V8.


[1]: https://logs.chromium.org/v/?s=v8%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8943028999055310736%2F%2B%2Fsteps%2Fpresubmit%2F0%2Fstdout
[2]: https://cs.chromium.org/chromium/src/v8/tools/presubmit.py?l=93&rcl=cd58665a4b60012b93672e557543a27b34972fea
[3]: https://cs.chromium.org/chromium/src/v8/DEPS?l=19&rcl=cd58665a4b60012b93672e557543a27b34972fea
[4]: https://cs.chromium.org/chromium/tools/depot_tools/recipes/recipe_modules/presubmit/api.py?l=10&rcl=7999d926809fdb560e58012dddd74b235ea1d99a

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

commit 2dca1d9ba1ca9b6cd6fe0bff34a12f9369cdfd0b
Author: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 22 10:37:18 2018

Roll src/third_party/depot_tools 120b2e4f2660..7999d926809f (1 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/120b2e4f2660..7999d926809f


git log 120b2e4f2660..7999d926809f --date=short --no-merges --format='%ad %ae %s'
2018-06-22 sergiyb@chromium.org Revert "cpplint: Pull in upstream changes"


Created with:
  gclient setdep -r src/third_party/depot_tools@7999d926809f

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.



BUG=chromium:852898
TBR=agable@chromium.org

Change-Id: I86c1fb4b07615d8f4569fb5271a4cc04bed8e8da
Reviewed-on: https://chromium-review.googlesource.com/1111696
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#569573}
[modify] https://crrev.com/2dca1d9ba1ca9b6cd6fe0bff34a12f9369cdfd0b/DEPS

After closer look at the source code of V8 presubmit at [3], I've realized that it's not counting the number of errors cpplint produced, but rather simply processing cpplint's stdout to extract useful information. In particular it searches for lines that match r'^.+[:(]\d+[:)]|^Done processing', which is designed to preserve lines like these:
 - Done processing /b/swarming/w/ir/cache/builder/v8_presubmit/v8/src/api-arguments.h
 - /b/swarming/w/ir/cache/builder/v8_presubmit/v8/src/api-natives.cc:23:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]

I suspect that the format of these lines in new version of cpplint changed and thus above regexp fails to match them. When no lines match this regexp, the output parser prints "Failed to process <source_file>" and returns 1. Therefore, this line is not produced by cpplint as I originally thought, but by V8's cpplint output parser.

Therefore I think only the issue (4) needs to be resolved before V8 can adjust presubmit scripts to fix (2). Issues (1) and (3) are not there - I misread the code.

 Issue 855614  has been merged into this issue.

I'm really sorry for causing too much pain for V8 team. I had no idea that simple CL could cause these issues. So as I understood, the main problem to be solved before landing it again is  issue 4 . I looked over the issue and its references, but since I don't have much background on these things, I couldn't make up a solution to solve it. Any help and guidance is appreciated. Thanks.

>> I'm really sorry for causing too much pain for V8 team. I had no idea that simple CL could cause these issues.

No worries, changing anything in depot_tools without breaking anyone is hard.

>> So as I understood, the main problem to be solved before landing it again is   issue 4  . I looked over the issue and its references, but since I don't have much background on these things, I couldn't make up a solution to solve it. Any help and guidance is appreciated. Thanks.

I'd be happy to help, but currently handling many other issues. How urgent is this?

It is not urgent, there were just a few changes I wanted to pull in the Chrome OS depot_tools. It is mainly touching anyone uploading a CL in a specific project. They can --no-verify it for now. But we need it to land at some point. 
Thanks.