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.

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

[WIP] move lib/include dir search into @rules in native_toolchain.py #6271

1 change: 1 addition & 0 deletions src/python/pants/backend/native/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
python_library(
dependencies=[
'3rdparty/python:future',
'src/python/pants/engine:isolated_process',
'src/python/pants/engine:rules',
'src/python/pants/util:objects',
'src/python/pants/util:osutil',
Expand Down
88 changes: 77 additions & 11 deletions src/python/pants/backend/native/config/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from abc import abstractproperty
from builtins import object

from pants.engine.isolated_process import ExecuteProcessRequest
from pants.engine.rules import SingletonRule
from pants.util.objects import datatype
from pants.util.osutil import all_normalized_os_names, get_normalized_os_name
Expand Down Expand Up @@ -44,19 +45,21 @@ def resolve_platform_specific(self, platform_specific_funs):

class Executable(object):

@abstractproperty
@property
def path_entries(self):
"""A list of directory paths containing this executable, to be used in a subprocess's PATH.

This may be multiple directories, e.g. if the main executable program invokes any subprocesses.
"""
return []

@abstractproperty
def library_dirs(self):
@property
def runtime_library_dirs(self):
"""Directories containing shared libraries that must be on the runtime library search path.

Note: this is for libraries needed for the current Executable to run -- see LinkerMixin below
for libraries that are needed at link time."""
return []

@abstractproperty
def exe_filename(self):
Expand All @@ -76,14 +79,21 @@ def as_invocation_environment_dict(self):
})
return {
'PATH': create_path_env_var(self.path_entries),
lib_env_var: create_path_env_var(self.library_dirs),
lib_env_var: create_path_env_var(self.runtime_library_dirs),
}

def as_execute_process_request(self, more_args=None):
argv = [self.exe_filename] + self.extra_args + (more_args or [])
Copy link
Contributor

Choose a reason for hiding this comment

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

more_args can be a tuple here to eliminate the or

Copy link
Contributor

Choose a reason for hiding this comment

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

Only sort-of. If you want to be defensive, this is appropriate. The caller can explicitly pass None.

return ExecuteProcessRequest.create_with_empty_snapshot(
argv=tuple(argv),
description=repr(self),
env=self.as_invocation_environment_dict)


class Assembler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
]), Executable):
pass

Expand All @@ -109,10 +119,26 @@ def as_invocation_environment_dict(self):
class Linker(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'linking_library_dirs',
'extra_args',
]), LinkerMixin): pass
]), LinkerMixin):

