Python 3 - fixes to get green contrib #6340

Eric-Arellano · 2018-08-13T04:44:30Z

All contrib folders should now be green on Python 2 and Python 3, excluding googlejavaformat and python.

This fixes multiple issues causing Py3 failures + adds Py3 to Travis.

* Shlex expects unicode in Py3 * Use builtins instead of binary_type and text_type

* Make thrift_linter test's mock's arg order be deterministic * Fix hasher unicode issue

Python 3 raises a syntax error upon print statements, so the tests don't work as expected. Instead, bad class variable names works consistently across both Py2 and Py3.

Eric-Arellano

There are still two major failures that I haven't figured out yet.

googlejavaformat

Complains with Exception in thread "main" java.text.ParseException: [[Fatal Error] resolve-ivy.xml:1:1: Content is not allowed in prolog.

Reproduce:

git fetch refs/pull/6340/head:py3-contrib && git checkout py3-contrib
./pants clean-all
./pants --python-setup-interpreter-constraints=["CPython>=3.5"] test contrib/googlejavaformat/tests/python/pants_test/contrib/googlejavaformat:

python

For some reason _compile_target() is not failing. Specifically, line 168 of python_eval.py returns an errorcode of 1 in Py2 but 0 in Py3 (use the debugger at line 168).

Reproduce:

git fetch refs/pull/6340/head:py3-contrib && git checkout py3-contrib
./pants clean-all
./pants --python-setup-interpreter-constraints=["CPython>=3.5"] test contrib/python/tests/python/pants_test/contrib/python/checks/tasks:

Eric-Arellano · 2018-08-13T04:46:11Z

contrib/python/tests/python/pants_test/contrib/python/checks/tasks/checkstyle/test_checker.py

-from pants.contrib.python.checks.tasks.checkstyle.print_statements_subsystem import \
-  PrintStatementsSubsystem
+from pants.contrib.python.checks.tasks.checkstyle.variable_names_subsystem import \
+  VariableNamesSubsystem


The Print Statements tests don't work as expected on Py3. Because they raise a syntax error with Python 3, the tests weren't working as intended.

So, I changed to using Variable Names. The specific error doesn't matter much - this file tests the checker mechanics, and there are other files to check each specific subsystem.

Eric-Arellano · 2018-08-13T04:47:13Z

contrib/python/tests/python/pants_test/contrib/python/checks/tasks/checkstyle/test_checker.py

@@ -121,21 +125,3 @@ def test_syntax_error_nit(self):
                     """     |invalid python\n"""
                     """     |""",
                     str(nits[0]))
-
-  def test_multiline_nit_printed_only_once(self):


I couldn't think of an alternative test - it seems like the original was more related to print logic than checker logic?

Let me know if you have ideas.

Eric-Arellano · 2018-08-13T04:47:34Z

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_thrift_linter.py

@@ -40,15 +40,15 @@ def get_default_jvm_options():
    thrift_target = self.create_library('a', 'java_thrift_library', 'a', ['A.thrift'])
    task = self.create_task(self.context(target_roots=thrift_target))
    self._prepare_mocks(task)
-    expected_include_paths = {'src/thrift/tweet', 'src/thrift/users'}
-    expected_paths = {'src/thrift/tweet/a.thrift', 'src/thrift/tweet/b.thrift'}
+    expected_include_paths = ['src/thrift/users', 'src/thrift/tweet']


Set isn't deterministic between Py2 vs Py3, so have to use a list.

…variables

The original fixes I made over-used bytes and had issues with mixing bytes and strings.

stuhood

Looks good to me! In the interest of landing this, if you want to "skipIf" the failing test, that would be fine for now.

stuhood · 2018-08-16T02:33:08Z

contrib/go/tests/python/pants_test/contrib/go/subsystems/test_go_distribution.py

    self.assertEqual(default_gopath, go_cmd.check_output().decode('utf-8').strip())

+    goroot_env = r'GOROOT=[^ ]+'


Would be good to lift these into constants.

