From 9a25d32f7df950b56688f1ba1c137cf5d54fde12 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 30 Nov 2018 10:48:29 -0800 Subject: [PATCH] fill out ???s and link to issues for TODOs --- pants.ini | 2 +- src/python/pants/backend/native/config/environment.py | 5 ++++- .../backend/python/tasks/test_ctypes_integration.py | 6 +++--- .../python/tasks/util/build_local_dists_test_base.py | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pants.ini b/pants.ini index 0954362280f..66cdcd67fea 100644 --- a/pants.ini +++ b/pants.ini @@ -403,6 +403,6 @@ fragment: True [sitegen] config_path: src/docs/docsite.json -# TODO(#???): this should be removed when we can fully support both toolchains. +# TODO(#6848): this should be removed when we can fully support both toolchains. [native-build-settings] toolchain_variant: llvm diff --git a/src/python/pants/backend/native/config/environment.py b/src/python/pants/backend/native/config/environment.py index 2ded3778ab5..0f4df867865 100644 --- a/src/python/pants/backend/native/config/environment.py +++ b/src/python/pants/backend/native/config/environment.py @@ -96,7 +96,10 @@ def linking_library_dirs(self): @abstractproperty def extra_object_files(self): - """???""" + """A list of object files required to perform a successful link. + + This includes crti.o from libc for gcc on Linux, for example. + """ @property def as_invocation_environment_dict(self): diff --git a/tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py b/tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py index 104a8385820..cbbb3b090f5 100644 --- a/tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py +++ b/tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py @@ -188,7 +188,7 @@ def _assert_ctypes_interop_with_mock_buildroot(self, toolchain_variant): command=['binary', self._binary_target_with_interop], # Explicitly set to True (although this is the default). config={ - # TODO: don't make it possible to forget to add the toolchain_variant option! + # TODO(#6848): don't make it possible to forget to add the toolchain_variant option! 'native-build-settings': { 'toolchain_variant': toolchain_variant, 'strict_deps': True, @@ -200,7 +200,7 @@ def _assert_ctypes_interop_with_mock_buildroot(self, toolchain_variant): self.assertIn(self._include_not_found_message_for_variant[toolchain_variant], pants_binary_strict_deps_failure.stdout_data) - # TODO(#???): we need to provide the libstdc++.so.6.dylib which comes with gcc on osx in the + # TODO(#6848): we need to provide the libstdc++.so.6.dylib which comes with gcc on osx in the # DYLD_LIBRARY_PATH during the 'run' goal somehow. attempt_pants_run = Platform.create().resolve_platform_specific({ 'darwin': lambda: toolchain_variant != 'gnu', @@ -224,7 +224,7 @@ def test_ctypes_third_party_integration(self): pants_binary = self.run_pants(['binary', self._binary_target_with_third_party]) self.assert_success(pants_binary) - # TODO(#???): this fails when run with gcc on osx as it requires gcc's libstdc++.so.6.dylib to + # TODO(#6848): this fails when run with gcc on osx as it requires gcc's libstdc++.so.6.dylib to # be available on the runtime library path. pants_run = self.run_pants(['-q', 'run', self._binary_target_with_third_party]) self.assert_success(pants_run) diff --git a/tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py b/tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py index 53cfcbe425f..3831dbed46d 100644 --- a/tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py +++ b/tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py @@ -115,7 +115,7 @@ def _create_distribution_synthetic_target(self, python_dist_target, extra_target target_roots=([python_dist_target] + extra_targets), for_task_types=(all_synthesized_task_types), for_subsystems=[PythonRepos, LibcDev], - # TODO(#???): we should be testing all of these with both of our toolchains. + # TODO(#6848): we should be testing all of these with both of our toolchains. options={ 'native-build-settings': { 'toolchain_variant': 'llvm',