@property
def with_tupled_collections(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

The 3 with_tupled_collections are unused - kill. Additionally, with_* reads like a verb and not like a noun, so either @property is not appropriate or the naming is suboptimal.

"""???"""
# FIXME: convert these to using `copy()` when #6269 is merged.
return Linker(
path_entries=tuple(self.path_entries),
exe_filename=self.exe_filename,
runtime_library_dirs=tuple(self.runtime_library_dirs),
linking_library_dirs=tuple(self.linking_library_dirs),
extra_args=tuple(self.extra_args))
# return self.copy(
# path_entries=tuple(self.path_entries),
# runtime_library_dirs=tuple(self.runtime_library_dirs),
# linking_library_dirs=tuple(self.linking_library_dirs),
# extra_args=tuple(self.extra_args))


class CompilerMixin(Executable):
Expand All @@ -134,7 +160,7 @@ def as_invocation_environment_dict(self):
class CCompiler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'include_dirs',
'extra_args',
]), CompilerMixin):
Expand All @@ -143,15 +169,35 @@ class CCompiler(datatype([
def as_invocation_environment_dict(self):
ret = super(CCompiler, self).as_invocation_environment_dict.copy()

ret['CC'] = self.exe_filename
ret.update({
'CC': self.exe_filename,
# This avoids decoding errors parsing unicode smart quotes that gcc outputs for fun.
'LC_ALL': 'C',
})

return ret

@property
def with_tupled_collections(self):
"""???"""
# FIXME: convert these to using `copy()` when #6269 is merged.
return CCompiler(
path_entries=tuple(self.path_entries),
exe_filename=self.exe_filename,
runtime_library_dirs=tuple(self.runtime_library_dirs),
include_dirs=tuple(self.include_dirs),
extra_args=tuple(self.extra_args))
# return self.copy(
# path_entries=tuple(self.path_entries),
# runtime_library_dirs=tuple(self.runtime_library_dirs),
# include_dirs=tuple(self.include_dirs),
# extra_args=tuple(self.extra_args))


class CppCompiler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'include_dirs',
'extra_args',
]), CompilerMixin):
Expand All @@ -160,10 +206,30 @@ class CppCompiler(datatype([
def as_invocation_environment_dict(self):
ret = super(CppCompiler, self).as_invocation_environment_dict.copy()

ret['CXX'] = self.exe_filename
ret.update({
'CXX': self.exe_filename,
# This avoids decoding errors parsing unicode smart quotes that gcc outputs for fun.
'LC_ALL': 'C',
})

return ret

@property
def with_tupled_collections(self):
"""???"""
# FIXME: convert these to using `copy()` when #6269 is merged.
return CppCompiler(
path_entries=tuple(self.path_entries),
exe_filename=self.exe_filename,
runtime_library_dirs=tuple(self.runtime_library_dirs),
include_dirs=tuple(self.include_dirs),
extra_args=tuple(self.extra_args))
# return self.copy(
# path_entries=tuple(self.path_entries),
# runtime_library_dirs=tuple(self.runtime_library_dirs),
# include_dirs=tuple(self.include_dirs),
# extra_args=tuple(self.extra_args))


# NB: These wrapper classes for LLVM and GCC toolchains are performing the work of variants. A
# CToolchain cannot be requested directly, but native_toolchain.py provides an LLVMCToolchain,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ def assembler(self):
return Assembler(
path_entries=self.path_entries(),
exe_filename='as',
library_dirs=[])
runtime_library_dirs=[])

def linker(self):
return Linker(
path_entries=self.path_entries(),
exe_filename='ld',
library_dirs=[],
runtime_library_dirs=[],
linking_library_dirs=[],
extra_args=[])

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/native/subsystems/binaries/gcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def c_compiler(self, platform):
return CCompiler(
path_entries=self.path_entries,
exe_filename='gcc',
library_dirs=self._common_lib_dirs(platform),
runtime_library_dirs=self._common_lib_dirs(platform),
include_dirs=self._common_include_dirs,
extra_args=[])

Expand All @@ -89,7 +89,7 @@ def cpp_compiler(self, platform):
return CppCompiler(
path_entries=self.path_entries,
exe_filename='g++',
library_dirs=self._common_lib_dirs(platform),
runtime_library_dirs=self._common_lib_dirs(platform),
include_dirs=(self._common_include_dirs + self._cpp_include_dirs),
extra_args=[])

Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/native/subsystems/binaries/llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def linker(self, platform):
path_entries=self.path_entries,
exe_filename=platform.resolve_platform_specific(
self._PLATFORM_SPECIFIC_LINKER_NAME),
library_dirs=[],
runtime_library_dirs=[],
linking_library_dirs=[],
Copy link
Contributor

Choose a reason for hiding this comment

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

Im slightly confused on the difference here between runtime_library_dirs and linking_library_dirs. Is the latter for static libs?

Copy link
Contributor Author

@cosmicexplorer cosmicexplorer Sep 1, 2018

Choose a reason for hiding this comment

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

Directories containing shared libraries that must be on the runtime library search path. (this is LD_LIBRARY_PATH or DYLD_LIBRARY_PATH) vs Directories to search for libraries needed at link time. (this is LIBRARY_PATH, it's for both static and shared libs).

extra_args=[])

Expand All @@ -106,7 +106,7 @@ def c_compiler(self):
return CCompiler(
path_entries=self.path_entries,
exe_filename='clang',
library_dirs=self._common_lib_dirs,
runtime_library_dirs=self._common_lib_dirs,
include_dirs=self._common_include_dirs,
extra_args=[])

Expand All @@ -118,7 +118,7 @@ def cpp_compiler(self):
return CppCompiler(
path_entries=self.path_entries,
exe_filename='clang++',
library_dirs=self._common_lib_dirs,
runtime_library_dirs=self._common_lib_dirs,
include_dirs=(self._cpp_include_dirs + self._common_include_dirs),
extra_args=[])

Expand Down
109 changes: 0 additions & 109 deletions src/python/pants/backend/native/subsystems/libc_dev.py

This file was deleted.

Loading