Skip to content

Commit

Permalink
remove support for using the pants native toolchain with distutils Ex…
Browse files Browse the repository at this point in the history
…tensions in setup.py (#7126)

### Problem

Resolves #7016 (see [mirrored `pants-devel` post](https://groups.google.com/forum/#!topic/pants-devel/Y37d0tf4bKo)). This unblocks #6855 and #7046, as well as further work on #7122.

Closes #5661, closes #6841. I also made a long, long-overdue [github project for native code](https://github.com/pantsbuild/pants/projects/11).

### Solution

- Use the host environment to invoke compilers and linkers as desired in distutils, don't try to inject our own toolchain. See #5661 and #7016 for why this is extremely difficult to maintain. 

### Result

Further native backend iteration is unblocked.
  • Loading branch information
cosmicexplorer authored Jan 25, 2019
1 parent 8858f9f commit d8f0bd5
Show file tree
Hide file tree
Showing 38 changed files with 477 additions and 1,086 deletions.
3 changes: 1 addition & 2 deletions build-support/known_py3_pex_failures.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
tests/python/pants_test/backend/jvm/tasks:scala_repl_integration
tests/python/pants_test/backend/project_info/tasks:idea_plugin_integration
tests/python/pants_test/backend/python/tasks:integration
tests/python/pants_test/backend/python/tasks:isort_run_integration
tests/python/pants_test/backend/python/tasks:python_native_code_testing_1
tests/python/pants_test/backend/python:integration
tests/python/pants_test/base:exiter_integration
tests/python/pants_test/engine/legacy:changed_integration
tests/python/pants_test/engine/legacy:console_rule_integration
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Empty file.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

5 changes: 3 additions & 2 deletions src/python/pants/backend/native/tasks/conan_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,9 @@ def execute_codegen(self, target, target_workdir):
.format(cmdline, env, exit_code),
exit_code=exit_code)

# Read the stdout from the read-write buffer, from the beginning of the output.
conan_install_stdout = workunit.output('stdout').read_from(0)
# Read the stdout from the read-write buffer, from the beginning of the output, and convert
# to unicode.
conan_install_stdout = workunit.output('stdout').read_from(0).decode('utf-8')
pkg_sha = conan_requirement.parse_conan_stdout_for_pkg_sha(conan_install_stdout)

installed_data_dir = os.path.join(
Expand Down
78 changes: 2 additions & 76 deletions src/python/pants/backend/python/subsystems/python_native_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,10 @@

from __future__ import absolute_import, division, print_function, unicode_literals

import os
from builtins import str
from collections import defaultdict

from pex.pex import PEX

from pants.backend.native.config.environment import CppToolchain, CToolchain, Platform
from pants.backend.native.subsystems.native_toolchain import NativeToolchain
from pants.backend.native.subsystems.xcode_cli_tools import MIN_OSX_VERSION_ARG
from pants.backend.native.targets.native_library import NativeLibrary
from pants.backend.python.python_requirement import PythonRequirement
from pants.backend.python.subsystems.python_setup import PythonSetup
Expand All @@ -22,8 +17,7 @@
from pants.binaries.executable_pex_tool import ExecutablePexTool
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_property
from pants.util.objects import SubclassesOf, datatype
from pants.util.strutil import create_path_env_var
from pants.util.objects import SubclassesOf


class PythonNativeCode(Subsystem):
Expand Down Expand Up @@ -65,14 +59,11 @@ def _python_setup(self):
def pydist_has_native_sources(self, target):
return target.has_sources(extension=tuple(self._native_source_extensions))

def native_target_has_native_sources(self, target):
return target.has_sources()

@memoized_property
def _native_target_matchers(self):
return {
SubclassesOf(PythonDistribution): self.pydist_has_native_sources,
SubclassesOf(NativeLibrary): self.native_target_has_native_sources,
SubclassesOf(NativeLibrary): NativeLibrary.produces_ctypes_native_library,
}

def _any_targets_have_native_sources(self, targets):
Expand Down Expand Up @@ -158,68 +149,3 @@ def base_requirements(self):
PythonRequirement('setuptools=={}'.format(self.python_setup.setuptools_version)),
PythonRequirement('wheel=={}'.format(self.python_setup.wheel_version)),
]


class SetupPyNativeTools(datatype([
('c_toolchain', CToolchain),
('cpp_toolchain', CppToolchain),
('platform', Platform),
])):
"""The native tools needed for a setup.py invocation.
This class exists because `SetupPyExecutionEnvironment` is created manually, one per target.
"""


# TODO: It might be pretty useful to have an Optional TypeConstraint.
class SetupPyExecutionEnvironment(datatype([
# If None, don't set PYTHONPATH in the setup.py environment.
('setup_requires_pex', PEX),
# If None, don't execute in the toolchain environment.
'setup_py_native_tools',
])):

_SHARED_CMDLINE_ARGS = {
'darwin': lambda: [
MIN_OSX_VERSION_ARG,
'-Wl,-dylib',
'-undefined',
'dynamic_lookup',
],
'linux': lambda: ['-shared'],
}

def as_environment(self):
ret = {}

# TODO(#5951): the below is a lot of error-prone repeated logic -- we need a way to compose
# executables more hygienically. We should probably be composing each datatype's members, and
# only creating an environment at the very end.
native_tools = self.setup_py_native_tools
if native_tools:
# An as_tuple() method for datatypes could make this destructuring cleaner! Alternatively,
# constructing this environment could be done more compositionally instead of requiring all of
# these disparate fields together at once.
c_toolchain = native_tools.c_toolchain
c_compiler = c_toolchain.c_compiler
c_linker = c_toolchain.c_linker

cpp_toolchain = native_tools.cpp_toolchain
cpp_compiler = cpp_toolchain.cpp_compiler
cpp_linker = cpp_toolchain.cpp_linker

all_path_entries = (
c_compiler.path_entries +
c_linker.path_entries +
cpp_compiler.path_entries +
cpp_linker.path_entries)
# TODO(#6273): We prepend our toolchain to the PATH instead of overwriting it -- we need
# better control of the distutils compilation environment if we want to actually isolate the
# PATH (distutils does lots of sneaky things).
ret['PATH'] = create_path_env_var(all_path_entries, env=os.environ.copy(), prepend=True)

# GCC will output smart quotes in a variety of situations (leading to decoding errors
# downstream) unless we set this environment variable.
ret['LC_ALL'] = 'C'

return ret
Loading

0 comments on commit d8f0bd5

Please sign in to comment.