commit f239399 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 22:34:25 2018 -0700 Remove stray pdb import commit 3babfb9 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 22:23:19 2018 -0700 Add back Py3 tests that now work commit a46b827 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 22:22:52 2018 -0700 Fix backend python issues commit 88fa18a Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 20:08:42 2018 -0700 Fix backend project_info issues commit f4d4a2d Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 19:54:39 2018 -0700 Actually fix console separator issues commit d0c83aa Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 19:38:39 2018 -0700 Fix issues with backend native commit 9a1b3b7 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 19:33:48 2018 -0700 Fix most issues with backend jvm commit 62bc2a4 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 14:47:42 2018 -0700 Fix logging issue with passing object to format() commit a4bf759 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 08:05:07 2018 -0700 Decode test's execute_task() output commit b81cd24 Author: Eric Arellano <earellano@foursquare.com> Date: Thu Aug 16 08:03:57 2018 -0700 Fix issues with backend graph_info commit 5d28b9a Author: Eric Arellano <earellano@foursquare.com> Date: Sun Aug 12 23:13:06 2018 -0700 Fix console_task's console_separator unicode issues commit a8674f9 Author: Eric Arellano <earellano@foursquare.com> Date: Sun Aug 12 22:48:42 2018 -0700 Fix backend codegen unicode vs bytes issues commit c893f8c Author: Eric Arellano <earellano@foursquare.com> Date: Sun Aug 12 22:47:04 2018 -0700 Return unicode from stdout in pants integration test commit f4ab07c Author: Benjy Weinberger <benjyw@gmail.com> Date: Thu Aug 16 19:43:41 2018 -0600 Make requirements on codegen products optional. (pantsbuild#6357) We declare product requirements in various places to force codegen to run before, e.g., dependency resolution. However currently these requirements are mandatory, so deregistering all codegen backends will cause the round engine to find no producers of these products, and fail. This commit changes these requirements to optional, so that if there are no codegen backends, we don't attempt to depend on their products. Also, minor unrelated documentation nits that weren't worth a separate pull request. commit b0a2469 Author: Eric Arellano <ericarellano@me.com> Date: Thu Aug 16 12:08:29 2018 -0700 Python 3 - fixes to get green contrib (pantsbuild#6340) All contrib folders should now be green on Python 2 and Python 3, excluding `googlejavaformat` and `python`. This fixes multiple issues causing Py3 failures + adds Py3 to Travis. commit 070a5c5 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Wed Aug 15 19:30:15 2018 -0700 Run clippy with nightly rust on CI (pantsbuild#6347) This enables all default lints, and a selection of pedantic ones. It also fixes all code to be clean with respect to those lints. Each commit is independently reviewable. It's a shame that the lint selecting can't be shared across creates in some way. commit 8ce6297 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Tue Aug 14 20:04:02 2018 -0700 Use --entry-point not -c when building pex (pantsbuild#6349) I deterministically get this error: RuntimeError: Ambiguous script specification pants matches multiple entry points:pants = pants.bin.pants_loader:main pants = pants.bin.pants_loader:main when building with -c. pantsbuild#6267 changed this behaviour, but seems broken - this commit allows me to successfully build pexes. commit 30878d8 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Tue Aug 14 19:11:19 2018 -0700 Fix formatting of store.rs (pantsbuild#6350) It looks like pantsbuild#6336 merged with bad formatting. commit 5f2ad63 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Tue Aug 14 18:40:32 2018 -0700 Allow pex download path to be overridden (pantsbuild#6348) commit b7c607f Author: Stu Hood <stuhood@twitter.com> Date: Tue Aug 14 17:40:34 2018 -0700 Recover from cancelled remote execution RPCs (pantsbuild#6188) ### Problem It's possible for an individual RPC to be cancelled while an entire Operation continues, but this is not currently handled. See the problem description on pantsbuild#6100. ### Solution Recover from `RpcStatus::Cancelled` by retrying the same operation. ### Result Without the Fixes pantsbuild#6100. commit a96001e Author: Ity Kaul <ity@users.noreply.github.com> Date: Tue Aug 14 16:19:31 2018 -0700 Download Directory recursively from remote CAS (pantsbuild#6336) ### Problem We dont currently have the ability to download a `Directory`, and its contents, recursively. ### Solution Add `ensure_local_has_recursive_directory()` to be able to download a `Directory`, and its contents, recursively. commit e391cd1 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Tue Aug 14 16:11:22 2018 -0700 Process execution: Create symlink to JDK on demand (pantsbuild#6346) This creates a symlink `.jdk` pointing at the passed JDK, if one was passed. This is a hack to allow for a remote execution platform to provide a pointer at its pre-installed JDK without needing to pollute absolute paths into the action definition. Long-term, we want the JDK to be part of the input directory for the action, but that's a lot of work, and we want to experiment with pre-installed JDKs in the mean time. commit d79b7c3 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Mon Aug 13 21:26:08 2018 -0700 Simplify ExecuteProcessRequest construction (pantsbuild#6345) Remove create_from_snapshot and create_with_empty_snapshot - they don't pull their weight. Allow default values for all defaultable values. commit 33674ce Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Mon Aug 13 18:57:20 2018 -0700 Use forked version of grpcio (pantsbuild#6344) This includes tikv/grpc-rs#211 which stops us seeing RpcFinished errors when the server responds very quickly to requests. Unfortunately, this will slow down building pants, as you'll need to checkout the git repository, but that should be a once-per-machine cost. commit ccdf523 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Mon Aug 13 18:36:28 2018 -0700 ci.sh uses positive rather than negative flags (pantsbuild#6342) This makes it much easier to reason about what's happening, and keeps .travis.yml more concise. commit 68f4b59 Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Mon Aug 13 16:12:44 2018 -0700 Merge directories with identical files (pantsbuild#6343) This is useful for merging Snapshots for Targets if multiple targets own the same file. This should be rare, but does happen (e.g. multiple overlapping resource targets). commit e189acd Author: Daniel Wagner-Hall <dawagner@gmail.com> Date: Mon Aug 13 09:50:23 2018 -0700 Set chunk size in process_executor (pantsbuild#6337) commit 20d267f Author: Paul <yaup@cs.washington.edu> Date: Sun Aug 12 14:41:26 2018 -0700 added fullpath to fix path concat issue with files when not in git root (pantsbuild#6331) ### Problem As described in pantsbuild#6301 , when the git root and the build root is not the same, `changed` functionalities can be messed up because `git` returns path relative to the working directory, which produces bad paths when it is [concatenated with the git root](https://github.com/pantsbuild/pants/blob/c9f3af460a7504e082931a4b5a668b66f0cd4ed0/src/python/pants/scm/git.py#L161). ### Solution By adding the `--full-name` tag to [`git ls-files`](https://github.com/pantsbuild/pants/blob/c9f3af460a7504e082931a4b5a668b66f0cd4ed0/src/python/pants/scm/git.py#L178), the file concatenation is done properly ### Result Doing `./pants list --changed...` will concatenate file names correctly when it is not run from the git root.

All contrib folders should now be green on Python 2 and Python 3, excluding `googlejavaformat` and `python`. This fixes multiple issues causing Py3 failures + adds Py3 to Travis.

Eric Arellano added 11 commits August 12, 2018 08:12

Fix issues with strutil.

f19f3a5

* Shlex expects unicode in Py3 * Use builtins instead of binary_type and text_type

Fix issues with contrib go.

022d6c2

Fix integration test logger issue

aeb72be

Fix integration test issue with binary stdin

dfb5d0d

Fix node issue so that its stdout returns unicode

e6174d9

Fix Py3 issues with contrib scrooge.

8bc11cf

* Make thrift_linter test's mock's arg order be deterministic * Fix hasher unicode issue

Fix BeautifulSoup warnings

8c79503

Replace test_checker's example of print statements to bad class name.

8b78fbe

Python 3 raises a syntax error upon print statements, so the tests don't work as expected. Instead, bad class variable names works consistently across both Py2 and Py3.

Partially fix contrib python_eval - unicode hasher issues

2126adc

Fix ivy resolver hash issues

ec8f547

Add Python 3 contrib job to CI

17137b2

Eric-Arellano commented Aug 13, 2018

View reviewed changes

Eric Arellano added 8 commits August 12, 2018 23:01

Pass unicode to json.loads()

6c16095

Remove decodes() now that node's command.py decodes

fd184f2

Fix regex for non-deterministic ordering of go command's environment …

7fd5da4

…variables

Fix ivy XML failing to read generated files due to b'' string prefix

4194f10

Fix exceptions no longer having message attribute

b89f16e

Fix unicode vs bytes issues in nailgun_executor.

302a6df

The original fixes I made over-used bytes and had issues with mixing bytes and strings.

Merge remote-tracking branch 'upstream/master' into py3-green_contrib

c8bfdf2

Make bytes check more idiomatic from pantsbuild#6316

2a64b85

stuhood approved these changes Aug 16, 2018

View reviewed changes

Eric Arellano added 2 commits August 15, 2018 23:39

Skip Python Eval tests pantsbuild#6354

503b0c6

DRY regex changes

71dd278

stuhood merged commit b0a2469 into pantsbuild:master Aug 16, 2018

Eric-Arellano deleted the py3-green_contrib branch August 17, 2018 05:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 - fixes to get green contrib #6340

Python 3 - fixes to get green contrib #6340

Eric-Arellano commented Aug 13, 2018

Eric-Arellano left a comment

Eric-Arellano Aug 13, 2018

Eric-Arellano Aug 13, 2018

Eric-Arellano Aug 13, 2018

stuhood left a comment

stuhood Aug 16, 2018

		self.assertEqual(default_gopath, go_cmd.check_output().decode('utf-8').strip())

		goroot_env = r'GOROOT=[^ ]+'

Python 3 - fixes to get green contrib #6340

Python 3 - fixes to get green contrib #6340

Conversation

Eric-Arellano commented Aug 13, 2018

Eric-Arellano left a comment

Choose a reason for hiding this comment

googlejavaformat

python

Eric-Arellano Aug 13, 2018

Choose a reason for hiding this comment

Eric-Arellano Aug 13, 2018

Choose a reason for hiding this comment

Eric-Arellano Aug 13, 2018

Choose a reason for hiding this comment

stuhood left a comment

Choose a reason for hiding this comment

stuhood Aug 16, 2018

Choose a reason for hiding this comment