Skip to content

Commit 53e6ac3

Browse files
Add libc search noop option (pantsbuild#6122)
### 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.
1 parent 35f7873 commit 53e6ac3

File tree

9 files changed

+161
-14
lines changed

9 files changed

+161
-14
lines changed

Diff for: pants.travis-ci.ini

+4
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,7 @@ worker_count: 4
1313
[test.pytest]
1414
# Increase isolation. This is the direction that all tests will head in soon via #4586.
1515
fast: false
16+
17+
[libc]
18+
# Currently, we only need to search for a libc installation to test the native toolchain.
19+
enable_libc_search: True

Diff for: src/python/pants/backend/native/subsystems/libc_dev.py

+32-8
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,15 @@
1717
class LibcDev(Subsystem):
1818
"""Subsystem to detect and provide the host's installed version of a libc "dev" package.
1919
20-
This subsystem exists to give a useful error message if the package isn't
21-
installed, and to allow a nonstandard install location.
20+
A libc "dev" package is provided on most Linux systems by default, but may not be located at any
21+
standardized path. We define a libc dev package as one which provides crti.o, an object file which
22+
is part of any libc implementation and is required to create executables (more information
23+
available at https://wiki.osdev.org/Creating_a_C_Library).
24+
25+
NB: This is currently unused except in CI, because we have no plans to support creating native
26+
executables from C or C++ sources yet (PRs welcome!). It is used to provide an "end-to-end" test
27+
of the compilation and linking toolchain in CI by creating and invoking a "hello world"
28+
executable.
2229
"""
2330

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

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

7687
@memoized_property
77-
def host_libc(self):
88+
def _host_libc(self):
7889
"""Use the --libc-dir option if provided, otherwise invoke a host compiler to find libc dev."""
7990
libc_dir_option = self.get_options().libc_dir
80-
maybe_libc_crti = os.path.join(libc_dir_option, self._LIBC_INIT_OBJECT_FILE)
81-
if os.path.isfile(maybe_libc_crti):
82-
return HostLibcDev(crti_object=maybe_libc_crti,
83-
fingerprint=hash_file(maybe_libc_crti))
91+
if libc_dir_option:
92+
maybe_libc_crti = os.path.join(libc_dir_option, self._LIBC_INIT_OBJECT_FILE)
93+
if os.path.isfile(maybe_libc_crti):
94+
return HostLibcDev(crti_object=maybe_libc_crti,
95+
fingerprint=hash_file(maybe_libc_crti))
96+
raise self.HostLibcDevResolutionError(
97+
"Could not locate {} in directory {} provided by the --libc-dir option."
98+
.format(self._LIBC_INIT_OBJECT_FILE, libc_dir_option))
8499

85100
return self._get_host_libc_from_host_compiler()
101+
102+
def get_libc_dirs(self, platform):
103+
if not self.get_options().enable_libc_search:
104+
return []
105+
106+
return platform.resolve_platform_specific({
107+
'darwin': lambda: [],
108+
'linux': lambda: [self._host_libc.get_lib_dir()],
109+
})

Diff for: src/python/pants/backend/native/subsystems/native_toolchain.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ def select_linker(platform, native_toolchain):
7474
if platform.normalized_os_name == 'darwin':
7575
# TODO(#5663): turn this into LLVM when lld works.
7676
linker = yield Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools)
77-
libc_dirs = []
7877
else:
7978
linker = yield Get(Linker, Binutils, native_toolchain._binutils)
80-
libc_dirs = [native_toolchain._libc_dev.host_libc.get_lib_dir()]
79+
80+
libc_dirs = native_toolchain._libc_dev.get_libc_dirs(platform)
8181

8282
# NB: We need to link through a provided compiler's frontend, and we need to know where all the
8383
# compiler's libraries/etc are, so we set the executable name to the C++ compiler, which can find

Diff for: src/python/pants/backend/native/subsystems/utils/parse_search_dirs.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ def _parse_libraries_from_compiler_search_dirs(self, compiler_exe, env):
4747
except OSError as e:
4848
# We use `safe_shlex_join` here to pretty-print the command.
4949
raise self.ParseSearchDirsError(
50-
"Invocation of '{}' with argv {!r} failed."
50+
"Invocation of '{}' with argv '{}' failed."
5151
.format(compiler_exe, safe_shlex_join(cmd)),
5252
e)
5353
except subprocess.CalledProcessError as e:
5454
raise self.ParseSearchDirsError(
55-
"Invocation of '{}' with argv {!r} exited with non-zero code {}. output:\n{}"
55+
"Invocation of '{}' with argv '{}' exited with non-zero code {}. output:\n{}"
5656
.format(compiler_exe, safe_shlex_join(cmd), e.returncode, e.output),
5757
e)
5858

