Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up khan-linter to be python3-only. #42

Merged
merged 2 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
deps vendor_deps: go_deps check_setup
rm -r vendor/py2/* || true
rm -r vendor/py3/* || true
pip2 install --target=vendor/py2 -r requirements.txt
pip3 install --target=vendor/py3 -r requirements.txt
[ -d vendor/py2/backports ] && ! [ -s vendor/py2/backports/__init__.py ] && echo "from pkgutil import extend_path; __path__ = extend_path(__path__, __name__)" > vendor/py2/backports/__init__.py
npm install
npm update
npm prune
Expand Down
19 changes: 9 additions & 10 deletions githook.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
#!/usr/bin/env python3

"""Hooks for git that perform linting.

This file contains two git hooks:
Expand Down Expand Up @@ -37,8 +38,6 @@
import subprocess
import sys

import six

import hook_lib


Expand All @@ -60,7 +59,7 @@ def commit_msg_hook(commit_message_file):

# If the commit message is empty or unchanged from the template, abort.
if not commit_message:
six.print_("Aborting commit, empty commit message")
print("Aborting commit, empty commit message")
return 1

try:
Expand All @@ -70,7 +69,7 @@ def commit_msg_hook(commit_message_file):
pass
else:
if commit_message == _normalized_commit_message(template):
six.print_("Aborting commit, commit message unchanged")
print("Aborting commit, commit message unchanged")
return 1

# If we're a merge, don't try to do a lint-check.
Expand Down Expand Up @@ -181,21 +180,21 @@ def pre_push_hook(_unused_arg_remote_name, _unused_arg_remote_location):
# Lint the files, if any. If there are any errors, print a helpful
# message, and return a nonzero status code to abort the push.
if files_to_lint:
six.print_("khan-linter: linting {} files with unpushed "
"changes...".format(len(files_to_lint)))
print("khan-linter: linting {} files with unpushed "
"changes...".format(len(files_to_lint)))
num_errors = hook_lib.lint_files(files_to_lint)
if num_errors > 0:
six.print_(
print(
'\n--- %s lint errors. Push aborted. ---' % num_errors,
file=sys.stderr)
six.print_(
print(
'Running `make fixc` may help to autofix the errors.',
file=sys.stderr)
return 1

# Yay, everything went okay! Return a zero status code, in order to
# allow the push.
six.print_('khan-linter: all lint checks passed', file=sys.stderr)
print('khan-linter: all lint checks passed', file=sys.stderr)
return 0


Expand Down
29 changes: 12 additions & 17 deletions hook_lib.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! Presumably some refactor removed main and forgot to clean this up. Did you need to chmod? Im assuming it had the binary bit given it had a shebang.


"""Library for code shared by hghook.py and githook.py.

This library does the actual linting, after the VCS-specific commands
Expand All @@ -10,8 +8,6 @@
import subprocess
import sys

import six


def lint_files(files_to_lint):
"""Given a list of filenames in the commit, lint them all.
Expand Down Expand Up @@ -49,18 +45,17 @@ def lint_commit_message(commit_message):
num_errors = 0

if not re.search('^(test plan|review):', commit_message, re.I | re.M):
six.print_('Missing "Test plan:" or "Review:" section '
'in the commit message.', file=sys.stderr)
print('Missing "Test plan:" or "Review:" section '
'in the commit message.', file=sys.stderr)
num_errors += 1

elif re.search('^ <see below>$', commit_message, re.M):
six.print_('Must enter a "Test plan:" (or "Review:") '
'in the commit message.', file=sys.stderr)
print('Must enter a "Test plan:" (or "Review:") '
'in the commit message.', file=sys.stderr)
num_errors += 1

if re.search('^<one-line summary, followed by ', commit_message, re.M):
six.print_('Must enter a summary in the commit message.',
file=sys.stderr)
print('Must enter a summary in the commit message.', file=sys.stderr)
num_errors += 1

# TODO(csilvers): verify the first-line summary is actually 1 line long?
Expand All @@ -77,12 +72,12 @@ def report_errors_and_exit(num_errors, commit_message, save_filename):
# save the commit message so we don't need to retype it
with open(save_filename, 'w') as f:
f.write(commit_message)
six.print_('\n--- %s commit message errors ---\n'
'Commit message saved to %s'
% (num_errors, save_filename),
file=sys.stderr)
six.print_('Use "git commit -a --template .git/commit.save" to commit'
' with a fixed message.', file=sys.stderr)
print('\n--- %s commit message errors ---\n'
'Commit message saved to %s'
% (num_errors, save_filename),
file=sys.stderr)
print('Use "git commit -a --template .git/commit.save" to commit'
' with a fixed message.', file=sys.stderr)
sys.exit(1)
six.print_('khan-linter: commit message passed', file=sys.stderr)
print('khan-linter: commit message passed', file=sys.stderr)
sys.exit(0)
39 changes: 15 additions & 24 deletions lint_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,16 @@
import re
import sys

import six


if six.PY2:
def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if isinstance(text, unicode):
text = text.encode('utf-8')
six.print_(text, end=end)
else:
def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if end is None:
end = os.linesep
if isinstance(text, str):
text = (text + end).encode('utf-8')
else: # already bytes
text += end.encode('utf-8')
sys.stdout.buffer.write(text)

def print_(text, end=None):
"""Always emit text as utf-8. Useful when piping output elsewhere."""
if end is None:
end = os.linesep
if isinstance(text, str):
text = (text + end).encode('utf-8')
else: # already bytes
text += end.encode('utf-8')
sys.stdout.buffer.write(text)


def get_real_cwd():
Expand Down Expand Up @@ -56,13 +47,13 @@ def add_arc_fix_str(lintline, bad_line, to_remove, to_add,
# that they can interoperate, and ensures that we use byte indexes instead
# of Unicode character indexes (because the autopatcher expects the
# former).
if isinstance(lintline, unicode):
if isinstance(lintline, str):
lintline = lintline.encode('utf-8')
if isinstance(bad_line, unicode):
if isinstance(bad_line, str):
bad_line = bad_line.encode('utf-8')
if isinstance(to_add, unicode):
if isinstance(to_add, str):
to_add = to_add.encode('utf-8')
if isinstance(to_remove, unicode):
if isinstance(to_remove, str):
to_remove = to_remove.encode('utf-8')

(location, errcode, msg) = lintline.split(' ', 2)
Expand All @@ -76,7 +67,7 @@ def add_arc_fix_str(lintline, bad_line, to_remove, to_add,
if not to_remove:
return lintline + "\0\0%s\0" % to_add

if not isinstance(to_remove, basestring):
if not isinstance(to_remove, str):
# `to_remove` is a regexp.
if search_backwards:
# Match the last instance of `to_remove` before col.
Expand Down
15 changes: 4 additions & 11 deletions linters.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
"""Linters process files or lists of files for correctness."""

import io
import itertools
import json
import logging
Expand All @@ -11,20 +12,12 @@
import sys

import lint_util
import six
import six.moves

from six.moves import cStringIO as StringIO

# Add vendor path so we can find (our packaged versions of) flake8
_CWD = lint_util.get_real_cwd()
_parent_dir = os.path.abspath(_CWD)
if six.PY2:
_vendor_version = 'py2'
else:
_vendor_version = 'py3'

_vendor_dir = os.path.join(_parent_dir, 'vendor', _vendor_version)
_vendor_dir = os.path.join(_parent_dir, 'vendor', 'py3')
sys.path.insert(0, _vendor_dir)

import static_content_refs
Expand Down Expand Up @@ -170,7 +163,7 @@ def _capture_stdout_of(fn, *args, **kwargs):
"""Call fn(*args, **kwargs) and return (fn_retval, fn_stdout_output_fp)."""
try:
orig_stdout = sys.stdout
sys.stdout = StringIO()
sys.stdout = io.StringIO()
retval = fn(*args, **kwargs)
sys.stdout.seek(0) # so new read()/readlines() calls will return
return (retval, sys.stdout)
Expand Down Expand Up @@ -675,7 +668,7 @@ def lint_files(self, files):
stdout_lines = []
# We need to keep sum(|files|) less than about 250k for OS X's
# commandline limit. 2000 files at a time should do that.
for i in six.moves.range(0, len(files), 2000):
for i in range(0, len(files), 2000):
stdout_lines.extend(self._run_eslint(files[i:i + 2000]))

# eslint_reporter specifies that errors are reported on
Expand Down
24 changes: 0 additions & 24 deletions python_version_compat_test.py

This file was deleted.

5 changes: 2 additions & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
# to the /vendor/* directory in with your change as well.

# Required by python lint checking
# This is flake8 3.5.0 with one more commit to support `paths` in `.flake8`.
git+git://github.com/pycqa/flake8.git@dd1e9d1cb7e9a232946c06aca1564d48d4d6f65e
flake8==3.9.2

# Required for yaml lint checking
pyyaml==3.12
pyyaml==6.0.1


15 changes: 6 additions & 9 deletions runlint.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3

"""Run some linters on files of various types."""

Expand Down Expand Up @@ -43,9 +43,6 @@

import linters
import lint_util
import six

from six.moves import xrange


_DEFAULT_BLACKLIST_PATTERN = '<ancestor>/lint_blacklist.txt'
Expand Down Expand Up @@ -158,7 +155,7 @@ def _extended_fnmatch_compile(pattern):

# If the code below this line has horrible syntax highlighting, check
# this out: http://stackoverflow.com/questions/13210816/sublime-texts-syntax-highlighting-of-regexes-in-python-leaks-into-surrounding-c
_METACHAR_RE = re.compile(r'[[*?!]')
_METACHAR_RE = re.compile(r'[*?![]')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to tell which way is more readable.



def _parse_one_blacklist_line(line):
Expand Down Expand Up @@ -320,7 +317,7 @@ def _file_in_blacklist(fname, blacklist_pattern):
# The blacklist can have regexp patterns in it, so we need to
# check those too, one by one:
for blacklist_entry in blacklist:
if not isinstance(blacklist_entry, six.string_types):
if not isinstance(blacklist_entry, str):
if blacklist_entry.match(fname):
return True

Expand All @@ -335,7 +332,7 @@ def _files_under_directory(rootdir, blacklist_pattern):
# backwards so we can use del. (Weird os.walk() semantics:
# calling del on an element of dirs suppresses os.walk()'s
# traversal into that dir.)
for i in xrange(len(dirs) - 1, -1, -1):
for i in range(len(dirs) - 1, -1, -1):
absdir = os.path.join(root, dirs[i])
if os.path.islink(absdir):
_get_logger().debug('... skipping directory %s: is a symlink'
Expand Down Expand Up @@ -458,7 +455,7 @@ def _run_extra_linter(extra_linter_filename, files, verbose):
if linter:
linter_to_files.setdefault(linter, set()).add(f)

for (linter_filename, files) in six.iteritems(linter_to_files):
for (linter_filename, files) in linter_to_files.items():
if not os.access(linter_filename, os.R_OK | os.X_OK):
continue
files = sorted(files)
Expand Down Expand Up @@ -822,7 +819,7 @@ def main(files_and_directories,
except Exception:
_get_logger().error(u"ERROR linting %r with %s:\n%s" % (
files, type(lint_processor),
traceback.format_exc().decode('utf-8')))
traceback.format_exc()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cant keep track of what needs .decode('utf-8') anymore. 😅

num_framework_errors += 1
continue

Expand Down
2 changes: 1 addition & 1 deletion runlint_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3

import os
import unittest
Expand Down
Loading