Conan (third party) support for ctypes native libraries #5998

CMLivingston · 2018-06-21T21:33:31Z

Problem

The new targets introduced in #5815 have no means of declaring, fetching, and using third party native dependencies.

Solution

Integrate the Conan package manager to fetch packages from a remote package store via a new task, copy the package data to the results directory of third_party_native_library targets in play, and create a product that the compile and link tasks can consume. Plumb the directory paths provided by the product through to the command lines of the compile and link steps.

Result

Users can now depend on third party native libraries in their ctypes_compatible_cpp_library targets from either conan-center or a remote URI that they specify via an option.

Some notes

The conan home directory is currently under .pants.d, instead of ~ or ~/.cache/pants. I did this for the short term to make debugging cache problems and other issues as simple as a ./pants clean-all. I don't think the perf loss will be too bad (1-2 seconds).
Some string manipulation could be better done as regex, and I have TODOs for that
I am going to follow up with upstream Conan about getting a flag for cleaner client output so the parse method does not need to be so ugly.

@rules

This commit is a reimplementation of registering @rules for backends, because this PR began before that one was split off. add some simple examples to demonstrate how to use backend rules ...actually make the changes to consume backend rules in register.py revert accidental change to a test target remove extraneous log statement fix lint errors add native backend to release.sh isolate native toolchain path and hope for the best add config subdir to native backend really just some more attempts start trying to dispatch based on platform extend EngineInitializer to add more rules from a backend refactor Platform to use new methods in osutil.py refactor the native backend to be a real backend and expose rules register a rule in the python backend to get a setup.py environment make python_dist() tests pass make lint pass

stuhood · 2018-06-22T16:39:05Z

@CMLivingston : Thanks!

Before we begin reviewing this, would you mind squashing or rebasing it down to 1-5 commits (whatever looks reasonable rebase wise)?

CMLivingston · 2018-06-22T18:21:59Z

Done @stuhood

stuhood

Thanks!

stuhood · 2018-06-22T18:38:42Z

src/python/pants/backend/native/register.py

@@ -35,6 +38,7 @@ def build_file_aliases():
 def register_goals():
  # FIXME(#5962): register these under the 'compile' goal when we eliminate the product transitive
  # dependency from export -> compile.
+  task(name='native-third-party-prep', action=NativeThirdPartyPrep).install('native-compile')


In general, avoid increasing the size of existing goals... bias toward creating new goals for things that folks are not intended to run directly from the CLI.

This disagrees with some previous guidance (and it will temporarily mean a distracting number of goals listed in ./pants goals... unless we ended up with a facility for hiding them there?).

But grouping tasks together into goals (that users are not directly running) creates unnecessary dependencies between them. In this case: you can't run native-third-party-prep without also running shared-libraries.

I am confused here. It is piggybacking off the new native-compile goal created by Danny, right? And you would never run native-third-party-prep without running the compile and shared library tasks.

Unless a goal is meant to run from the commandline, goal names (and goals themselves, really) aren't useful. The reason to collect tasks together into a goal is because you want a human to be able to use that goal name on the CLI and force all tasks in the goal to run. Maybe that's the intent with native-compile, but most likely what you'd want them to run is ./pants compile, which will have a dependency on native-compile, and transitively on dep resolution and etc.

stuhood · 2018-06-22T18:39:20Z

src/python/pants/backend/native/subsystems/conan_prep.py

+log = logging.getLogger(__name__)
+
+
+class ConanPrep(Subsystem):


IMO, drop Prep from the name, and from the options scope.

stuhood · 2018-06-22T18:40:06Z

src/python/pants/backend/native/subsystems/conan_prep.py

