Skip to content

Commit

Permalink
Add libc search noop option (#6122)
Browse files Browse the repository at this point in the history
### Problem

Our `LibcDev` subsystem searches for `crti.o`, a file necessary to create executables on Linux. We use this in order to test both clang and gcc in `test_native_toolchain.py`. This is not needed except within Pants CI right now, and if the host system doesn't contain this file, it will always error out.

### Solution

- Add `--enable-libc-search` to `NativeToolchain`, defaulting to `False`, so that systems without this currently-unnecessary file don't need to worry about this implementation detail.
- Create `@platform_specific` decorator for individual tests which should only run on a specific platform to validate the expected behavior of this flag.

### Result

Pants invocations using the native toolchain on Linux hosts without a `crti.o` do not error out.
  • Loading branch information
cosmicexplorer committed Jul 15, 2018
1 parent 35f7873 commit 53e6ac3
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 14 deletions.
4 changes: 4 additions & 0 deletions pants.travis-ci.ini
Expand Up @@ -13,3 +13,7 @@ worker_count: 4
[test.pytest]
# Increase isolation. This is the direction that all tests will head in soon via #4586.
fast: false

[libc]
# Currently, we only need to search for a libc installation to test the native toolchain.
enable_libc_search: True
40 changes: 32 additions & 8 deletions src/python/pants/backend/native/subsystems/libc_dev.py
Expand Up @@ -17,8 +17,15 @@
class LibcDev(Subsystem):
"""Subsystem to detect and provide the host's installed version of a libc "dev" package.
This subsystem exists to give a useful error message if the package isn't
installed, and to allow a nonstandard install location.
A libc "dev" package is provided on most Linux systems by default, but may not be located at any
standardized path. We define a libc dev package as one which provides crti.o, an object file which
is part of any libc implementation and is required to create executables (more information
available at https://wiki.osdev.org/Creating_a_C_Library).
NB: This is currently unused except in CI, because we have no plans to support creating native
executables from C or C++ sources yet (PRs welcome!). It is used to provide an "end-to-end" test
of the compilation and linking toolchain in CI by creating and invoking a "hello world"
executable.
"""

options_scope = 'libc'
Expand All @@ -37,7 +44,11 @@ def _parse_search_dirs(self):
def register_options(cls, register):
super(LibcDev, cls).register_options(register)

register('--libc-dir', type=dir_option, default='/usr/lib', fingerprint=True, advanced=True,
register('--enable-libc-search', type=bool, default=False, fingerprint=True, advanced=True,
help="Whether to search for the host's libc installation. Set to False if the host "
"does not have a libc install with crti.o -- this file is necessary to create "
"executables on Linux hosts.")
register('--libc-dir', type=dir_option, default=None, fingerprint=True, advanced=True,
help='A directory containing a host-specific crti.o from libc.')
register('--host-compiler', type=str, default='gcc', fingerprint=True, advanced=True,
help='The host compiler to invoke with -print-search-dirs to find the host libc.')
Expand Down Expand Up @@ -74,12 +85,25 @@ def _get_host_libc_from_host_compiler(self):
fingerprint=hash_file(libc_crti_object_file))

@memoized_property
def host_libc(self):
def _host_libc(self):
"""Use the --libc-dir option if provided, otherwise invoke a host compiler to find libc dev."""
libc_dir_option = self.get_options().libc_dir
maybe_libc_crti = os.path.join(libc_dir_option, self._LIBC_INIT_OBJECT_FILE)
if os.path.isfile(maybe_libc_crti):
return HostLibcDev(crti_object=maybe_libc_crti,
fingerprint=hash_file(maybe_libc_crti))
if libc_dir_option:
maybe_libc_crti = os.path.join(libc_dir_option, self._LIBC_INIT_OBJECT_FILE)
if os.path.isfile(maybe_libc_crti):
return HostLibcDev(crti_object=maybe_libc_crti,
fingerprint=hash_file(maybe_libc_crti))
raise self.HostLibcDevResolutionError(
"Could not locate {} in directory {} provided by the --libc-dir option."
.format(self._LIBC_INIT_OBJECT_FILE, libc_dir_option))

return self._get_host_libc_from_host_compiler()

def get_libc_dirs(self, platform):
if not self.get_options().enable_libc_search:
return []

return platform.resolve_platform_specific({
'darwin': lambda: [],
'linux': lambda: [self._host_libc.get_lib_dir()],
})
Expand Up @@ -74,10 +74,10 @@ def select_linker(platform, native_toolchain):
if platform.normalized_os_name == 'darwin':
# TODO(#5663): turn this into LLVM when lld works.
linker = yield Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools)
libc_dirs = []
else:
linker = yield Get(Linker, Binutils, native_toolchain._binutils)
libc_dirs = [native_toolchain._libc_dev.host_libc.get_lib_dir()]

libc_dirs = native_toolchain._libc_dev.get_libc_dirs(platform)

# NB: We need to link through a provided compiler's frontend, and we need to know where all the
# compiler's libraries/etc are, so we set the executable name to the C++ compiler, which can find
Expand Down
Expand Up @@ -47,12 +47,12 @@ def _parse_libraries_from_compiler_search_dirs(self, compiler_exe, env):
except OSError as e:
# We use `safe_shlex_join` here to pretty-print the command.
raise self.ParseSearchDirsError(
"Invocation of '{}' with argv {!r} failed."
"Invocation of '{}' with argv '{}' failed."
.format(compiler_exe, safe_shlex_join(cmd)),
e)
except subprocess.CalledProcessError as e:
raise self.ParseSearchDirsError(
"Invocation of '{}' with argv {!r} exited with non-zero code {}. output:\n{}"
"Invocation of '{}' with argv '{}' exited with non-zero code {}. output:\n{}"
.format(compiler_exe, safe_shlex_join(cmd), e.returncode, e.output),
e)

