From 21084cdbb2383ceae68de96011dc357e69335b4f Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Mon, 10 Sep 2018 18:27:28 -0700 Subject: [PATCH 1/3] Split conan resolve by native_external_library targets. --- .../native/tasks/link_shared_libraries.py | 41 ++++++------ .../backend/native/tasks/native_compile.py | 23 ++++--- .../tasks/native_external_library_fetch.py | 65 ++++++++++--------- 3 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/python/pants/backend/native/tasks/link_shared_libraries.py b/src/python/pants/backend/native/tasks/link_shared_libraries.py index 1440f525a24..8e59f514039 100644 --- a/src/python/pants/backend/native/tasks/link_shared_libraries.py +++ b/src/python/pants/backend/native/tasks/link_shared_libraries.py @@ -32,17 +32,7 @@ class LinkSharedLibraryRequest(datatype([ ('external_lib_dirs', tuple), ('external_lib_names', tuple), ])): - - @classmethod - def with_external_libs_product(cls, external_libs_product=None, *args, **kwargs): - if external_libs_product is None: - lib_dirs = () - lib_names = () - else: - lib_dirs = (external_libs_product.lib_dir,) - lib_names = external_libs_product.lib_names - - return cls(*args, external_lib_dirs=lib_dirs, external_lib_names=lib_names, **kwargs) + pass class LinkSharedLibraries(NativeTask): @@ -154,21 +144,30 @@ def _make_link_request(self, deps = self._retrieve_single_product_at_target_base(native_target_deps_product, vt.target) all_compiled_object_files = [] - for dep_tgt in deps: - self.context.log.debug("dep_tgt: {}".format(dep_tgt)) - object_files = self._retrieve_single_product_at_target_base(compiled_objects_product, dep_tgt) - self.context.log.debug("object_files: {}".format(object_files)) - object_file_paths = object_files.file_paths() - self.context.log.debug("object_file_paths: {}".format(object_file_paths)) - all_compiled_object_files.extend(object_file_paths) - - return LinkSharedLibraryRequest.with_external_libs_product( + if compiled_objects_product.get(dep_tgt): + self.context.log.debug("dep_tgt: {}".format(dep_tgt)) + object_files = self._retrieve_single_product_at_target_base(compiled_objects_product, dep_tgt) + self.context.log.debug("object_files: {}".format(object_files)) + object_file_paths = object_files.file_paths() + self.context.log.debug("object_file_paths: {}".format(object_file_paths)) + all_compiled_object_files.extend(object_file_paths) + + external_lib_dirs = [] + external_lib_names = [] + if external_libs_product is not None: + for nelf in external_libs_product.get_for_targets(deps): + if nelf.lib_dir: + external_lib_dirs.append(nelf.lib_dir) + external_lib_names.extend(nelf.lib_names) + + return LinkSharedLibraryRequest( linker=self.linker, object_files=tuple(all_compiled_object_files), native_artifact=vt.target.ctypes_native_library, output_dir=vt.results_dir, - external_libs_product=external_libs_product) + external_lib_dirs=tuple(external_lib_dirs), + external_lib_names=tuple(external_lib_names)) _SHARED_CMDLINE_ARGS = { 'darwin': lambda: ['-Wl,-dylib'], diff --git a/src/python/pants/backend/native/tasks/native_compile.py b/src/python/pants/backend/native/tasks/native_compile.py index ad57a888548..c491f192219 100644 --- a/src/python/pants/backend/native/tasks/native_compile.py +++ b/src/python/pants/backend/native/tasks/native_compile.py @@ -10,6 +10,7 @@ from collections import defaultdict from pants.backend.native.config.environment import Executable +from pants.backend.native.targets.external_native_library import ExternalNativeLibrary from pants.backend.native.targets.native_library import NativeLibrary from pants.backend.native.tasks.native_external_library_fetch import NativeExternalLibraryFiles from pants.backend.native.tasks.native_task import NativeTask @@ -51,7 +52,7 @@ class NativeCompile(NativeTask, AbstractClass): # operate on for `strict_deps` calculation. # NB: `source_target_constraint` must be overridden. source_target_constraint = None - dependent_target_constraint = SubclassesOf(NativeLibrary) + dependent_target_constraint = SubclassesOf(ExternalNativeLibrary, NativeLibrary) # `NativeCompile` will use `workunit_label` as the name of the workunit when executing the # compiler process. `workunit_label` must be set to a string. @@ -128,14 +129,14 @@ def execute(self): source_targets = self.context.targets(self.source_target_constraint.satisfied_by) with self.invalidated(source_targets, invalidate_dependents=True) as invalidation_check: - for vt in invalidation_check.invalid_vts: + for vt in invalidation_check.all_vts: deps = self.native_deps(vt.target) self._add_product_at_target_base(native_deps_product, vt.target, deps) - compile_request = self._make_compile_request(vt, deps, external_libs_product) - self.context.log.debug("compile_request: {}".format(compile_request)) - self._compile(compile_request) + if not vt.valid: + compile_request = self._make_compile_request(vt, deps, external_libs_product) + self.context.log.debug("compile_request: {}".format(compile_request)) + self._compile(compile_request) - for vt in invalidation_check.all_vts: object_files = self.collect_cached_objects(vt) self._add_product_at_target_base(object_files_product, vt.target, object_files) @@ -192,18 +193,20 @@ def get_compiler(self): def _compiler(self): return self.get_compiler() - def _get_third_party_include_dirs(self, external_libs_product): + def _get_third_party_include_dirs(self, external_libs_product, dependencies): if not external_libs_product: return [] - directory = external_libs_product.include_dir - return [directory] if directory else [] + return [nelf.include_dir + for nelf in external_libs_product.get_for_targets(dependencies) + if nelf.include_dir] def _make_compile_request(self, versioned_target, dependencies, external_libs_product): target = versioned_target.target include_dirs = [self._include_dirs_for_target(dep_tgt) for dep_tgt in dependencies] - include_dirs.extend(self._get_third_party_include_dirs(external_libs_product)) + include_dirs.extend(self._get_third_party_include_dirs(external_libs_product, dependencies)) + print('>>> include_dirs: {}'.format(include_dirs)) sources_and_headers = self.get_sources_headers_for_target(target) diff --git a/src/python/pants/backend/native/tasks/native_external_library_fetch.py b/src/python/pants/backend/native/tasks/native_external_library_fetch.py index 0eb75ce820c..4a863e9428a 100644 --- a/src/python/pants/backend/native/tasks/native_external_library_fetch.py +++ b/src/python/pants/backend/native/tasks/native_external_library_fetch.py @@ -15,10 +15,10 @@ from pants.backend.native.targets.external_native_library import ExternalNativeLibrary from pants.base.build_environment import get_pants_cachedir from pants.base.exceptions import TaskError +from pants.goal.products import UnionProducts from pants.invalidation.cache_manager import VersionedTargetSet from pants.task.task import Task from pants.util.contextutil import environment_as -from pants.util.dirutil import safe_mkdir from pants.util.memo import memoized_property from pants.util.objects import Exactly, datatype from pants.util.process_handler import subprocess @@ -94,6 +94,10 @@ def register_options(cls, register): register('--conan-remotes', type=list, default=['https://conan.bintray.com'], advanced=True, fingerprint=True, help='The conan remote to download conan packages from.') + @classmethod + def implementation_version(cls): + return super(NativeExternalLibraryFetch, cls).implementation_version() + [('NativeExternalLibraryFetch', 0)] + @classmethod def subsystem_dependencies(cls): return super(NativeExternalLibraryFetch, cls).subsystem_dependencies() + (Conan.scoped(cls),) @@ -103,7 +107,10 @@ def product_types(cls): return [NativeExternalLibraryFiles] @property - def cache_target_dirs(self): + def create_target_dirs(self): + # We create per-target directories in order to act as isolated collections of fetched files. + # But do not attempt to automatically cache then (cache_target_dirs), because the entire resolve + # must be have its own merged cachekey/VT. return True @memoized_property @@ -128,40 +135,35 @@ def execute(self): with self.invalidated(native_lib_tgts, invalidate_dependents=True) as invalidation_check: resolve_vts = VersionedTargetSet.from_versioned_targets(invalidation_check.all_vts) - vts_results_dir = self._prepare_vts_results_dir(resolve_vts) if invalidation_check.invalid_vts or not resolve_vts.valid: for vt in invalidation_check.all_vts: - self._fetch_packages(vt, vts_results_dir) + self._fetch_packages(vt) - native_external_libs_product = self._collect_external_libs(vts_results_dir) + native_external_libs_product = self._collect_external_libs(invalidation_check.all_vts) self.context.products.register_data(NativeExternalLibraryFiles, native_external_libs_product) - def _prepare_vts_results_dir(self, vts): - """ - Given a `VersionedTargetSet`, prepare its results dir. - """ - vt_set_results_dir = os.path.join(self.workdir, vts.cache_key.hash) - safe_mkdir(vt_set_results_dir) - return vt_set_results_dir - - def _collect_external_libs(self, results_dir): + def _collect_external_libs(self, vts): """ Sets the relevant properties of the task product (`NativeExternalLibraryFiles`) object. """ - lib_dir = os.path.join(results_dir, 'lib') - include_dir = os.path.join(results_dir, 'include') - - lib_names = [] - if os.path.isdir(lib_dir): - for filename in os.listdir(lib_dir): - lib_name = self._parse_lib_name_from_library_filename(filename) - if lib_name: - lib_names.append(lib_name) - - return NativeExternalLibraryFiles(include_dir=include_dir, - lib_dir=lib_dir, - lib_names=tuple(lib_names)) + product = UnionProducts() + for vt in vts: + lib_dir = os.path.join(vt.results_dir, 'lib') + include_dir = os.path.join(vt.results_dir, 'include') + + lib_names = [] + if os.path.isdir(lib_dir): + for filename in os.listdir(lib_dir): + lib_name = self._parse_lib_name_from_library_filename(filename) + if lib_name: + lib_names.append(lib_name) + + nelf = NativeExternalLibraryFiles(include_dir=include_dir, + lib_dir=lib_dir, + lib_names=tuple(lib_names)) + product.add_for_target(vt.target, [nelf]) + return product def _get_conan_data_dir_path_for_package(self, pkg_dir_path, pkg_sha): return os.path.join(self.workdir, @@ -237,14 +239,13 @@ def _copy_package_contents_from_conan_dir(self, results_dir, conan_requirement, if os.path.exists(src_include): copy_tree(src_include, dest_include) - def _fetch_packages(self, vt, vts_results_dir): + def _fetch_packages(self, vt): """ Invoke the conan pex to fetch conan packages specified by a `ExternalLibLibrary` target. - :param vt: a versioned target containing conan package specifications. - :param vts_results_dir: the results directory of the VersionedTargetSet - for the purpose of aggregating package contents. + :param vt: a versioned target containing conan package specifications, and with a results_dir + that we can clone outputs into. """ # NB: CONAN_USER_HOME specifies the directory to use for the .conan data directory. @@ -274,4 +275,4 @@ def _fetch_packages(self, vt, vts_results_dir): ) pkg_sha = conan_requirement.parse_conan_stdout_for_pkg_sha(stdout) - self._copy_package_contents_from_conan_dir(vts_results_dir, conan_requirement, pkg_sha) + self._copy_package_contents_from_conan_dir(vt.results_dir, conan_requirement, pkg_sha) From b8b1c4199d5dec90b6739448758d181fa59fcb98 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sun, 14 Oct 2018 13:29:08 -0700 Subject: [PATCH 2/3] remove an artifact of a debugging session long past --- src/python/pants/backend/native/tasks/native_compile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/pants/backend/native/tasks/native_compile.py b/src/python/pants/backend/native/tasks/native_compile.py index c491f192219..ba9b0fd25ed 100644 --- a/src/python/pants/backend/native/tasks/native_compile.py +++ b/src/python/pants/backend/native/tasks/native_compile.py @@ -206,7 +206,6 @@ def _make_compile_request(self, versioned_target, dependencies, external_libs_pr include_dirs = [self._include_dirs_for_target(dep_tgt) for dep_tgt in dependencies] include_dirs.extend(self._get_third_party_include_dirs(external_libs_product, dependencies)) - print('>>> include_dirs: {}'.format(include_dirs)) sources_and_headers = self.get_sources_headers_for_target(target) From 30285b6ff3c9dea9181d3368f533d4854355b4c3 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sun, 14 Oct 2018 13:32:36 -0700 Subject: [PATCH 3/3] move a TODO closer to the code it's referencing --- src/python/pants/backend/native/tasks/native_compile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/native/tasks/native_compile.py b/src/python/pants/backend/native/tasks/native_compile.py index ba9b0fd25ed..8706fdbaa06 100644 --- a/src/python/pants/backend/native/tasks/native_compile.py +++ b/src/python/pants/backend/native/tasks/native_compile.py @@ -223,12 +223,12 @@ def _make_compile_argv(self, compile_request): err_flags = ['-Werror'] if compile_request.fatal_warnings else [] # We are going to execute in the target output, so get absolute paths for everything. - # TODO: If we need to produce static libs, don't add -fPIC! (could use Variants -- see #5788). buildroot = get_buildroot() argv = ( [compiler.exe_filename] + compiler.extra_args + err_flags + + # TODO: If we need to produce static libs, don't add -fPIC! (could use Variants -- see #5788). ['-c', '-fPIC'] + [ '-I{}'.format(os.path.join(buildroot, inc_dir))