+
+  def conan_requirements(self):
+    return (
+      'conan==1.4.4',


Would recommend putting all of these into a --requirements list option on your Subsystem, so that someone can override the version if they'd like by doing something like --conan-requirements='["conan==1.5.0", ..]'.

stuhood · 2018-06-22T18:42:00Z

src/python/pants/backend/native/subsystems/conan_prep.py

+      with safe_concurrent_creation(conan_pex_path) as safe_path:
+        builder = PEXBuilder(safe_path, pex_info=pex_info)
+        reqs = [PythonRequirement(req) for req in self.conan_requirements()]
+        interpreter = builder.interpreter


Is there a good reason to use a different interpreter here? If possible it would be good to minimize the differences between these two paths, so that you don't have a difference between the python used while we're bootstrapping vs the rest of the time.

So you are saying introduce a dependency on pyprep/interpreter selection?

No: in the other branch of this if/else, you use interpreter = PythonInterpreter.get().

Ah, that is what PEXBuilder is using under the hood. Will update to make more explicit. ✅

stuhood · 2018-06-22T18:48:57Z

src/python/pants/backend/native/tasks/link_shared_libraries.py

+  def _get_third_party_lib_args(self):
+    lib_args = []
+    tp_lib_tgts = self.context.targets(NativeThirdPartyPrep.native_library_constraint.satisfied_by)
+    if tp_lib_tgts:


Rather than making this conditional, you should probably update the task that produces the product to always produce the product (even if there are no targets).

stuhood · 2018-06-22T19:17:41Z

src/python/pants/backend/native/tasks/native_third_party_prep.py

+    if os.path.exists(include):
+      task_product['include'] = include
+
+    self.context.products.register_data(self.ThirdPartyLibraryFiles, task_product)


This usage of products confuses me... as far as I can tell, this method will fail if it is called for more than one target. But my understanding of the products API is somewhat limited to the JVM usecase, where we pass around an instance of an object, and mutate that object to add additional files.

IMO, making ThirdPartyLibraryFiles an actual class with an "add_include" method, etc, would be clearer. Then the only products method you ever used would be get_data.

3rdparty_files = self.context.products.get_data(self.ThirdPartyLibraryFiles, self.ThirdPartyLibraryFiles) 3rdparty_files.add_include(..)

IE, get the ThirdPartyLibraryFiles product, and initialize it with a constructor if it doesn't exist yet. Then, given the instance, add some files to it.

stuhood · 2018-06-22T19:18:52Z

src/python/pants/backend/native/tasks/native_third_party_prep.py

+  def ensure_conan_remote_configuration(self, conan_binary):
+    """
+    Ensure that the conan registry.txt file is sanitized and loaded with
+    a pants-specifc remote for package fetching.


stuhood · 2018-06-22T19:19:03Z

src/python/pants/backend/native/tasks/native_third_party_prep.py

+    Ensure that the conan registry.txt file is sanitized and loaded with
+    a pants-specifc remote for package fetching.
+
+    :param conan_binary: The conan client pex to use for manupulating registry.txt.


manipulating

stuhood · 2018-06-22T19:21:43Z

src/python/pants/backend/native/tasks/native_third_party_prep.py

+
+  def ensure_conan_remote_configuration(self, conan_binary):
+    """
+    Ensure that the conan registry.txt file is sanitized and loaded with


There is not much information on this file... is it supposed to be configured and committed in the repo?

If it is supposed to be committed in the repo, its locally should be passed in as config. If it's not supposed to be committed, then you should explicitly make sure it is located under the workdir (...and possibly also the vt.results_dir, if it's resolve-specific).

https://docs.conan.io/en/latest/reference/config_files/registry.txt.html doesn't make it very clear.

It is not intended to be committed, not resolve-specific, and is currently under the self.workdir of the task. This file registers remote storage locations and will be the same for every resolve, very similar to how we configure python-repos. ✅

stuhood · 2018-06-22T19:23:51Z

src/python/pants/backend/native/tasks/native_third_party_prep.py

+
+  def execute(self):
+    native_lib_tgts = self.context.targets(self.native_library_constraint.satisfied_by)
+    with self.invalidated(native_lib_tgts,


@wisechengyi: Can you work with Chris to talk through the cache key strategy here? It seems likely that this will need to do something similar to coursier.

… task

benjyw

This is super-awesome!

I have one naming nit: I'm not in love with the "third party" verbiage. True, we tend to use it in repos, but we don't really use it in pants itself. I'd prefer more technical terminology like "external", which is neutral regarding ownership of the code. E.g., if I use code I authored and published to conan, then it's not third-party in the legal/organizational sense, but it is still external in how it was fetched.

Apart from that, see some inline comments.

benjyw · 2018-06-22T23:54:23Z

src/python/pants/backend/native/subsystems/conan.py

+  def implementation_version(cls):
+    return super(Conan, cls).implementation_version() + [('Conan', 1)]
+
+  def bootstrap_conan(self):


This is neat - one day we should generalize it in a manner similar to JVMTools.

Agreed, this is really neat!

benjyw · 2018-06-22T23:58:36Z

src/python/pants/backend/native/tasks/native_third_party_fetch.py

+                                                               remote_url,
+                                                               '--insert'])
+    try:
+      stdout = subprocess.check_output(add_pants_conan_remote_cmdline.split())