Expand Down
1 change: 1 addition & 0 deletions tests/python/pants_test/backend/native/subsystems/BUILD
Expand Up @@ -9,6 +9,7 @@ python_tests(
'src/python/pants/util:process_handler',
'src/python/pants/util:strutil',
'tests/python/pants_test:test_base',
'tests/python/pants_test/backend/native/util',
'tests/python/pants_test/engine:scheduler_test_base',
'tests/python/pants_test/subsystem:subsystem_utils',
],
Expand Down
@@ -0,0 +1,79 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from pants.backend.native.config.environment import Platform
from pants.backend.native.subsystems.libc_dev import LibcDev
from pants.backend.native.subsystems.utils.parse_search_dirs import ParseSearchDirs
from pants_test.backend.native.util.platform_utils import platform_specific
from pants_test.subsystem.subsystem_util import global_subsystem_instance, init_subsystems
from pants_test.test_base import TestBase


class TestLibcDirectorySearchFailure(TestBase):

def setUp(self):
init_subsystems([LibcDev], options={
'libc': {
'enable_libc_search': True,
'libc_dir': '/does/not/exist',
},
})

self.libc = global_subsystem_instance(LibcDev)
self.platform = Platform.create()

@platform_specific('linux')
def test_libc_search_failure(self):
with self.assertRaises(LibcDev.HostLibcDevResolutionError) as cm:
self.libc.get_libc_dirs(self.platform)
expected_msg = (
"Could not locate crti.o in directory /does/not/exist provided by the --libc-dir option.")
self.assertEqual(expected_msg, str(cm.exception))

@platform_specific('darwin')
def test_libc_search_noop_osx(self):
self.assertEqual([], self.libc.get_libc_dirs(self.platform))


class TestLibcSearchDisabled(TestBase):

def setUp(self):
init_subsystems([LibcDev], options={
'libc': {
'enable_libc_search': False,
'libc_dir': '/does/not/exist',
},
})

self.libc = global_subsystem_instance(LibcDev)
self.platform = Platform.create()

@platform_specific('linux')
def test_libc_disabled_search(self):
self.assertEqual([], self.libc.get_libc_dirs(self.platform))


class TestLibcCompilerSearchFailure(TestBase):

def setUp(self):
init_subsystems([LibcDev], options={
'libc': {
'enable_libc_search': True,
'host_compiler': 'this_executable_does_not_exist',
},
})

self.libc = global_subsystem_instance(LibcDev)
self.platform = Platform.create()

@platform_specific('linux')
def test_libc_compiler_search_failure(self):
with self.assertRaises(ParseSearchDirs.ParseSearchDirsError) as cm:
self.libc.get_libc_dirs(self.platform)
expected_msg = (
"Invocation of 'this_executable_does_not_exist' with argv "
"'this_executable_does_not_exist -print-search-dirs' failed.")
self.assertIn(expected_msg, str(cm.exception))
@@ -1,5 +1,5 @@
# coding=utf-8
# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals
Expand All @@ -13,13 +13,14 @@
from pants.backend.native.register import rules as native_backend_rules
from pants.backend.native.subsystems.binaries.gcc import GCC
from pants.backend.native.subsystems.binaries.llvm import LLVM
from pants.backend.native.subsystems.libc_dev import LibcDev
from pants.backend.native.subsystems.native_toolchain import NativeToolchain
from pants.util.contextutil import environment_as, pushd, temporary_dir
from pants.util.dirutil import is_executable, safe_open
from pants.util.process_handler import subprocess
from pants.util.strutil import safe_shlex_join
from pants_test.engine.scheduler_test_base import SchedulerTestBase
from pants_test.subsystem.subsystem_util import global_subsystem_instance
from pants_test.subsystem.subsystem_util import global_subsystem_instance, init_subsystems
from pants_test.test_base import TestBase


Expand All @@ -28,6 +29,12 @@ class TestNativeToolchain(TestBase, SchedulerTestBase):
def setUp(self):
super(TestNativeToolchain, self).setUp()

init_subsystems([LibcDev, NativeToolchain], options={
'libc': {
'enable_libc_search': True,
},
})

self.platform = Platform.create()
self.toolchain = global_subsystem_instance(NativeToolchain)
self.rules = native_backend_rules()
Expand Down
8 changes: 8 additions & 0 deletions tests/python/pants_test/backend/native/util/BUILD
@@ -0,0 +1,8 @@
# coding=utf-8

python_library(
dependencies=[
'src/python/pants/backend/native/config',
'src/python/pants/util:osutil',
],
)
24 changes: 24 additions & 0 deletions tests/python/pants_test/backend/native/util/platform_utils.py
@@ -0,0 +1,24 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from pants.backend.native.config.environment import Platform
from pants.util.osutil import all_normalized_os_names


def platform_specific(normalized_os_name):
if normalized_os_name not in all_normalized_os_names():
raise ValueError("unrecognized platform: {}".format(normalized_os_name))

def decorator(test_fn):
def wrapper(self, *args, **kwargs):
# FIXME: This should be drawn from the v2 engine somehow.
platform = Platform.create()

if platform.normalized_os_name == normalized_os_name:
test_fn(self, *args, **kwargs)

return wrapper
return decorator

0 comments on commit 53e6ac3

Please sign in to comment.