Diff for: tests/python/pants_test/backend/native/subsystems/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ python_tests(
99
'src/python/pants/util:process_handler',
1010
'src/python/pants/util:strutil',
1111
'tests/python/pants_test:test_base',
12+
'tests/python/pants_test/backend/native/util',
1213
'tests/python/pants_test/engine:scheduler_test_base',
1314
'tests/python/pants_test/subsystem:subsystem_utils',
1415
],
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# coding=utf-8
2+
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
3+
# Licensed under the Apache License, Version 2.0 (see LICENSE).
4+
5+
from __future__ import absolute_import, division, print_function, unicode_literals
6+
7+
from pants.backend.native.config.environment import Platform
8+
from pants.backend.native.subsystems.libc_dev import LibcDev
9+
from pants.backend.native.subsystems.utils.parse_search_dirs import ParseSearchDirs
10+
from pants_test.backend.native.util.platform_utils import platform_specific
11+
from pants_test.subsystem.subsystem_util import global_subsystem_instance, init_subsystems
12+
from pants_test.test_base import TestBase
13+
14+
15+
class TestLibcDirectorySearchFailure(TestBase):
16+
17+
def setUp(self):
18+
init_subsystems([LibcDev], options={
19+
'libc': {
20+
'enable_libc_search': True,
21+
'libc_dir': '/does/not/exist',
22+
},
23+
})
24+
25+
self.libc = global_subsystem_instance(LibcDev)
26+
self.platform = Platform.create()
27+
28+
@platform_specific('linux')
29+
def test_libc_search_failure(self):
30+
with self.assertRaises(LibcDev.HostLibcDevResolutionError) as cm:
31+
self.libc.get_libc_dirs(self.platform)
32+
expected_msg = (
33+
"Could not locate crti.o in directory /does/not/exist provided by the --libc-dir option.")
34+
self.assertEqual(expected_msg, str(cm.exception))
35+
36+
@platform_specific('darwin')
37+
def test_libc_search_noop_osx(self):
38+
self.assertEqual([], self.libc.get_libc_dirs(self.platform))
39+
40+
41+
class TestLibcSearchDisabled(TestBase):
42+
43+
def setUp(self):
44+
init_subsystems([LibcDev], options={
45+
'libc': {
46+
'enable_libc_search': False,
47+
'libc_dir': '/does/not/exist',
48+
},
49+
})
50+
51+
self.libc = global_subsystem_instance(LibcDev)
52+
self.platform = Platform.create()
53+
54+
@platform_specific('linux')
55+
def test_libc_disabled_search(self):
56+
self.assertEqual([], self.libc.get_libc_dirs(self.platform))
57+
58+
59+
class TestLibcCompilerSearchFailure(TestBase):
60+
61+
def setUp(self):
62+
init_subsystems([LibcDev], options={
63+
'libc': {
64+
'enable_libc_search': True,
65+
'host_compiler': 'this_executable_does_not_exist',
66+
},
67+
})
68+
69+
self.libc = global_subsystem_instance(LibcDev)
70+
self.platform = Platform.create()
71+
72+
@platform_specific('linux')
73+
def test_libc_compiler_search_failure(self):
74+
with self.assertRaises(ParseSearchDirs.ParseSearchDirsError) as cm:
75+
self.libc.get_libc_dirs(self.platform)
76+
expected_msg = (
77+
"Invocation of 'this_executable_does_not_exist' with argv "
78+
"'this_executable_does_not_exist -print-search-dirs' failed.")
79+
self.assertIn(expected_msg, str(cm.exception))

Diff for: tests/python/pants_test/backend/native/subsystems/test_native_toolchain.py

+9-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# coding=utf-8
2-
# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
2+
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
33
# Licensed under the Apache License, Version 2.0 (see LICENSE).
44

55
from __future__ import absolute_import, division, print_function, unicode_literals
@@ -13,13 +13,14 @@
1313
from pants.backend.native.register import rules as native_backend_rules
1414
from pants.backend.native.subsystems.binaries.gcc import GCC
1515
from pants.backend.native.subsystems.binaries.llvm import LLVM
16+
from pants.backend.native.subsystems.libc_dev import LibcDev
1617
from pants.backend.native.subsystems.native_toolchain import NativeToolchain
1718
from pants.util.contextutil import environment_as, pushd, temporary_dir
1819
from pants.util.dirutil import is_executable, safe_open
1920
from pants.util.process_handler import subprocess
2021
from pants.util.strutil import safe_shlex_join
2122
from pants_test.engine.scheduler_test_base import SchedulerTestBase
22-
from pants_test.subsystem.subsystem_util import global_subsystem_instance
23+
from pants_test.subsystem.subsystem_util import global_subsystem_instance, init_subsystems
2324
from pants_test.test_base import TestBase
2425

2526

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

32+
init_subsystems([LibcDev, NativeToolchain], options={
33+
'libc': {
34+
'enable_libc_search': True,
35+
},
36+
})
37+
3138
self.platform = Platform.create()
3239
self.toolchain = global_subsystem_instance(NativeToolchain)
3340
self.rules = native_backend_rules()

Diff for: tests/python/pants_test/backend/native/util/BUILD

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# coding=utf-8
2+
3+
python_library(
4+
dependencies=[
5+
'src/python/pants/backend/native/config',
6+
'src/python/pants/util:osutil',
7+
],
8+
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# coding=utf-8
2+
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
3+
# Licensed under the Apache License, Version 2.0 (see LICENSE).
4+
5+
from __future__ import absolute_import, division, print_function, unicode_literals
6+
7+
from pants.backend.native.config.environment import Platform
8+
from pants.util.osutil import all_normalized_os_names
9+
10+
11+
def platform_specific(normalized_os_name):
12+
if normalized_os_name not in all_normalized_os_names():
13+
raise ValueError("unrecognized platform: {}".format(normalized_os_name))
14+
15+
def decorator(test_fn):
16+
def wrapper(self, *args, **kwargs):
17+
# FIXME: This should be drawn from the v2 engine somehow.
18+
platform = Platform.create()
19+
20+
if platform.normalized_os_name == normalized_os_name:
21+
test_fn(self, *args, **kwargs)
22+
23+
return wrapper
24+
return decorator

0 commit comments

Comments
 (0)