What's the split() for? Doesn't Pex.cmdline return a list already?

That will return a string.

benjyw · 2018-06-23T00:00:53Z

src/python/pants/backend/native/tasks/native_third_party_fetch.py

+
+  def copy_package_contents_from_conan_dir(self, results_dir, conan_requirement, pkg_sha):
+    """
+    Copy the contents of the fetched pacakge into the results directory of the versioned


Typo: s/pacakge/package/

Can we symlink instead of copying? That's what we do for ivy-resolved jars (iirc they are symlinks from the .ivy2 cache). Or possibly that's the wrong thing to do? Either way, we should be consistent?

It would be consistent with Coursier and Ivy, so I will look into supporting that.

After further investigation, this may not be feasible for the current iteration and would probably be better for a follow up ticket.

The reason is that the fetch task aggregate the contents of subdirectories of each conan package into two "product" folders: include/ and lib/ under the results dir of the versioned target set. AFAICT, this would require symlinking recursively to every file in the dirs of each conan package from the data dir, and relying on the symlinking command to de-dupe. Based on my attempts to do this, I could not come up with a simple and effective solution. I am happy to hear suggestions though, but it may make sense to punt on this to unblock in the short term.

benjyw · 2018-06-23T00:01:39Z

src/python/pants/backend/native/tasks/native_third_party_fetch.py

+        # Invoke conan to pull package from remote.
+        try:
+          process = subprocess.Popen(
+            cmdline.split(),


Same q here re split().

Pex cmdline returns a string.

…s by using a list with the option, always product a task product even if it is empty

wisechengyi

Thanks Chris! Please see comments

wisechengyi · 2018-06-26T05:16:10Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+            cwd=vts_results_dir,
+            stdout=subprocess.PIPE
+          )
+        except OSError as e:


Any particular reason subprocess.Popen is used here instead of subprocess.check_output like other places?

wisechengyi · 2018-06-26T05:17:13Z

src/python/pants/engine/legacy/source_mapper.py

@@ -93,4 +93,3 @@ def iter_target_addresses_for_sources(self, sources):
      if (LegacyAddressMapper.any_is_declaring_file(legacy_address, sources_set) or
          self._owns_any_source(sources_set, hydrated_target)):
        yield legacy_address
-


leave this file untouched?

wisechengyi · 2018-06-26T05:21:56Z

testprojects/src/python/python_distribution/ctypes_with_third_party/BUILD

+)
+
+python_dist(
+  name='ctypes_with_third_party',


nit: rename to python_dist_with_third_party_c to be more descriptive?

wisechengyi · 2018-06-26T05:22:55Z

testprojects/src/python/python_distribution/ctypes_with_third_party/BUILD

+)
+
+python_binary(
+  name='bin_with_third_party',


similar nit: python_binary_with_third_party_c

wisechengyi · 2018-06-26T05:33:06Z

tests/python/pants_test/backend/python/tasks/test_native_external_library_fetch.py

+      rang/3.1.0@rang/stable: Installing package\n
+      Requirements\n    rang/3.1.0@rang/stable from 'pants-conan-remote'
+      \nPackages\n    rang/3.1.0@rang/stable:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9\n\n
+      rang/3.1.0@rang/stable: Already installed!\n"


non-blocking: would be worth exploring if there is more machine friendly output from conan, and possibly using conan as a library https://github.com/conan-io/conan

I will make a ticket to follow up with the conan team about this, and also to explore direct library usage for a later iteration.

If you've made a Conan ticket, could you link it in a comment next to where we parse the output?

cosmicexplorer

This is really great! I'm super excited we're getting so much closer to doing real work with the native backend. Wrapping Conan in a pex was a fantastic move and like benjy said, opens this up to further extension later. There are some errors (like not requiring the third party libs product in LinkSharedLibraries) and some points where we're doing a ton of unnecessary work (copying files from where Conan downloaded them to). I left several comments on regexes, and some comments about using datatype. Overall this is great, but especially those points I'd like to be addressed. If any of the changes seem to not make sense, let me know, but I would really like most of the changes to be made before merging, especially as a lot of them allow removing a good amount of code, and that will make it significantly easier to extend in the future.

cosmicexplorer · 2018-06-27T16:02:35Z

src/python/pants/backend/native/tasks/native_compile.py

@@ -55,6 +56,7 @@ class NativeCompile(NativeTask, AbstractClass):
  # `NativeCompile` will use `workunit_label` as the name of the workunit when executing the
  # compiler process. `workunit_label` must be set to a string.
  workunit_label = None
+  workunit_name = 'native-compile'


Is this used anywhere?

Yup, on line 278 in the NativeCompileError.

Oh, that was a mistake, it's supposed to be workunit_label. Could you make that change?

cosmicexplorer · 2018-06-27T16:08:52Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+
+  @staticmethod
+  def _build_conan_cmdline_args(pkg_spec, os_name=None):
+    os_name = os_name or get_normalized_os_name()


We should be using the Platform datatype from src/python/pants/backend/native/config/environment.py anywhere we do platform-specific logic -- it handles raising an exception and checking that we cover all possible platforms. Also, it looks like conan_os_opt will always be None in that error message? It seems like this method could be refactored to:

CONAN_OS_NAME = { 'darwin': lambda: 'Macos', 'linux': lambda: 'Linux', } @memoized_property def fetch_cmdline_args(self): platform = Platform.create() conan_os_name = platform.resolve_platform_specific(cls.CONAN_OS_NAME) args = ['install', self.pkg_spec, '-s', 'os={}'.format(conan_os_name)] return args

(adding an argument for the current platform, which is provided by the call in parse())

It also looks like this can be made into an instance method? I refactored the above to reflect that. conan_os_name (lowercase) could also be made into a memoized property.

cosmicexplorer · 2018-06-27T16:13:09Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+    pass
+
+  class NativeExternalLibraryFiles(object):
+    def __init__(self):


Making the property names lib and include seems to really obscure what they mean (lib could mean the library name, for example) -- is there a reason these aren't just lib_dir and include_dir? Will Conan always have a single include and lib dir?

No reason, and fair point. I will change it now. Yes conan will always have single include/lib dirs.

cosmicexplorer · 2018-06-27T16:14:20Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+      self._lib = None
+      self._lib_names = []
+
+    @property


Is there a reason we have explicit setters and getters? It seems like we could remove all of these and just do:

def __init__(self): self.include = None self.lib = None self.lib_names = []

cosmicexplorer · 2018-06-27T16:18:38Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+      args.extend(['-s', 'os=' + conan_os_opt])
+    return args
+
+  @classmethod


I would recommend calling this method create or from_conan_spec, or something else -- parse could mean any of a number of things. Since this method won't throw if we use platform.resolve_platform_specific() above, I would probably remove this method entirely and (in reference to nearby comments) remove the constructor as well, and make it a datatype object with just a 'pkg_spec' member.

cosmicexplorer · 2018-06-27T17:11:21Z

src/python/pants/backend/native/tasks/link_shared_libraries.py

+    if tp_files_product.lib_names:
+      for lib_name in tp_files_product.lib_names:
+        lib_args.append('-l{}'.format(lib_name))
+      lib_dir_arg = '-L{}'.format(tp_files_product.lib)


I think this logic (generating command-line arguments from lib and lib_names) should be moved to the NativeExternalLibraryFetch.NativeExternalLibraryFiles class. That way you could remove this method entirely.

cosmicexplorer · 2018-06-27T17:12:04Z

src/python/pants/backend/native/tasks/native_compile.py

@@ -185,10 +187,20 @@ def get_compiler(self):
  def _compiler(self):
    return self.get_compiler()

+  def get_third_party_include_dirs(self):
+    inc_dir = []
+    tp_files_product = self.context.products.get_data(NativeExternalLibraryFetch.NativeExternalLibraryFiles)


Again, it's usually much more clear to fetch products in the body of the execute() method, and if that is done this method can be removed entirely as well (making it into a separate method actually obscures the logic here a little).

cosmicexplorer · 2018-06-27T17:17:00Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+    self.fetch_cmdline_args = fetch_cmdline_args
+
+  def parse_conan_stdout_for_pkg_sha(self, stdout):
+    # TODO(cmlivingston): regex this


This should just be:

def parse_conan_stdout_for_pkg_sha(self, stdout): pkg_spec_escaped = re.escape(self.pkg_spec) pkg_spec_pattern = re.compile('^{}:(.*)$'.format(pkg_spec_escaped), flags=re.MULTILINE) return pkg_spec_pattern.search(stdout).group(1)

Also, Stu informed me on another PR that the TODO(name) style is a remnant of when Pants used to be a smaller project, and that a TODO or FIXME with a sufficient explanation inline or a link to an issue should be used instead at this point. I tend to use e.g. TODO(#5998) when the TODO would be fixed by that issue or PR, but that's purely something I made up -- all that's important is the issue link.

This is a pretty tricky output to parse and I am trying to get this merged asap so I will make a follow-up ticket to fix this.

Sounds like a plan!

cosmicexplorer · 2018-06-27T17:22:27Z

src/python/pants/backend/native/targets/external_native_library.py

+from pants.build_graph.target import Target
+
+
+class ExternalNativeLibrary(Target):


This is a good name for this target at this point in time! It may be worth revisiting later, along with the C/C++ library target names.

cosmicexplorer · 2018-06-27T17:34:32Z

src/python/pants/backend/native/tasks/native_compile.py

+    if directory:
+      inc_dir = [directory]
+    return inc_dir
+
  def _make_compile_request(self, versioned_target, dependencies):
    target = versioned_target.target
    include_dirs = [self._include_dirs_for_target(dep_tgt) for dep_tgt in dependencies]


Generally, if I'm mutating a variable, I isolate that along with the initialization of the variable and surround it by at least one line of whitespace on both sides, a habit I learned the hard way, and I would recommend doing that here and anywhere else this occurs.

…/ctypes-test-project-third-party

…pex cmdline list

cosmicexplorer

Great! We should really break out a ticket about making sure we only add the thirdparty libs we actually depend on in each target (respecting strict_deps) to compile and link command lines but once that's done this should be good to go.

cosmicexplorer · 2018-07-17T22:18:59Z

src/python/pants/backend/native/subsystems/conan.py

+    if os.path.exists(conan_pex_path):
+      conan_binary = WrappedPEX(PEX(conan_pex_path, interpreter))
+      return self.ConanBinary(pex=conan_binary)
+    else:  


This line has trailing whitespace.

cosmicexplorer · 2018-07-17T22:26:12Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+  }
+
+  def parse_conan_stdout_for_pkg_sha(self, stdout):
+    # TODO(cmlivingston): regex this


This should have its own ticket, see the comments here.

This is fixed now.

cosmicexplorer · 2018-07-17T22:29:32Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+    return pkg_sha
+
+  @memoized_property
+  def directory_path(self):


This should probably have a comment linking to either a description of the package spec or a description of the conan directory layout -- it's not clear why this works currently.

cosmicexplorer · 2018-07-17T22:30:52Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+
+  def _prepare_vts_results_dir(self, vts):
+    """
+    Given a `VergetTargetSet`, prepare its results dir.


