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

Specify unicode vs bytes for Path and FileContent types #6303

Merged
merged 12 commits into from Aug 7, 2018

Conversation

Projects
None yet
3 participants
@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Aug 3, 2018

Path and FileContent from fs.py had no type constraints, which would lead to inconsistent behavior between Py2 and Py3. In Py2, for example, Path would store its value as a byte string, when we really want it represented as unicode.

@stuhood stuhood requested review from illicitonion and stuhood Aug 3, 2018

@illicitonion
Copy link
Contributor

illicitonion left a comment

Looks great, thanks!

@stuhood

stuhood approved these changes Aug 3, 2018

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor

Eric-Arellano commented Aug 4, 2018

./pants test tests/python/pants_test/base:exiter_integration fails now, which tries to compile a file with this as its source code import sys¡

Exception caught: (<class 'pex.compiler.CompilationFailure'>)
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_loader.py", line 73, in <module>
    main()
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_loader.py", line 69, in main
    PantsLoader.run()
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_loader.py", line 65, in run
    cls.load_and_execute(entrypoint)
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_loader.py", line 58, in load_and_execute
    entrypoint_main()
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_exe.py", line 38, in main
    PantsRunner(exiter, start_time=start_time).run()
  File "/Users/earellano/code/pants/src/python/pants/bin/pants_runner.py", line 53, in run
    return runner.run()
  File "/Users/earellano/code/pants/src/python/pants/bin/local_pants_runner.py", line 161, in run
    self._run()
  File "/Users/earellano/code/pants/src/python/pants/bin/local_pants_runner.py", line 225, in _run
    goal_runner_result = self._maybe_run_v1(run_tracker, reporting)
  File "/Users/earellano/code/pants/src/python/pants/bin/local_pants_runner.py", line 178, in _maybe_run_v1
    return goal_runner_factory.create().run()
  File "/Users/earellano/code/pants/src/python/pants/bin/goal_runner.py", line 204, in run
    return self._run_goals()
  File "/Users/earellano/code/pants/src/python/pants/bin/goal_runner.py", line 175, in _run_goals
    result = self._execute_engine()
  File "/Users/earellano/code/pants/src/python/pants/bin/goal_runner.py", line 163, in _execute_engine
    result = engine.execute(self._context, self._goals)
  File "/Users/earellano/code/pants/src/python/pants/engine/legacy_engine.py", line 26, in execute
    self.attempt(context, goals)
  File "/Users/earellano/code/pants/src/python/pants/engine/round_engine.py", line 233, in attempt
    goal_executor.attempt(explain)
  File "/Users/earellano/code/pants/src/python/pants/engine/round_engine.py", line 49, in attempt
    task.execute()
  File "/Users/earellano/code/pants/src/python/pants/backend/python/tasks/gather_sources.py", line 52, in execute
    pex = self._get_pex_for_versioned_targets(interpreter, invalidation_check.all_vts)
  File "/Users/earellano/code/pants/src/python/pants/backend/python/tasks/gather_sources.py", line 81, in _get_pex_for_versioned_targets
    self._build_pex(interpreter, safe_path, [vt.target for vt in versioned_targets])
  File "/Users/earellano/code/pants/src/python/pants/backend/python/tasks/gather_sources.py", line 88, in _build_pex
    builder.freeze()
  File "/Users/earellano/code/pants/build-support/pants_dev_deps.venv/lib/python2.7/site-packages/pex/pex_builder.py", line 459, in freeze
    self._precompile_source()
  File "/Users/earellano/code/pants/build-support/pants_dev_deps.venv/lib/python2.7/site-packages/pex/pex_builder.py", line 377, in _precompile_source
    compiled_relpaths = compiler.compile(self._chroot.path(), source_relpaths)
  File "/Users/earellano/code/pants/build-support/pants_dev_deps.venv/lib/python2.7/site-packages/pex/compiler.py", line 88, in compile
    'encountered %r during bytecode compilation.\nstderr was:\n%s\n' % (e, e.stderr)

Exception message: encountered NonZeroExit("received exit code 1 during execution of `[u'/usr/bin/python', '/var/folders/3q/rl02b2k936bb2x8mqfg48t180000gn/T/tmpLUJSih']` while trying to execute `[u'/usr/bin/python', '/var/folders/3q/rl02b2k936bb2x8mqfg48t180000gn/T/tmpLUJSih']`",) during bytecode compilation.
stderr was:
Traceback (most recent call last):
  File "/var/folders/3q/rl02b2k936bb2x8mqfg48t180000gn/T/tmpLUJSih", line 46, in <module>
    main(root, relpaths)
  File "/var/folders/3q/rl02b2k936bb2x8mqfg48t180000gn/T/tmpLUJSih", line 31, in main
    compiled, errored = compile(root, relpaths)
  File "/var/folders/3q/rl02b2k936bb2x8mqfg48t180000gn/T/tmpLUJSih", line 23, in compile
    py_compile.compile(abspath, cfile=pyc_abspath, dfile=relpath, doraise=True)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/py_compile.py", line 115, in compile
    py_exc = PyCompileError(err.__class__, err, dfile or file)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/py_compile.py", line 49, in __init__
    errmsg = tbtext.replace('File "<string>"', 'File "%s"' % file)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 67: ordinal not in range(128)

I don't have much time to look at this this weekend with moving back to AZ 🌵. I git bisected this to the second commit 1ca0ad1. Any thoughts?

@@ -703,7 +703,8 @@ impl Snapshot {
}

fn store_path(item: &Path) -> Value {
externs::store_bytes(item.as_os_str().as_bytes())
// TODO(illicitonion): see https://github.com/pantsbuild/pants/issues/6302
externs::store_utf8(&item.to_string_lossy())

This comment has been minimized.

@illicitonion

illicitonion Aug 6, 2018

Contributor

If you add this to externs.rs just below store_utf8:

///
/// Store a buffer of utf8 bytes to pass to Python. This will end up as a Python `unicode`.
///
#[cfg(unix)]
pub fn store_utf8_osstr(utf8: &OsStr) -> Value {
  use std::os::unix::ffi::OsStrExt;
  let bytes = utf8.as_bytes();
  with_externs(|e| (e.store_utf8)(e.context, bytes.as_ptr(), bytes.len() as u64).into())
}

and here instead call externs::store_utf8(item.as_os_str());

you should be able to close #6302 :)

@illicitonion illicitonion merged commit 97e1ef1 into pantsbuild:master Aug 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:py3-fixes_path branch Aug 7, 2018

CMLivingston pushed a commit to CMLivingston/pants that referenced this pull request Aug 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment