Skip to content
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.

Sign up for GitHub

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

Associate cli arguments with executables and refactor llvm/gcc c/c++ toolchain selection #6217

Merged
merged 24 commits into from
Jul 24, 2018
Merged

Associate cli arguments with executables and refactor llvm/gcc c/c++ toolchain selection #6217

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f330514
introduce CToolchain and CppToolchain to pair related tools
cosmicexplorer Jul 23, 2018
9e36cd0
make the rest of the native backend subsystem testing work
cosmicexplorer Jul 23, 2018
b0e7b52
make python_dist() compilation work with the new native backend
cosmicexplorer Jul 23, 2018
8193ca9
fix python dist integration tests
cosmicexplorer Jul 23, 2018
1c0c880
move argument generation into the executable objects themselves
cosmicexplorer Jul 23, 2018
6e3bcbc
make everything work on osx
cosmicexplorer Jul 23, 2018
9a16efb
remove/update TODOs/FIXMEs
cosmicexplorer Jul 23, 2018
2bc7698
remove unnecessary `RootRule`s
cosmicexplorer Jul 23, 2018
900a476
refactor ParseSearchDirs
cosmicexplorer Jul 23, 2018
78fe179
add FIXME
cosmicexplorer Jul 23, 2018
e67044c
refactor gcc subsystem to use globs instead of guessing platform-spec…
cosmicexplorer Jul 23, 2018
3441e74
remove GCCCCompiler, GCCCLinker, etc
cosmicexplorer Jul 23, 2018
6843579
inject CToolchain and CppToolchain and select which with other @rules
cosmicexplorer Jul 23, 2018
b2a750b
centralize -mmacosx-version-min=10.11 argument creation
cosmicexplorer Jul 24, 2018
973f26e
clarify some comments on the clang args we add
cosmicexplorer Jul 24, 2018
3051235
split off indexing into BinaryTool archives using the new ArchiveFile…
cosmicexplorer Jul 24, 2018
09526d2
remove -nostdinc from clang args
cosmicexplorer Jul 24, 2018
8f34e37
make searching gcc BinaryTool paths work on osx
cosmicexplorer Jul 24, 2018
8fd7baa
fix up docstring formatting
cosmicexplorer Jul 24, 2018
e5a5d7e
add the environment to the compiler or linker invocation so we can re…
cosmicexplorer Jul 24, 2018
f4a8a7d
only add --gcc-toolchain argument to clang and clang++ on linux
cosmicexplorer Jul 24, 2018
9846475
remove all python backend rules
cosmicexplorer Jul 24, 2018
7ebfe51
remove TODO in SetupPyExecutionEnvironment
cosmicexplorer Jul 24, 2018
ac543fe
remove CToolchain/CppToolchain rules and add explanatory comment in e…
cosmicexplorer Jul 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/python/pants/backend/native/config/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ class CompilerMixin(Executable):
def include_dirs(self):
"""Directories to search for header files to #include during compilation."""

@property
def as_invocation_environment_dict(self):
ret = super(CompilerMixin, self).as_invocation_environment_dict.copy()

if self.include_dirs:
ret['CPATH'] = create_path_env_var(self.include_dirs)

return ret


class CCompiler(datatype([
'path_entries',
Expand All @@ -132,10 +141,7 @@ class CCompiler(datatype([

@property
def as_invocation_environment_dict(self):
ret = super(CompilerMixin, self).as_invocation_environment_dict.copy()

if self.include_dirs:
ret['C_INCLUDE_PATH'] = create_path_env_var(self.include_dirs)
ret = super(CCompiler, self).as_invocation_environment_dict.copy()

ret['CC'] = self.exe_filename
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you been able to verify that setup.py compilation is taking place with the correct exe? I recall we hit some cases where it wasn't actually respecting this variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the correct exe_filename? There is no testing that the setup.py compilation does anything other than succeed, currently, but if you make the build fail (e.g. by inserting a syntax error into a C/C++ source), pex will print the stdout and stderr to the terminal stderr, and you can see that modifying exe_filenames (or e.g. selecting GCCCToolchain instead of the llvm one in python_native_code.py) will change the compiler used for the generated command line to build setup.py native sources.

At this point, I'm not sure what we should be testing wrt setup.py compilation other than success. If you have some itches you want to scratch, an issue just listing what tests we should add, or a PR doing that, would be great so I or someone else can address it in full. This wouldn't need to more than a few sentences.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that may have been the case (wrt not respecting CC), but I don't remember the situation, and the issue may have been e.g. that we were adding the wrong entries to the PATH.


Expand All @@ -152,10 +158,7 @@ class CppCompiler(datatype([

@property
def as_invocation_environment_dict(self):
ret = super(CompilerMixin, self).as_invocation_environment_dict.copy()

if self.include_dirs:
ret['CPLUS_INCLUDE_PATH'] = create_path_env_var(self.include_dirs)
ret = super(CppCompiler, self).as_invocation_environment_dict.copy()

ret['CXX'] = self.exe_filename

Expand Down
35 changes: 19 additions & 16 deletions src/python/pants/backend/native/subsystems/binaries/gcc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

import os

from pants.backend.native.config.environment import CCompiler, CppCompiler
from pants.backend.native.config.environment import CCompiler, CppCompiler, Platform
from pants.backend.native.subsystems.utils.archive_file_mapper import ArchiveFileMapper
from pants.binaries.binary_tool import NativeTool
from pants.engine.rules import RootRule, rule
from pants.engine.selectors import Select
from pants.util.memo import memoized_property
from pants.util.memo import memoized_method, memoized_property


class GCC(NativeTool):
Expand Down Expand Up @@ -42,11 +42,14 @@ def _filemap(self, all_components_list):
def path_entries(self):
return self._filemap([('bin',)])

@memoized_property
def _common_lib_dirs(self):
return self._filemap([
@memoized_method
def _common_lib_dirs(self, platform):
lib64_tuples = platform.resolve_platform_specific({
'darwin': lambda: [],
'linux': lambda: [('lib64',)],
})
return self._filemap(lib64_tuples + [
('lib',),
('lib64',),
('lib/gcc',),
('lib/gcc/*', self.version()),
])
Expand All @@ -58,11 +61,11 @@ def _common_include_dirs(self):
('lib/gcc/*', self.version(), 'include'),
])

def c_compiler(self):
def c_compiler(self, platform):
return CCompiler(
path_entries=self.path_entries,
exe_filename='gcc',
library_dirs=self._common_lib_dirs,
library_dirs=self._common_lib_dirs(platform),
include_dirs=self._common_include_dirs,
extra_args=[])

Expand All @@ -82,23 +85,23 @@ def _cpp_include_dirs(self):

return most_cpp_include_dirs + [plat_cpp_header_dir]

def cpp_compiler(self):
def cpp_compiler(self, platform):
return CppCompiler(
path_entries=self.path_entries,
exe_filename='g++',
library_dirs=self._common_lib_dirs,
library_dirs=self._common_lib_dirs(platform),
include_dirs=(self._common_include_dirs + self._cpp_include_dirs),
extra_args=[])


@rule(CCompiler, [Select(GCC)])
def get_gcc(gcc):
return gcc.c_compiler()
@rule(CCompiler, [Select(GCC), Select(Platform)])
def get_gcc(gcc, platform):
return gcc.c_compiler(platform)


@rule(CppCompiler, [Select(GCC)])
def get_gplusplus(gcc):
return gcc.cpp_compiler()
@rule(CppCompiler, [Select(GCC), Select(Platform)])
def get_gplusplus(gcc, platform):
return gcc.cpp_compiler(platform)


def create_gcc_rules():
Expand Down