cosmicexplorer · 2018-07-17T22:32:05Z

src/python/pants/backend/native/tasks/native_external_library_fetch.py

+    safe_mkdir(vt_set_results_dir)
+    return vt_set_results_dir
+
+  def _populate_task_product(self, results_dir, task_product):


Ah, that's kind of silly. This is great then.

cosmicexplorer · 2018-07-17T22:37:06Z

tests/python/pants_test/util/test_dirutil.py

-                                longest_dir_prefix, mergetree, read_file, relative_symlink, relativize_paths,
-                                rm_rf, safe_concurrent_creation, safe_file_dump, safe_mkdir, safe_mkdtemp,
-                                safe_open, safe_rm_oldest_items_in_dir, safe_rmtree, touch)
+                                absolute_symlink, check_no_overlapping_paths, fast_relpath,


Can this be reverted? Is travis lint erroring out on this?

My local pre-commit hook bombed out due to import order.

cosmicexplorer · 2018-07-17T22:38:00Z

src/python/pants/backend/native/subsystems/native_toolchain.py

@@ -71,39 +71,44 @@ def select_linker(platform, native_toolchain):
  #   'darwin': lambda: Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools),
  #   'linux': lambda: Get(Linker, Binutils, native_toolchain._binutils),
  # })
+  #


