New issue
Advanced search Search tips

Issue 804891 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

checkstyle.py crashes when trying to commit changes to java files.

Project Member Reported by lilyhoughton@chromium.org, Jan 23 2018

Issue description

Trying to run |git cl upload| after commtting a spurious change to net/android/java/src/org/chromium/new/ProxyChangeListener.java causes a crash with the following stacktrace:

Running presubmit upload checks ...
Error: Could not find or load main class com.puppycrawl.tools.checkstyle.Main
Traceback (most recent call last):
  File "/Users/lilyhoughton/depot_tools/git_cl.py", line 6209, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/Users/lilyhoughton/depot_tools/git_cl.py", line 6191, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/Users/lilyhoughton/depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/Users/lilyhoughton/depot_tools/git_cl.py", line 5043, in CMDupload
    return cl.CMDUpload(options, args, orig_args)
  File "/Users/lilyhoughton/depot_tools/git_cl.py", line 1646, in CMDUpload
    change=change)
  File "/Users/lilyhoughton/depot_tools/git_cl.py", line 1578, in RunHook
    gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit())
  File "/Users/lilyhoughton/depot_tools/presubmit_support.py", line 1421, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "/Users/lilyhoughton/depot_tools/presubmit_support.py", line 1328, in ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "<string>", line 2791, in CheckChangeOnUpload
  File "<string>", line 2524, in _CommonChecks
  File "<string>", line 1530, in _CheckJavaStyle
  File "/Users/lilyhoughton/chromium/src/tools/android/checkstyle/checkstyle.py", line 60, in RunCheckstyle
    formatted_checkstyle_output = FormatCheckstyleOutput(stdout)
  File "/Users/lilyhoughton/chromium/src/tools/android/checkstyle/checkstyle.py", line 21, in FormatCheckstyleOutput
    if 'Checkstyle ends with' in lines[-1]:
IndexError: list index out of range

The change in question is simply adding or changing a comment in the file.  (In this case, s/proxy/proxy_resolution/ on line 253, though I was able to reproduce with other, equally insignificant-seeming changes).
 

Comment 1 by mef@chromium.org, Jan 23 2018

Cc: mef@chromium.org agrieve@chromium.org
I'm getting the same error trying to upload https://chromium-review.googlesource.com/c/chromium/src/+/868076 from iOS.

I suspect that Checkstyle presubmit is triggered by having Java file in the CL, but it fails because Android toolchain is missing on iOS.

I'm not sure why the exception is not caught properly though: https://cs.chromium.org/chromium/src/tools/android/checkstyle/checkstyle.py?rcl=f7f021e668317ba7d5d278425fe45eaea712ee57&l=53


Comment 2 by mef@chromium.org, Jan 23 2018

Cc: jbudorick@chromium.org
Labels: OS-Android
I've created https://chromium-review.googlesource.com/c/chromium/src/+/881662 to only run checkstyle.py on Linux.

WDYT?
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2018

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

commit 18551e8ea2f444ccfce423c49b026b16f265b718
Author: Misha Efimov <mef@chromium.org>
Date: Thu Jan 25 15:55:16 2018

Only run tools/android/checkstyle on Linux.

Bug:  804891 
Change-Id: I1329a27ab71dce901d8494ffeb06aecb61ab5da9
Reviewed-on: https://chromium-review.googlesource.com/881662
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531905}
[modify] https://crrev.com/18551e8ea2f444ccfce423c49b026b16f265b718/tools/android/checkstyle/checkstyle.py

Comment 4 by mef@chromium.org, Jan 25 2018

Owner: mef@chromium.org
Status: Fixed (was: Untriaged)

Sign in to add a comment