Travis CI: Lint for Python syntax errors and undefined names

Several things were changed between Python 2 and Python 3. Here we are making changes to make the code run on both Py2 and Py3. We are doing this because the end of life of Python 2 is in 167 days. We are using print() function because legacy print statements are syntax errors on Py3.reduce() was moved in Python 3 and raw_input() was removed so we make changes to avoid NameErrors at runtime. Signed-off-by: Christian Clauss [email protected] [[email protected]]

cclauss (1):
Travis CI: Lint for Python syntax errors and undefined names

 .travis.yml                        |  4 +++
 contrib/fast-import/import-zips.py |  3 +-
 contrib/hg-to-git/hg-to-git.py     | 47 +++++++++++++++---------------
 git-p4.py                          |  3 ++
 4 files changed, 33 insertions(+), 24 deletions(-)

base-commit: 9d418600f4d10dcbbfb0b5fdbc71d509e03ba719
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-290%2Fcclauss%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-290/cclauss/patch-1-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/290

gitgitgadget

From: cclauss <[email protected]>

Several things were changed between Python 2 and Python 3. There are a few Python 3 incompatibilities to work on.
Here we are making changes to make the code run on both Py2 and Py3. We are doing this because the end of life of Python 2 is in 167 days. We are using print() function because legacy print statements are syntax errors on Py3.
reduce() was moved in Python 3 and raw_input() was removed so we make changes to avoid NameErrors being raised at runtime.
We are also putting flake8 lint tests in place on Travis CI to avoid any backsliding on future pull requests.

---
 .travis.yml                        |  4 +++
 contrib/fast-import/import-zips.py |  3 +-
 contrib/hg-to-git/hg-to-git.py     | 47 +++++++++++++++---------------
 git-p4.py                          |  3 ++
 4 files changed, 33 insertions(+), 24 deletions(-)

index ffb1bc46f2..62557dc8fd 100644
@@ -39,6 +39,10 @@ matrix:
       compiler:
       script: ci/test-documentation.sh
       after_failure:
+    - env: jobname=LintPython
+      language: python
+      before_install: pip install flake8
+      script: flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics
 
 before_install: ci/install-dependencies.sh
 script: ci/run-build-and-tests.sh

index d12c296223..138194475c 100755
@@ -8,6 +8,7 @@
 ##  python import-zips.py *.zip
 ##  git log --stat import-zips
 
+from __future__ import print_function
 from os import popen, path
 from sys import argv, exit, hexversion, stderr
 from time import mktime

@@ -19,7 +20,7 @@
     exit(1)
 
 if len(argv) < 2:
-    print 'usage:', argv[0], '<zipfile>...'
+    print('usage:', argv[0], '<zipfile>...')
     exit(1)
 
 branch_ref = 'refs/heads/import-zips'

index 3991e7d1a7..9faee25db2 100755
@@ -36,6 +36,8 @@
     unicode = str
     bytes = bytes
     basestring = (str,bytes)
+    raw_input = input
+    from functools import reduce
 else:
     # 'unicode' exists, must be Python 2
     str = str

@@ -3968,6 +3970,7 @@ def renameBranch(self, branch_name):
                 break
 
         if not found:
+            sync = P4Sync()
             sys.exit("gave up trying to rename existing branch {0}".format(sync.branch))
 
     def findLastP4Revision(self, starting_point):

gitgitgadget

I'll tweak this line (and your sign-off) to read as "Christian Clauss <[email protected]>" as you had in your cover letter.

All sounds sensible, and the above is quite a good problem description.

Nothing is wrong here, but the convention in our codebase is to describe the changes as if we are giving orders to the codebase "to be like so". And as you have enumeration, I would write something like this if I were describing this change:

- Use the print() function, because Py3 no longer has the print statement.

- Import reduce() from functools, because Py3 requires this, and importing also works with Py2 (even though it wasn't necessary).

- Use input() instead of raw_input(), as the former can be used with both but the latter was removed in Py3.

Also add a CI job to Travis CI to run flake8 lint to avoid an backsliding on future pull requests.

https://python-future.org/compatible_idioms.html#raw-input seems to suggest, just like you import reduce from functools, you need to import input from builtins. Is it not the case?

How was the set of "select"ed violations to catch chosen? How are we going to maintain this list going forward?

The rest of the patch looked sensible.

Thanks.

It seems to me that this patch does too many things at once, and perhaps it would be better to split it into a couple of smaller patches that do only one thing, e.g.:

- use print function instead of statement in 'hg-to-git' (which constitutes the bulk of this patch),
- do the same in 'contrib/fast-import/import-zips.py'
- import 'reduce' and fix 'raw_input' in 'git-p4'
- and once all that is done and the linter runs clean, add the linter job to Travis CI.

This would ease the job of the reader, now and in the future, and it would better stand out that ...