This whole file needs much better documentation. I will do that as part of #5951.

cosmicexplorer · 2018-07-17T22:39:09Z

src/python/pants/backend/native/tasks/cpp_compile.py

@@ -41,6 +41,16 @@ def get_compile_settings(self):
  def get_compiler(self):
    return self._request_single(CppCompiler, self._toolchain)

+  def _make_compile_argv(self, compile_request):
+    # FIXME: this is a temporary fix, do not do any of this kind of introspection.


Could you please add a link to #5951 here?

cosmicexplorer · 2018-07-17T22:44:17Z

src/python/pants/backend/native/tasks/link_shared_libraries.py

        else:
          link_request = self._make_link_request(
-            vt, compiled_objects_product, native_target_deps_product)
+            vt, compiled_objects_product, native_target_deps_product, external_libs_product)


If I'm understanding this correctly, all thirdparty libs are exposed to all targets being compiled, which doesn't seem like it's what we want, and we should break out a ticket to only expose the thirdparty targets each C or C++ target depends on.

cosmicexplorer · 2018-07-17T22:48:25Z

tests/python/pants_test/backend/python/tasks/test_native_external_library_fetch.py

+      rang/3.1.0@rang/stable: Installing package\n
+      Requirements\n    rang/3.1.0@rang/stable from 'pants-conan-remote'
+      \nPackages\n    rang/3.1.0@rang/stable:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9\n\n
+      rang/3.1.0@rang/stable: Already installed!\n"


