Py3 fixes contrib python fsq #22
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If `BuildConfigInitializer.get` were to be called with a `working_set` different from that passed on the initial call, a caching bug would be triggered. Eliminate this un-exercised possibility.
### Problem Invocations of zinc do not support jars. ### Solution Add functionality to the zinc wrapper to jar up the contents of _classesDirectory when an option is specified.
…toolchain selection (pantsbuild#6217) ### Problem pantsbuild#5951 explains the problem addressed by moving CLI arguments to individual `Executable` objects -- this reduces greatly the difficulty in generating appropriate command lines for the executables invoked. In this PR, it can be seen to remove a significant amount of repeated boilerplate. Additionally, we weren't distinguishing between a `Linker` to link the compiled object files of `gcc` or `g++` vs `clang` or `clang++`. We were attempting to generate a linker object which would work with *any of* `gcc`, `g++`, `clang`, or `clang++`, and this wasn't really feasible. Along with the above, this made it extremely difficult and error-prone to generate correct command lines / environments for executing the linker, which led to e.g. not being able to find `crti.o` (as one symptom addressed by this problem). ### Solution - Introduce `CToolchain` and `CppToolchain` in `environment.py`, which can be generated from `LLVMCToolchain`, `LLVMCppToolchain`, `GCCCToolchain`, or `GCCCppToolchain`. These toolchain datatypes are created in `native_toolchain.py`, where a single `@rule` for each ensures that no C or C++ compiler that someone can request was made without an accompanying linker, which will be configured to work with the compiler. - Introduce the `extra_args` property to the `Executable` mixin in `environment.py`, which `Executable` subclasses can just declare a datatype field named `extra_args` in order to override. This is used in `native_toolchain.py` to ensure platform-specific arguments and environment variables are set in the same `@rule` which produces a paired compiler and linker -- there is a single place to look at to see where all the process invocation environment variables and command-line arguments are set for a given toolchain. - Introduce the `ArchiveFileMapper` subsystem and use it to declare sets of directories to resolve within our BinaryTool archives `GCC` and `LLVM`. This subsystem allows globbing (and checks that there is a unique expansion), which makes it robust to e.g. platform-specific paths to things like include or lib directories. ### Result Removes several FIXMEs, including heavily-commented parts of `test_native_toolchain.py`. Partially addresses pantsbuild#5951 -- `setup_py.py` still generates its own execution environment from scratch, and this could be made more hygienic in the future. As noted in pantsbuild#6179 and pantsbuild#6205, this PR seems to immediately fix the CI failures in those PRs.
to run the demo binary: $ (cd src/rust/engine/ui && cargo run -p ui --release)
Running any test suite that extends `TestBase` will fail due to ~5 different problems. To reproduce, add `compatibility='CPython>=3.5',` to `pants_test/util/BUILD` entry of `meta`, then run ``` ./pants test tests/python/pants_test/util:meta ```
…() (pantsbuild#6226) ## Problem Python 2 allows us to freely mix bytes and unicode with IO. Python 3 forces us to choose between bytes vs unicode. This led to failing unit tests for `contextutil`. ## Solution Our code base has mixed use of `temporary_file()`. In some instances, we write unicode and it clearly seems to be a good case for text. In others, like downloads, bytes make sense. Hence, I think the best approach is to enable turning on or off binary mode, with a default given. The defaults should emulate Python 3's native implementation (principle of least surprise), as discussed below. ### temporary_file() In both Py2 and Py3, `tempfile` defaults to `w+b`, as it does not presume what you are writing to the file ([docs](https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryFile)). E.g. with our contrib code for downloads, bytes makes sense. So, I kept the default to bytes. ## Incremental port This change shouldn't break anything in Python 2, because it allows for intermixing string types. That's great, because it means that we can incrementally choose whether something should be text vs bytes.
### Problem `zinc_language_mixin.py` accesses its own deprecated property to get the fatal_warnings option value, which triggers a guaranteed warning. ### Solution Directly consume the fatal_warnings option value to avoid triggering the deprecation warning for the property access.
…tsbuild#6222) ### Problem In Py2, `os.environ` expects either bytes or ASCII strings. So, to pass it unicode, you must encode into bytes and pass the bytes, which is why we have several helper functions. In Py3, it expects unicode and will crash upon receiving bytes. We didn't catch this because there is no backport for the `os` module, and this is the first time running `contextutil` with Py3. ### Solution If Py2, keep existing conversion from unicode to bytes. If Py3, don't modify the passed in unicode. ### Testing CI is only going to prove this doesn't break Py2. It has no guarantees Py3 will work.. I got the unit tests to work, but there may still be other issues I don't know how to detect because we can't yet run CI in Py3 (too many other issues that have to be resolved)
…ild#6229) ### Problem Dirutil tests failing due to unicode vs bytes. `dirutil.py` itself is fine. I decided `safe_file_dump()` and `read_file()` should still work with bytes, not unicode. ### Related PRs To get completely green on Py3, requires pantsbuild#6226 and pantsbuild#6228. This can be merged before them both, though, as it shouldn't break Py2 and we aren't running Py3 yet.
…ild#6235) * Fix netrc issue with builtins rename * Fix TypeError exception message rewording between Py2 vs Py3 * Fix retry not reraising exception on Py3
…ntsbuild#6233) For `temporary_directory()`, we were using a mix of bytes and unicode in the arguments, which fails in Py3 because it does not allow mixing the two. The arguments could be either, but unicode makes more sense and requires less changes in our codebase. This changes the return type to be a `str`, not `bytes`, which impacted the TarFile logic.
Also, simplify the tests a bit
### Problem Testing analysis portability externally to zinc is not straightforward, because individual fields not being made portable do not necessarily cause loud failures, or even actual failures to compile incrementally. This test confirms one type of portability: https://github.com/pantsbuild/pants/blob/master/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_cache_compile_integration.py#L167, but there are other forms that can cause silent failures. See pantsbuild#6239 for an example. ### Solution Re-introduce the ability to dump analysis as text, to be used for integration tests that confirm that no references to previous workspaces are included.
…ptions_2.11;0.0.5 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
…ache_2.11;0.0.5 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
…til_2.11;0.0.5 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
…nalysis_2.11;0.0.5 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
…xtractor_2.11;0.0.5 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
…ompiler_2.11;0.0.6 # Prevent Travis-CI from running for this automated jar publish commit: # http://docs.travis-ci.com/user/how-to-skip-a-build/ [ci skip]
Running a `./pants lint` across the whole repo revealed an issue accessing a non-existant `self.linker.platform` attribute in `LinkSharedLibraries`. Add basic unit tests to exercise task execution with a full cache miss and then a full hit.
### Problem This adds a few more unit tests on top of pantsbuild#6201 -- specifically, testing in isolation that the platform is correctly set on a `python_dist()` which has no native sources itself, but depends on a `ctypes_compatible_c(pp)?_library()` (which is what pantsbuild#6201 fixes). It also moves all testing related to the `ctypes_compatible_c(pp)?_library()` targets into separate testing files to separate functionality more clearly. Resolves pantsbuild#6201.
Both these libraries are not supported in Py3, as they are backports.
Below syntax issue causes a failure to compile if the target file touches bin/daemon_pants_runner.py.
Calls to `sorted()` would fail in Py3 with some of the engine code, because Py3 doesn't allow relational comparisons for objects that don't implement `__lt__`, unlike Py2. It was complaining `ABCMeta < ABCMeta` is an invalid comparison.
Everything should be green with Py2 and Py3, excluding the native engine issue discussed on Slack + pending PRs pantsbuild#6251, pantsbuild#6245, and pantsbuild#6244.
I had missed porting this folder originally, because there isn't a corresponding source folder. Ran futurize over it + got Python 3 tests passing.
* Fix unicode being written to bytes file * Fix binary vs unicode issues in rest of test root * Fix one more binary.format() issue
## Problem Beautifulsoup4 version 4.3 doesn't run on Python 3.5+, due to a change made to the standard library. See https://stackoverflow.com/questions/28745153/importing-bs4-in-python-3-5. ## Solution Bumping from 4.3 to 4.6, the newest version. We can technically get away with 4.4 if we want to reduce the scope of bump, although the [changelog](https://bazaar.launchpad.net/~leonardr/beautifulsoup/bs4/view/head:/NEWS.txt) looks fairly innocuous. ## Testing Ran below in Py2 and Py3 mode (by changing BUILD's compatibility). Py3 still fails due to issues I'll address in another PR, but it no longer gives the beautifulsoup problem. ``` ./pants run src/python/pants/releases:packages -- list ./pants run src/python/pants/releases:packages -- list-owners ./pants run src/python/pants/releases:packages -- check-my-ownership ```
Not all tests are green on Py3 due to the ImportError rust issue, but this fixes a lot of them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.