If you've made a Conan ticket, could you link it in a comment next to where we parse the output?

CMLivingston · 2018-07-18T17:56:49Z

ping @stuhood for final review

stuhood

This looks great. Thanks for iterating Chris!

stuhood · 2018-07-18T18:09:41Z

src/python/pants/backend/native/tasks/link_shared_libraries.py


    all_shared_libs_by_name = {}

+    # FIXME: convert this to a v2 engine dependency injection.


Or just to a constant/global, frankly. It's not going to change, regardless of options or anything else.

CMLivingston mentioned this pull request Jun 21, 2018

Let plugins declare @rules and move the native backend into contrib #5970

Closed

CMLivingston force-pushed the clivingston/ctypes-test-project-third-party branch 2 times, most recently from 163063d to 50d12ba Compare June 22, 2018 17:54

Fix git history...v2

91641b7

CMLivingston force-pushed the clivingston/ctypes-test-project-third-party branch from 50d12ba to 91641b7 Compare June 22, 2018 18:07

Merge with master

5898ce8

Fix broken int test

cb79337

stuhood requested review from stuhood, benjyw and cosmicexplorer June 22, 2018 18:30

stuhood reviewed Jun 22, 2018

View reviewed changes

CMLivingston force-pushed the clivingston/ctypes-test-project-third-party branch from eff38de to cb79337 Compare June 22, 2018 21:04

Chris Livingston added 3 commits June 22, 2018 14:37

Add workaround for linux machines

41067f8

Add a conan requirement class, refactor conan subsystem, rename fetch…

70cb412

… task

Make use of PythonInterpreter.get() explicit

3ea6586

benjyw reviewed Jun 23, 2018

View reviewed changes

Chris Livingston added 4 commits June 25, 2018 12:37

Refactor task product into its own class, allow multiple conan remote…

afb45c8

…s by using a list with the option, always product a task product even if it is empty

Rename third party verbiage

8bc0e29

Add a thirdparty project that links a library file

1dab9c0

Return linker settings to normal

8631b9c

wisechengyi reviewed Jun 26, 2018

View reviewed changes

Chris Livingston added 5 commits June 26, 2018 08:39

Add __dso_handle workaround to attempt to pass CI

6a95c66

Add stdlib c++ to linker commandline for debugging

d6dde33

Refactor names

932a8af

Remove all usages of std lib in cpp examples to pass travis CI

bc50b16

Remove cruft

bab73bb

cosmicexplorer requested changes Jun 27, 2018

View reviewed changes

Chris Livingston added 2 commits July 10, 2018 14:46

Travis debug 3

05c2cf7

Dummy change to refresh test sources in CI

40b60bb

CMLivingston mentioned this pull request Jul 11, 2018

[WIP] Conan Support Experimental PR #6093

Closed

cosmicexplorer and others added 11 commits July 12, 2018 02:28

just try

1bf69da

Merge branch 'master' into native-thirdparty-passes-unit-tests

ee0fa06

revert changes to llvm.py and test_native_toolchain.py

0047140

add back g++ hack

7fc20ac

Change int test

529f58c

Merge branch 'master' of github.com:pantsbuild/pants into clivingston…

0818064

…/ctypes-test-project-third-party

Add cmdline slicing for conan calls due to new WrappedPex functionality

5803352

Remove pdb statement

5fcc69b

Merge branch 'master' of github.com:pantsbuild/pants into clivingston…

abdffa6

…/ctypes-test-project-third-party

Dummy change to trigger CI

e0e772a

Fix osx test failure

8d028c0

cosmicexplorer mentioned this pull request Jul 16, 2018

use ParseSearchDirs to find include dirs in the XCodeCLITools and LLVM subsystems #6143

Closed

Chris Livingston added 2 commits July 16, 2018 14:39

Merge branch 'master' of github.com:pantsbuild/pants into clivingston…

cf64653

…/ctypes-test-project-third-party

Add github issue link to fixme in llvm.py

51d4829

cosmicexplorer mentioned this pull request Jul 17, 2018

make sure clang++ is using libstdc++ on linux, not its own libc++ #6151

Closed

Add Danny's patch to fix broken unit tests

b7b5916

CMLivingston mentioned this pull request Jul 17, 2018

Improve Conan stdout parsing #6168

Closed

Add github issue link and comment on why it makes sense to slice the …

4a8bedb

…pex cmdline list

cosmicexplorer approved these changes Jul 17, 2018

View reviewed changes

Chris Livingston added 3 commits July 17, 2018 16:52

Cosmicexplorer final comments

ea4c701

Add more extensive testing

5dbaea4

Add 1 issue link

84be507

stuhood approved these changes Jul 18, 2018

View reviewed changes

cosmicexplorer merged commit 4aec12c into pantsbuild:master Jul 18, 2018

This was referenced Jul 19, 2018

Add basic native task unit tests. #6179

Merged

Expose C++ 3rdparty libraries from Conan only to the targets that depend on the 3rdparty library #6178

Closed

		log = logging.getLogger(__name__)


		class ConanPrep(Subsystem):

		from pants.build_graph.target import Target


		class ExternalNativeLibrary(Target):


		all_shared_libs_by_name = {}

		# FIXME: convert this to a v2 engine dependency injection.

Conan (third party) support for ctypes native libraries #5998

Conan (third party) support for ctypes native libraries #5998

Conversation

CMLivingston commented Jun 21, 2018 • edited Loading

Problem

Solution

Result

Some notes

stuhood commented Jun 22, 2018

CMLivingston commented Jun 22, 2018

stuhood left a comment

Choose a reason for hiding this comment

stuhood Jun 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CMLivingston Jun 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjyw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CMLivingston Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CMLivingston Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wisechengyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cosmicexplorer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CMLivingston commented Jun 21, 2018 •

edited

Loading

stuhood Jun 22, 2018 •

edited

Loading

CMLivingston Jun 22, 2018 •

edited

Loading

CMLivingston Jun 25, 2018 •

edited

Loading

CMLivingston Jun 25, 2018 •

edited

Loading

cosmicexplorer Jun 27, 2018 •

edited

Loading