combine Clang, GCC, Binutils, and XCodeCLITools to form the NativeToolchain subsystem and consume it in BuildLocalPythonDistributions for native code #5490

cosmicexplorer · 2018-02-20T22:40:07Z

Problem

python_dist() allows the use of native extension modules, but it currently uses whatever compiler, linker, libraries, etc that distutils can find first on the host to compile the native code. Providing these disparate elements of the native code compilation toolchain in Pants means users don't have to install it themselves, and Pants can ensure that the provided compiler supports whatever features we want to provide to developers of native extension modules with python_dist().

Solution

This PR uses the LLVM subsystem defined in #5471 (now called Clang) along with multiple other subsystems to compile native code in a setup.py-based project declared through a python_dist() target.

Result

We provide clang and gcc for all platforms, binutils on Linux to link, and we wrap the XCode command line developer tools to link on OSX. When a python_dist() target is built through the BuildLocalPythonDistributions task, it pulls in the bootstrapped toolchain and prepends it to the PATH so that our provided compiler is preferred (in addition to scrubbing CC and CXX). True sandboxed execution is preferable, but for now this at least guarantees that the provided compiler is used, which satisfies the goals stated at the top.

stuhood

This looks awesome. Thanks @cosmicexplorer! Adding Kris and Chris to take a look.

stuhood · 2018-02-20T22:52:41Z

src/python/pants/backend/python/sandboxed_interpreter.py

+from pants.util.memo import memoized_method
+
+
+class SandboxedInterpreter(PythonInterpreter):


Some pydocs would be good here.

Agreed, especially given the discussion over whether this is the right way to do this.

stuhood · 2018-02-20T22:55:48Z

src/python/pants/backend/python/sandboxed_interpreter.py

+    # will attempt to build a fat binary for 32- and 64-bit archs, which makes
+    # clang invoke "lipo", an osx command which does not appear to be open
+    # source. see Lib/distutils/sysconfig.py and Lib/_osx_support.py in CPython.
+    sanitized_env['ARCHFLAGS'] = '-arch x86_64'


Maybe I'm not understanding which things will run in this sandbox, but this seems specific to a particular usage of this class (for invoking setup.py). Would it be better for build_local_python_distributions.py to be able to set additional env vars?

I actually had the contents of sandboxed_interpreter.py as a private contextmanager method of BuildLocalPythonDistributions and I think I thought that decoupling the "sandbox" environment from the task that directly consumes it was a useful goal, but it's not clear to me how useful that is now. It's easy enough to move back and thinking about it again, I don't know if I really like the idea of overriding an @classmethod with an instance method at all (which is what's happening with sanitized_environment()). I think it's probably better to put it back in the BuildLocalPythonDistributions task.

Thinking about it again, it's likely because I wanted to take advantage of pex's battle scars, and thought that subclassing the PythonInterpreter class in this way would do that cleanly. That being said, the @classmethod turning into an instance method is probably not a good idea at all (we're relying on it getting invoked as self.sanitized_environment() instead of cls.sanitized_environment(), unless we change the pex code to just be an instance method as well (which might not be a bad idea?). I can confirm that it's easy to make it into a contextmanager private method, though.

One way of setting necessary env variables temporarily could be using environment_as from contextutil as I see you have imported below. I may be missing context here, but an alternative to PATH prepending could be to set CC and CXX (and any other env vars) using an environment_as wrapper around

setup_runner = SetupPyRunner(dist_target_dir, 'bdist_wheel', interpreter=sandboxed_interpreter) setup_runner.run()

Just a thought though, it seems like there are a few ways to go about this.

I think that not baking the sandboxing into pex would be good... python/setup.py invokes will not be the only place we want to use it. So yea, environment_as would be more general, and more portable to remote execution.

Yep, that's what I was thinking -- a contextmanager private method would be applying a few levels of contexts from contextutil and then yielding. The reasons I liked doing that in the interpreter subclass are related to the utilities the pex interpreter class already provides, but if that's not significant there's no reason to do it that way.

(Sorry, I keep forgetting to refresh the page) Ok, there's no hidden significance from my end behind tying this to the pex interpreter. I can push up a change today. Thanks @stuhood and @CMLivingston for the discussion.

UnrememberMe

Overall, this looks good. I suggest instead of be strict and throw an exception when llvm subsystem is missing, we simply output some warnings and use the system provided toolchain. We can error out later if the system does not provide toolchain. This is likely the case for MacOS anyway.

UnrememberMe · 2018-02-21T00:47:58Z

src/python/pants/backend/python/sandboxed_interpreter.py

+    # use our compiler at the front of the path
+    # TODO: when we provide ld and stdlib headers, don't add the original path
+    sanitized_env['PATH'] = ':'.join([
+      os.path.join(self._llvm_base_dir, 'bin'),


We need to make sure the packaged clang shadows gcc and g++ as well.

I don't understand this -- if a tool is looking for gcc or g++ specifically it's probably a better idea to understand why and then modify it to not do that instead of giving them clang, that seems like it could produce at worst subtly broken binaries and at best errors for arguments clang doesn't support.

UnrememberMe · 2018-02-21T00:50:11Z

src/python/pants/backend/python/sandboxed_interpreter.py

+  def __init__(self, llvm_base_dir, base_interp):
+
+    if not os.path.isdir(llvm_base_dir):
+      raise ToolchainLocationError(llvm_base_dir)


Here and below (env scrub) are assuming that we can provide packaged toolchains. Suggest to remove the errors and just emit some kind of warning when we have to fail back to use toolchains provided by the environment.

Would it be possible for this to cause reproducibility issues across devel and prod environments?

AFAIK, this failure would mean something other than "we don't have a toolchain for your OS". It would mean something like: "we tried downloading it and partially succeeded, but then someone maybe deleted some of it".

See below comment in the main PR thread.

And yes, @stuhood is correct!

UnrememberMe · 2018-02-21T00:57:26Z

src/python/pants/backend/python/tasks/build_local_python_distributions.py

    # Copy sources and setup.py over to vt results directory for packaging.
    # NB: The directory structure of the destination directory needs to match 1:1
    # with the directory structure that setup.py expects.
-    for src_relative_to_target_base in dist_tgt.sources_relative_to_target_base():
+    all_sources = list(dist_tgt.sources_relative_to_target_base())


what's the reason for this change? It seems like there is real difference here. Do I miss something?

Can confirm that dist_tgt.sources_relative_to_target_base() is a <class 'pants.source.wrapped_globs.EagerFilesetWithSpec'>, which stores file data in lists/tuples. Did you bump into any issues with this specifically?

It's an artifact of previous changes I made before reducing this diff to make it mergeable. It doesn't need to be there, the existing code obviously works. I just don't like function calls in the declaration of a for loop, it feels hard to read. The list() bit was from a previous iteration of the code where I was adding a list and a tuple together.

UnrememberMe · 2018-02-21T01:00:15Z

src/python/pants/backend/python/tasks/build_local_python_distributions.py

-  def _create_dist(self, dist_tgt, dist_target_dir):
-    """Create a .whl file for the specified python_distribution target."""
-    interpreter = self.context.products.get_data(PythonInterpreter)
+          for dependent in self.context.build_graph.dependents_of(vt.target.address):


This is same with existing codes except with build_graph is not stored in a local variables, right? Is there a reason for this change? I am afraid that I am missing something.

Yes, it's the same. Since the build graph is just a property access making it into a separate variable was a bit confusing to me. It doesn't affect the logic.

CMLivingston

Thanks, looks good! Added a couple follow-up comments.

cosmicexplorer · 2018-02-21T19:28:57Z

I suggest instead of be strict and throw an exception when llvm subsystem is missing, we simply output some warnings and use the system provided toolchain.

@UnrememberMe The existing cpp contrib basically does the fallback you describe, and we're replacing it because as @CMLivingston mentioned above, controlling the toolchain allows us to reliably orchestrate complex things such as native code in setup.py-based projects -- I think relinquishing that would be defeating the purpose. The reason packaging the toolchain (separately from its consumption in Pants) is nontrivial is because it needs to work across all of Pants's supported platforms reliably, so that we don't need to provide a fallback, in my understanding.

cosmicexplorer · 2018-02-21T19:50:06Z

Thanks for the discussion, check out BuildLocalPythonDistributions#_sandboxed_setuppy() in 1ed1fdb. If this is better moved to setup_py.py somewhere, let me know (that could also be done later if we need to modify the setup.py runner environment for another task at some point).

UnrememberMe · 2018-02-21T22:22:07Z

After discussion with @cosmicexplorer, I am totally ok with the current implementation.

cosmicexplorer · 2018-03-21T22:17:39Z

I just pushed up the current iteration of Pants consuming the native code toolchain I'm developing so that we can discuss real code. It has no tests now, which is going to change very soon. I think the ExecutionEnvironmentMixin is a step in the right direction before we can get into composable v2 Snapshots, etc. I think separating the individual tools in the toolchain (gcc, clang, binutils) from their usage with something like the contextmanager here is the easiest way to avoid having to put on band-aids after the fact. I left a couple longish comments intentionally, those will absolutely be removed when this is merged. This also maybe shouldn't be considered a WIP, but it's also definitely not the way anything has to be if this implementation causes problems I haven't considered. Please let me know if it could/should be improved.

Also, as I build out testing for this toolchain now it might end up being much easier to have (on Linux) gcc, clang, and ld all be installed into a single subdirectory of ~/.cache/pants, and merging the bin/, include/, lib/ etc of each. Not only does this avoid having to specify e.g. include and lib directories when we invoke the compiler, it would also then be much easier to try out compiling locally in a literal chroot -- I'm not sure that's a goal for any of us but it would definitely make it easier to avoid environment pollution spilling incompatible versions of glibc into your binaries, for example.

cosmicexplorer · 2018-03-21T23:26:52Z

Also, made a PR against pantsbuild/binaries for the packaging scripts for gcc, clang, and binutils, which are the only reason this PR can exist.

cosmicexplorer

To make the history a little bit more useful, I squashed everything into 3c3c7d2, because this isn't a huge change.

I'd like to get some feedback for the future, though (so I should probably make the rest of this into a separate issue in this repo?): the current subsystems should work for the purposes of this PR if we only compile with gcc, which is what we are doing now, but I think there may be some non-trivial work after this is merged to make the native toolchain something Pants developers and plugin developers can use effectively.

Currently every implementor of ExecutionPathEnvironmentis a NativeTool subclass providing a tgz archive, and just adds their bin/ directory to the path. However, just adding the bin dir to the path isn't enough: compilers (gcc and clang) in specific rely heavily on searching paths relative to the executable location for the standard library and more -- these are either installed alongside the compiler with make install, or assumed to already exist on the host. Right now invoking the provided clang fails, because of resources including headers and some object files that it requires from a gcc installation -- this can be overridden on a case-by-case basis when the compiler is invoked e.g. by adding include directories with -I<dir>, but manually recreating and maintaining the structure of the install paths in Pants seems brittle and a waste of time.

If that makes sense, I can see at least two ways to to make clang usable:

Install every part of the native toolchain into a single subdirectory of ~/.cache/pants, which would then have e.g. a 'bin' directory with clang, gcc, and ld at once. However, I'm concerned that any extensions of such a native toolchain e.g. in an internal Pants plugin could then be deeply tied to undeclared assumptions about this environment that would seem to make it extremely hard to signal for deprecated uses if we need to make any changes in the future. The most obvious workaround, making multiple copies of these toolchains in separate dirs, may take up tons of disk space very fast, and I don't think this is a good idea.
Make it possible to compose tools as if they are installed at the same prefix dir without colocating them in a single physical directory. A docker container might exactly address this for Linux, but would not be able to produce native code runnable on OSX.

However, in the vein of (2), a FUSE mount instead might not be absurd at all -- see this complete example code for a basic FUSE fs using one rust FUSE library that supports both OSX and Linux. I can see this being integrated into Pants somewhere around where we execute processes locally in rust really naturally -- the scheduler could accept an ExecuteProcessRequest which includes a request for e.g. gcc and ld to be available, then create a FUSE mountpoint which can virtualize the first suggestion (just installing the whole native toolchain into the same prefix directory). This should be pretty lower-overhead as FUSE goes since it's just doing simple filesystem operations under the hood, then modifying the PATH or even doing a literal chroot from that mountpoint before invoking the process. I believe this would require some installation on the pants user's part beforehand, but I don't feel like that should be a blocker (see compatibility for the aforementioned rust library).

I can investigate how easy it would be to do something like that in Rust (after this is merged) and show a proof of concept if that approach seems promising. I don't see how we can continue to concatenate bin dirs into the PATH because of e.g. the clang issue mentioned above without a lot of very brittle code, I don't see an immediate workaround that would allow us to package and invoke our toolchain in a way each tool supports, and I think such a FUSE mount wouldn't be too hard to make a proof of concept for, because they provide minimal example code and are already in rust. Let me know if I'm wrong about any/all of that!

cosmicexplorer · 2018-03-21T23:39:17Z

build-support/docker/centos6/Dockerfile

+# GCC requires these to compile.
+RUN yum install -y \
+  m4 \
+  wget



@stuhood I know we talked about keeping this image bare bone-ish -- (when you have time) let me know if we should hold off on this change for now.

It seems like this can easily be accomplished manually in the script that builds the binary, so I don't think this should be in the image.

Got it! Thanks -- I don't remember but you may have said that the last time I asked too...

baroquebobcat

Did a quick look. I'd like to see

tests (I see that's still a TODO in your PR title)
docstrings on the new subsystems

I don't have much context around the specifics, but I appreciate your comments explaining how you're thinking about things.

stuhood · 2018-03-26T21:42:19Z

I can investigate how easy it would be to do something like that in Rust (after this is merged) and show a proof of concept if that approach seems promising. I don't see how we can continue to concatenate bin dirs into the PATH because of e.g. the clang issue mentioned above without a lot of very brittle code, I don't see an immediate workaround that would allow us to package and invoke our toolchain in a way each tool supports, and I think such a FUSE mount wouldn't be too hard to make a proof of concept for, because they provide minimal example code and are already in rust. Let me know if I'm wrong about any/all of that!

I think that this is in general related to how we will compose Snapshots in the future, which is covered by #5502. So it's blocking both remoting and our goal of porting an entire pipeline to the v2 engine. cc @illicitonion , @kwlzn

If you could attach any thoughts you have on the matter to that ticket, that would be good.

stuhood

Looks good in general, although the biggest thing I'd like to see is an explanation inline in the code of the implications of not prepending anything on OSX. And maybe a ticket to followup for that.

stuhood · 2018-03-26T21:46:00Z

build-support/docker/centos6/Dockerfile

+# GCC requires these to compile.
+RUN yum install -y \
+  m4 \
+  wget



It seems like this can easily be accomplished manually in the script that builds the binary, so I don't think this should be in the image.

stuhood · 2018-03-26T21:47:11Z

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

+
+  PLATFORM_SPECIFIC_TOOLCHAINS = {
+    # TODO(cosmicexplorer): 'darwin' should have everything here, but there's no
+    # open-source linker for OSX.


This comment should be expanded to explain the impact of leaving this list empty.

Does the comment I added along with the class's docstring make the multi-platform strategy clear?

stuhood · 2018-03-26T21:47:19Z

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

+
+  options_scope = 'native-toolchain'
+
+  PLATFORM_SPECIFIC_TOOLCHAINS = {


Should be a private field most likely?

Absolutely, see the current version.

stuhood · 2018-03-26T21:48:38Z

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

+  @classmethod
+  def _get_platform_specific_toolchains(cls):
+    normed_os_name = normalize_os_name(get_os_name())
+    return cls.PLATFORM_SPECIFIC_TOOLCHAINS[normed_os_name]


Should call cls.PLATFORM_SPECIFIC_TOOLCHAINS.get here and handle the None case, to account for the fact that normalize_os_name will likely change in the future to add more OSes.

Done, see the current version.

stuhood · 2018-03-26T21:49:38Z

src/python/pants/backend/python/subsystems/python_dist_build_environment.py

+    # in the CPython source) instead of hoping setup.py knows what to do. The
+    # default UnixCCompiler from distutils will build a 32/64-bit "fat binary"
+    # unless you use their undocumented ARCHFLAGS env var, and there may be more
+    # dragons later on.


Does this need a TODO and/or a github issue?

Yes, and I will make one presently.

stuhood · 2018-03-26T21:50:45Z

src/python/pants/backend/python/tasks/build_local_python_distributions.py

+  def subsystem_dependencies(cls):
+    return super(BuildLocalPythonDistributions, cls).subsystem_dependencies() + (PythonDistBuildEnvironment.scoped(cls),)
+
+  # TODO: seriously consider subclassing UnixCCompiler as well as the build_ext


This is the second reference to this strategy. Probably justifies a github issue to link to in both places.

I will make an issue describing this -- I've currently just concatenated the comments but will make an issue and just link to that in a comment.

stuhood · 2018-03-26T21:53:11Z

src/python/pants/backend/python/tasks/build_local_python_distributions.py

+
+  def _create_dist(self, dist_tgt, dist_target_dir, interpreter):
+    """Create a .whl file for the specified python_distribution target."""
+    self.context.log.debug("dist_target_dir: '{}'".format(dist_target_dir))


This would likely be more useful if it included the target as well.... or could remove it.

Removed, I remember adding this line many moons ago to debug what ended up being this closed PR.

stuhood · 2018-03-26T21:57:07Z

src/python/pants/binaries/execution_environment_mixin.py

+  # chroot, or VM image, or something might be really interesting to just
+  # completely sidestep the installation problem.
+  @contextmanager
+  def execution_environment(self, prev_env=None):


It's not clear that this needs to be a member of a mixin. Would it be more complicated as a static method next to environment_as... ie, environment_path_as, with an argument for the additional path entries?

The idea was to abstract over subsystems that provide path entries, because NativeToolchain pulls in either XCodeCLITools or Binutils, depending on the platform, and the same code should consume either one. A mixin seems appropriate for that -- but having the mixin handle the environment and everything seems way overboard. I've changed this to an extremely simple mixin named ExecutablePathProvider instead, and consumers such as BuildLocalPythonDistributions can do any environment modifications themselves (I've also moved the path modification utility method to contextutil). Let me know if that addresses your concern, or if I should be abstracting in some other way.

illicitonion · 2018-03-27T13:14:03Z

I swear I left a comment on this PR last week, but I can't see it, so I guess I didn't...

I don't feel like I have a good enough understanding of the dependency from clang to gcc to be able to talk about the $PATH/merged-path/FUSE question. Could you put together an example (ideally a test) which fails if only clang is present, so I can see what from gcc is actually necessary?

From what you've described, it sounds like what clang needs from a gcc install is just a few directories of headers, and to specify a few -I flags which point into those directories. Does that sound like a reasonable characterisation? If so, my recommendation would be that we decompose the gcc "binary" into two separate artifacts; a "standard library headers" tar (which perhaps includes a file with a bunch of -I directives in it that can be sourced) and a "binaries and stuff" tar which contains the things clang doesn't need.

It definitely seems bad to need a full gcc in order to use a clang.

But it's possible there's a deeper interlinking here which I can't see, so an example would be handy :)

This is the first instance in Pants of selectively instantiating a subsystem based on the platform Pants is being invoked on, because gnu binutils (or at least, ld) doesn't support OSX. The XCode cli tools will be relied on instead for linking (more work needed here on effective error messages). Create by squashing 119 commits, most of which had been merged into master: many of these commits were merged with the first python_dist PR, or the PR that introduced the "LLVM" subsystem (now named "Clang"). Squashed commit of the following: commit 9ae7284 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Mar 21 15:03:38 2018 -0700 add some more comments to explain what i'm thinking commit 8d48d0a Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Mar 21 14:52:07 2018 -0700 some mild refactoring and more documentation commit 29e9622 Merge: ba727b7 a5410b6 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Mar 21 12:52:28 2018 -0700 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit ba727b7 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Mar 20 02:10:40 2018 -0700 gcc is linux specific again (but only for a little bit) commit aaae18c Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Mar 16 15:26:48 2018 -0700 correct gcc to non-platform specific, but remove from osx for now commit dfe7340 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Mar 16 14:38:39 2018 -0700 add platform-specific native toolchain and use to build python dists commit f06c83f Merge: f22dfca f9ee6fd Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Mar 16 12:20:11 2018 -0700 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit f22dfca Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Mar 7 16:27:59 2018 -0800 add fixme commit 7d76e70 Merge: a35256d d77483e Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Mar 2 15:47:08 2018 -0800 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit a35256d Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Mar 1 11:51:29 2018 -0800 update name of `compiler` package to `clang` commit 529324e Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 22 12:07:52 2018 -0800 fix lint errors commit 7763e5d Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 22 11:46:58 2018 -0800 rename LLVM -> Compiler commit 91e1353 Merge: 6a68876 1ed1fdb Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 22 11:28:16 2018 -0800 Merge branch 'dmcclanahan/python-dist-c++-sources' of github.com:cosmicexplorer/pants into dmcclanahan/python-dist-c++-sources commit 6a68876 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 21 11:45:30 2018 -0800 refactor out unnecessary interpreter wrapper to use a contextmanager commit 1ed1fdb Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 21 11:45:30 2018 -0800 refactor out unnecessary interpreter wrapper to use a contextmanager commit 41a1a9a Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 20 14:17:23 2018 -0800 revert unnecessary python_dist changes commit 1cfd60a Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 20 14:12:44 2018 -0800 make final simplifications commit a4ebbbd Merge: 7783678 71a33d6 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 20 13:57:19 2018 -0800 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit 7783678 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 20 13:45:38 2018 -0800 remove more implementation artifacts commit 88851f1 Merge: c7c90b2 c014e8d Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 20 10:44:14 2018 -0800 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit c7c90b2 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Feb 16 07:20:37 2018 -0800 refactor unnecessary complexity commit 35899ee Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 15 05:47:49 2018 -0800 try to understand what makes the breaking change commit d9fc2c4 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 14 13:55:02 2018 -0800 remove comments in sandboxed_interpreter.py commit ceae8bb Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 14 13:51:46 2018 -0800 slim down the pull request quite a bit commit 48d7a93 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 14 13:27:13 2018 -0800 rewrite BinaryTool a little and remove native toolchain subsystem commit cfe5c86 Merge: b1d39ec f55260a Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Feb 14 08:34:39 2018 -0800 Merge branch 'master' of github.com:pantsbuild/pants into dmcclanahan/python-dist-c++-sources commit b1d39ec Merge: 55dbd7e 7cdea9a Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue Feb 13 11:54:50 2018 -0800 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit 55dbd7e Author: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon Feb 12 19:08:59 2018 -0800 suddenly, with very few changes, everything "just works"! commit 608769e Merge: badd80f 569f14c Author: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon Feb 12 19:05:19 2018 -0800 Merge branch 'master' into dmcclanahan/python-dist-c++-sources commit badd80f Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon Feb 5 16:09:46 2018 -0800 add llvm distribution support, not just clang commit 0ac2bbe Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Feb 2 16:33:37 2018 -0800 remove source copying contextmanager and set clang arch for setup.py commit 6c52529 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Feb 2 12:16:55 2018 -0800 try cpp module sources and comment out future work commit f33677b Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri Feb 2 11:39:58 2018 -0800 add some more context, leave everything in an inconsistent state commit 5a6ad48 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 1 12:07:56 2018 -0800 plumb in a native toolchain subsystem commit 5ed218e Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu Feb 1 11:11:10 2018 -0800 use a contextmanager to copy source files and edit PYTHONPATH commit 3155138 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Jan 31 20:47:16 2018 -0800 now we can declare cpp_sources in python_dist targets! commit 2591288 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Jan 31 03:17:57 2018 -0800 cut off work for now commit 629ff46 Author: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed Jan 31 01:02:38 2018 -0800 clean up PythonDistribution and add c_sources field commit 9e3d2ca Author: Chris Livingston <clivingston@twitter.com> Date: Tue Jan 30 14:36:45 2018 -0800 Remove mod to travis yml commit 1a04e80 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Jan 30 13:13:37 2018 -0800 Rebase with master commit b4d367c Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 24 17:59:30 2018 -0800 Rename superhello to fasthello commit 59164c1 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 24 15:26:03 2018 -0800 Add remove command to travis.yml to remove problematic file from failing CI target commit 8ac832b Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 24 13:00:12 2018 -0800 Add clean-all statements to integration tests to gauge flakiness commit a787717 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 24 10:39:22 2018 -0800 Slightly modify test assertion for conflicting deps test commit 0b82067 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Jan 23 15:02:47 2018 -0800 Resolve merge conflicts from removal of tasks2 commit 7f757c0 Author: Chris Livingston <clivingston@twitter.com> Date: Mon Jan 22 12:28:18 2018 -0800 Disallow dependencies on a python_dist target commit 3024a34 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Jan 19 11:17:45 2018 -0800 Add xfail test to testprojects tests commit eb8598d Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 18 17:26:24 2018 -0800 Fix lint commit 654c20e Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 18 14:13:42 2018 -0800 Remove unused import commit 22b72f0 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 18 11:39:49 2018 -0800 Remove add_labels from PythonDistribution object commit a4150d0 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 17 19:17:59 2018 -0800 Remove duplicate functions to enforce DRY commit 178bcbd Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 17 14:21:47 2018 -0800 Fix issues with CI and failing testprojects target commit cae065c Author: Chris Livingston <clivingston@twitter.com> Date: Mon Jan 15 23:44:32 2018 -0800 Add integration tests for targets that conflict with transitive deps listed in the install_requires field of a python distribution's setup.py commit ffae43d Author: Chris Livingston <clivingston@twitter.com> Date: Mon Jan 15 22:36:45 2018 -0800 Remove crufty files commit c6edc08 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Jan 12 17:23:46 2018 -0800 remove unneeded dependency test commit 2925530 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Jan 12 17:21:33 2018 -0800 Edge case impl for same setup.py package name/version as a binary dep commit fe8dd55 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 11 22:36:45 2018 -0800 Simplify a few lines, add check for ambiguous python dists, and fix copyright date commit aabf88f Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 11 18:35:34 2018 -0800 Update TODO github issue link commit a6377e4 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Jan 11 16:36:12 2018 -0800 Remove tests that break testprojects integration testing commit d41e7ee Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 10 16:29:25 2018 -0800 Remove unnecessary targets from BUILD file in superhello_testproject commit 4fd1567 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 10 14:44:23 2018 -0800 Cleanup integration test and move superhello test project to examples/tests due to test breakage when placed in testprojects commit e8f6b98 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Jan 10 11:59:32 2018 -0800 Fix multiple binary target case and add integration test commit 9e0b2d6 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Jan 9 15:57:03 2018 -0800 Add TODO with github link for package conflict case in python dist backend commit a2116c4 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Jan 9 15:20:25 2018 -0800 Remove cruft commit 03bc4c0 Author: Chris Livingston <clivingston@twitter.com> Date: Mon Jan 8 16:15:21 2018 -0800 Remove unneccessary checks for invalid targets and streamline method signatures commit 4e9bb6b Author: Chris Livingston <clivingston@twitter.com> Date: Fri Jan 5 18:24:44 2018 -0800 Fix install directory clobbering and setup.py positioning commit f1ed6da Author: Chris Livingston <clivingston@twitter.com> Date: Fri Jan 5 18:23:02 2018 -0800 Fix install directory clobbering and setup.py positioning commit be2583a Author: Lionel Vital <lvital@twitter.com> Date: Fri Jan 5 17:48:46 2018 -0800 Addresses a few changes commit 022b1e7 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 21 16:50:35 2017 -0800 Add rjiang suggestion for counting setup.py files commit e2bf445 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 21 15:16:35 2017 -0800 Clean up comments, docstrings, and fix broken testprojects integration tests commit 3e1739f Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 21 12:42:23 2017 -0800 Fix merge conflict commit 365e708 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 21 12:30:08 2017 -0800 Style fixes and cruftslaying per rjiang's comments commit 5f4e07c Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 21 12:10:02 2017 -0800 Add detection of multiple setup.py files and throw an error. commit 056e267 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Dec 20 17:50:17 2017 -0800 Add integration testing and simple unit test for python create distributions task. Also cleanup code to DRY by creating util helper method and streamline invalid python dist target detection. commit 401a10d Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 19 18:12:14 2017 -0800 Working goals using invalidated blocks commit 53a1355 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 19 16:40:56 2017 -0800 Solid working state based off of vt.results dir caching commit dacd672 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 19 15:57:37 2017 -0800 Working caching under vt.results dir. Moving to tests. commit edf110d Author: Lionel Vital <lvital@twitter.com> Date: Tue Dec 19 14:58:12 2017 -0800 More cleanup + move stuff in task execution under invalidated commit 8c7b658 Author: Lionel Vital <lvital@twitter.com> Date: Fri Dec 15 15:43:56 2017 -0800 Easy nits and whitespace issues commit 31d3a91 Author: Lionel Vital <lvital@twitter.com> Date: Fri Dec 15 14:14:19 2017 -0800 Another whitespace error commit 177c379 Author: Lionel Vital <lvital@twitter.com> Date: Fri Dec 15 12:32:06 2017 -0800 Whitespace lint fixes and unused import commit 7b8e8be Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 14 17:43:54 2017 -0800 Remove unused imports created from refactor commit 51e9c64 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 14 14:44:11 2017 -0800 Remove unused imports and add guard from consuming python dist products commit 59ff560 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Dec 13 16:52:16 2017 -0800 Add guard statement for case where pants test run does not require data from PythonCreateDistributions task. commit fb1c903 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Dec 13 14:27:04 2017 -0800 Fix minor BUILD file error to pass CI commit 23f6999 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 12 16:37:39 2017 -0800 Working distribution create task and integration for pants run/binary/test commit 314ecfb Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 12 16:05:10 2017 -0800 pants test example commit a859f46 Author: Chris Livingston <clivingston@twitter.com> Date: Mon Dec 11 18:07:08 2017 -0800 Remove cruft commit 6881a72 Author: Chris Livingston <clivingston@twitter.com> Date: Mon Dec 11 18:01:16 2017 -0800 Revert pants run mods and fix bug in pex build util commit 1c981f7 Author: Lionel Vital <lvital@twitter.com> Date: Mon Dec 11 17:36:58 2017 -0800 Run/test commit 8436dad Author: Lionel Vital <lvital@twitter.com> Date: Mon Dec 11 17:14:20 2017 -0800 More cleanup of docs commit b6fa385 Author: Lionel Vital <lvital@twitter.com> Date: Mon Dec 11 16:48:33 2017 -0800 Minor cleanup + rename alias to be standard commit 4b64f2d Author: Chris Livingston <clivingston@twitter.com> Date: Fri Dec 8 16:08:53 2017 -0800 Further iteration on python distribution task, now functioning for python binary create commit 76aeda2 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Dec 8 15:20:34 2017 -0800 Working distribution creation task commit 7823d2a Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 7 17:37:24 2017 -0800 Progress on pex_build_util commit e4c0556 Author: Chris Livingston <clivingston@twitter.com> Date: Thu Dec 7 10:02:26 2017 -0800 Install Python Dist create task commit 4b65461 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 5 17:51:57 2017 -0800 Debug session 1 commit c340abf Author: Chris Livingston <clivingston@twitter.com> Date: Tue Dec 5 17:22:32 2017 -0800 Create python distribution task and refactor example commit 9bb228f Author: Lionel Vital <lvital@twitter.com> Date: Mon Dec 4 16:05:01 2017 -0800 PythonDistribution -> inherit Target instead of PythonTarget commit 7297096 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Dec 1 12:41:17 2017 -0800 First pass at kwlzn change suggestions commit e2f84cc Author: Chris Livingston <clivingston@twitter.com> Date: Wed Nov 29 10:38:58 2017 -0800 Add python_distribution to backend/targets build file commit 194452b Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 28 16:50:14 2017 -0800 pants run task implementation - first pass commit 6585142 Author: Lionel Vital <lvital@twitter.com> Date: Tue Nov 28 16:11:15 2017 -0800 First stab at run task commit 0ae435e Author: Lionel Vital <lvital@twitter.com> Date: Tue Nov 28 15:37:57 2017 -0800 Remove excess __init__.py files commit 1940c53 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 28 15:22:47 2017 -0800 Clean up misc style issues and delete unnecessary file. commit 9cd32a4 Author: Lionel Vital <lvital@twitter.com> Date: Tue Nov 28 12:59:37 2017 -0800 Clean up + add some docs commit c8a954e Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 28 12:16:26 2017 -0800 Add cleanup of egg-info in pex build util commit 83be228 Author: Chris Livingston <clivingston@twitter.com> Date: Fri Nov 24 10:32:26 2017 -0800 Add tensorflow dependency example to superhello; this puts ./pants binary on main:main in a good state for a workable demo commit 74bfb8c Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 16:45:55 2017 -0800 Finalized initial approach at packaging wheels w/ c sources into a pex commit 4e2b1b9 Author: Lionel Vital <lvital@twitter.com> Date: Tue Nov 21 15:02:05 2017 -0800 Minor cleanup commit dd99194 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 14:43:19 2017 -0800 Working superhello project equivalent to backends/tensorflow commit 5837f30 Author: Lionel Vital <lvital@twitter.com> Date: Tue Nov 21 14:27:00 2017 -0800 Modify example to use superhello commit 4f9bc98 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 13:51:52 2017 -0800 Working python dist example commit cd32058 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 12:22:23 2017 -0800 Changes to python binary creation + add new target definition commit 3aa0fe4 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 12:19:55 2017 -0800 Changes to hello2 package commit 46eab74 Author: Chris Livingston <clivingston@twitter.com> Date: Tue Nov 21 12:19:22 2017 -0800 Working package example for hello2 commit ca86bf6 Author: Chris Livingston <clivingston@twitter.com> Date: Wed Nov 15 13:01:29 2017 -0800 First pass at distribution task and target commit 3ec11c2 Author: Lionel Vital <lvital@twitter.com> Date: Thu Nov 9 23:27:59 2017 -0800 Python_distribution example

…omments

In response to review comments, remove a lot of indirection and instead just exposed the native toolchain as path entries, and allow consumers such as BuildLocalPythonDistributions to perform the environment modification. Also add a lot of explanatory remarks, some still in comments. Also introduce `XCodeCLITools` to wrap the OSX provided toolchain uniformly.

stuhood

This looks great @cosmicexplorer... thanks.

stuhood · 2018-03-30T17:42:58Z

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

+  containing executables to compile and link "native" code (for now, C and C++
+  are supported). Consumers of this subsystem can add these directories to their
+  PATH to invoke subprocesses which use these tools.
+


Great explanation.

cosmicexplorer · 2018-04-02T23:13:14Z

Travis looks like it's failing just on fetching the binaries from pantsbuild/binaries#62 -- there may be errors in the tests that use those binaries, but they are passing locally, so after that PR is merged this should hopefully be mergeable. Further comments are totally welcome!

stuhood · 2018-04-03T16:04:51Z

Restarted travis now that the binaries are out.

cosmicexplorer · 2018-04-03T16:09:17Z

I actually restarted travis last night, but the integration tests invoking setup.py projects failed when building native extensions with Could not find x86_64-linux-gnu-gcc. I'm going to look into this today, I think it can be resolved by setting CC and CXX in the environment, but if not I may have to look into subclassing distutils's UnixCCompiler class as mentioned in comments on previous commits (I don't think I'll need to go that far for this PR to pass).

cosmicexplorer · 2018-04-03T19:02:56Z

I realized that when invoking setup.py I was isolating the PATH to just our provided toolchain directories (clang, gcc, and binutils on linux), and I hadn't thoroughly tested how setup.py would respond to that in a linux environment that already has a compiler. I have reverted that in b4feaa2, and I will continue to investigate if the failure continues to occur in CI.

cosmicexplorer · 2018-04-05T03:07:01Z

CI is green, and I opened a few issues for the remaining support concerns, but this should be shippable.

…ibution and LLVM subsystems to use it (#5780) ### Problem `BinaryTool` is a great recent development which makes using binaries downloaded lazily from a specified place much more declarative and much more extensible. However, it's still only able to download from either our S3 hosting, or a mirror. The previous structure requires the urls provided to the global option `--binaries-baseurls` to point to an exact mirror of the hierarchy we provide in our S3 hosting, but that can change at any time. It's not incredibly difficult to write a script to mirror our hosting into an internal network, but in general there's no reason the layout of binaries in `~/.cache/pants/bin/` needs to determine where those binaries are downloaded from. Our bandwidth costs in S3 have recently increased due to the introduction of clang and gcc in #5490. *See #5777 and #5779 for further context on S3 hosting.* There are reliable binary downloads for some of these tools, which we would be remiss not to use if we can do it in a structured way. ### Solution - Introduce a `urls=` argument to multiple methods of `BinaryUtil` for `BinaryTool`s that don't download from our s3. - Add support for extracting (not creating) `.tar.xz` archives by adding the `xz` BinaryTool (see pantsbuild/binaries#66) and integrating it into BinaryTool's `archive_type` selection mechanism. - Use the above to download the `go` and `llvm` binaries from their official download urls. - Also, rename the `Clang` subsystem to `LLVM` as the binary download we use now (for ubuntu 16.04, currently) also contains many other LLVM tools, including e.g. `lld`. ### Result Urls for binary downloads can now be created in a structured way for external downloads, with the `--force-baseurls` option as an escape hatch. Some binaries now default to external urls provided for public use by the maintainers of the software to download, thanks to the introduction of the `xz` binary tool. Two out of the three largest bandwidth users among our provided binaries have been switched to use the download urls provided by the maintainers of each project (LLVM and Go). gcc still needs to be fixed, which will happen in a separate PR.

…with ctypes (#5815) ### Problem It is currently possible to expose native code to Python by compiling it in a `python_dist()` target, specifying C or C++ source files as a `distutils.core.Extension` in the `setup.py` file, as well as in the target's sources. `python_dist()` was introduced in #5141. We introduced a "native toolchain" to compile native sources for this use case in #5490. Exposing Python code this way requires using the Python native API and `#include <Python.h>` in your source files. However, python can also interact with native code that does not use the Python native API, using the provided `ctypes` library. For this to work, the `python_dist()` module using `ctypes` needs to have a platform-specific shared library provided within the package. This PR introduces the targets, tasks, and subsystems to compile and link a shared library from native code, then inserts it into the `python_dist()` where it is easily accessible. ### Solution - Introduce the `ctypes_compatible_c_library()` target which covers C sources (`ctypes_compatible_cpp_library()` for C++), and can specify what to name the shared library created from linking the object files compiled from its sources and dependencies. - Introduce `CCompile`, `CppCompile`, and `LinkSharedLibraries` to produce the shared libraries from the native sources. The compile tasks use options `CCompileSettings` or `CppCompileSettings` to define file extensions for "header" and "source" files. - Introduce the `CCompileSettings` and `CppCompileSettings` subsystems to control compile settings for those languages. - Convert `BuildLocalPythonDistributions` to proxy to the native backend through the new `PythonNativeCode` subsystem. - Move all the `BinaryTool` subsystems to a `subsystems/binaries` subdirectory, and expose them to the v2 engine through `@rule`s defined in the subsystem's file. - Move some of the logic in `pex_build_util.py` to `setup_py.py`, and expose datatypes composing the setup.py environment through `@rule`s in `setup_py.py`. `SetupPyRunner.for_bdist_wheel()` was created to set the wheel's platform, if the `python_dist()` target contains any native sources of its own, or depends on any `ctypes_compatible_*_library`s. **Note:** the new targets are specifically prefixed with `ctypes_compatible_` because we don't yet eclipse the functionality of `contrib/cpp`. When the targets become usable for more than this one use case, the name should be changed. ### Result To see how to link up native and Python code with `ctypes`, here's (most of) the contents of `testprojects/src/python/python_distribution/ctypes`: *BUILD*: ```python ctypes_compatible_c_library( name='c_library', sources=['some_math.h', 'some_math.c', 'src-subdir/add_three.h', 'src-subdir/add_three.c'], ctypes_dylib=native_artifact(lib_name='asdf-c'), ) ctypes_compatible_cpp_library( name='cpp_library', sources=['some_more_math.hpp', 'some_more_math.cpp'], ctypes_dylib=native_artifact(lib_name='asdf-cpp'), ) python_dist( sources=[ 'setup.py', 'ctypes_python_pkg/__init__.py', 'ctypes_python_pkg/ctypes_wrapper.py', ], dependencies=[ ':c_library', ':cpp_library', ], ) ``` *setup.py*: ```python setup( name='ctypes_test', version='0.0.1', packages=find_packages(), # Declare two files at the top-level directory (denoted by ''). data_files=[('', ['libasdf-c.so', 'libasdf-cpp.so'])], ) ``` *ctypes_python_pkg/ctypes_wrapper.py*: ```python import ctypes import os def get_generated_shared_lib(lib_name): # These are the same filenames as in setup.py. filename = 'lib{}.so'.format(lib_name) # The data files are in the root directory, but we are in ctypes_python_pkg/. rel_path = os.path.join(os.path.dirname(__file__), '..', filename) return os.path.normpath(rel_path) asdf_c_lib_path = get_generated_shared_lib('asdf-c') asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp') asdf_c_lib = ctypes.CDLL(asdf_c_lib_path) asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path) def f(x): added = asdf_c_lib.add_three(x) multiplied = asdf_cpp_lib.multiply_by_three(added) return multiplied ``` Now, the target `testprojects/src/python/python_distribution/ctypes` can be depended on in a BUILD file, and other python code can freely use `from ctypes_python_pkg.ctypes_wrapper import f` to start jumping into native code. #### Follow-up issues: 1. #5933 2. #5934 3. #5949 4. #5950 5. #5951 6. #5962 7. #5967 8. #5977

@rules

Closes pantsbuild#5831 Prep for release 1.8.0dev3 (pantsbuild#5937) Ban bad `readonly` shell pattern (pantsbuild#5924) This subverts `set -e` and means that failed commands don't fail the script, which is responsible for late failures on CI such as https://travis-ci.org/pantsbuild/pants/jobs/389174049 which failed to download protoc, but only failed when something tried to use it. Fixup macosx platform version. (pantsbuild#5938) This needs to match the Travis CI osx platform we pre-build wheels on. Work towards pantsbuild#4896. Allow pants to select targets by file(s) (pantsbuild#5930) Fixes pantsbuild#5912 Problem There should be an option to accept literal files, and then select the targets that own those files. Similar to how the --changed-parent triggers a diff and the targets are selected based on the result. The proposed syntax is something like: $ ./pants \ --owner-of=this/is/a/file/name.java \ # flag triggers owner lookup --owner-of=this/is/a/file/name/too.py \ # flag triggers owner lookup compile # goal Solution I've created a global option --owner-of= that takes a list of files as a parameter, and created a OwnerCalculator class to handle the logic similar to how ChangeCalculator works with the --changed-* subsystem. Result Now users will be able to run goals on files without needing to know which target owns those files. Also, the list-owners goal can be deprecated in favor of --owner-of=some/file.py list It is important to note that multiple target selection methods are not allowed, so it fails when more than one of --changed-*, --owner-of, or target specs are supplied. e.g. this fails: $ ./pants \ --owner-of=this/is/a/file/name.java \ # flag triggers owner lookup --owner-of=this/is/a/file/name/too.py \ # flag triggers owner lookup compile <another target> Integration test for daemon environment scrubbing (pantsbuild#5893) This shows that pantsbuild#5898 works, which itself fixed pantsbuild#5854 Add the --owner-of= usage on Target Address documentation (pantsbuild#5931) Problem The documentation of the feature proposed in PR pantsbuild#5930 Solution I decided to put it inside Target Addresses because that is where a user would look if they needed a feature like this, I think. Result More docs, and that's always good. Add script to get a list of failing pants from travis (pantsbuild#5946) [jvm-compile] template-methodify JvmCompile further; add compiler choices (pantsbuild#5923) Introduce `JvmPlatform.add_compiler_choice(name)`, which allows plugins to register compilers that can be configured. This patch pulls out some template methods in JvmCompile to make it easier to extend. It also pushes some of the implementations of those methods down into ZincCompile, where appropriate. These changes should be covered by existing tests, but it could make sense to add tests around the interfaces of the new template methods. I don't anticipate there being a large number of implementations at this time though, so I didn't think it'd be worth it. Add the following template methods * `create_empty_extra_products` Allows subclasses to create extra products that other subclasses might not need, that ought to be constructed even if no compile targets are necessary. * `register_extra_products_from_contexts` rename of `_register_vts`. This allows subclasses to register their extra products for particular targets. * `select_runtime_context` Not 100% happy with this, but I'm working on something that needs to have different types of compile contexts. It allows subclasses to specify a context that provides paths for the runtime classpath if the default context isn't quite right for the usages in the base class. * `create_compile_jobs` Pulled this out into a separate method so that subclasses can create multiple graph jobs per target. * Pushed down behavior from JvmCompile that should live in zinc via the template methods extracted above. There's probably more that could be done here, but this was the first cut of it. * Moved the execute definition from BaseZincCompile to ZincCompile so that it's possible to subclass BaseZincCompile with a different compiler name. release notes for 1.7.0.rc1 (pantsbuild#5942) Use target not make_target in some tests (pantsbuild#5939) This pushes parsing of the targets through the engine, rather than bypassing it. This is important because I'm about to make these targets require an EagerFilesetWithSpec as their source/sources arg, rather than being happy with a list of strings. Add new remote execution options (pantsbuild#5932) As described in pantsbuild#5904, a few configuration values that are important to testing of remote execution are currently hardcoded. Extract existing options to a `ExecutionOptions` collection (which should become a `Subsystem` whenever we add support for consuming `Subsystems` during bootstrap), and add the new options. Fixes pantsbuild#5904. Separate the resolution cache and repository cache in Ivy (pantsbuild#5844) move glob matching into its own file (pantsbuild#5945) See pantsbuild#5871, where we describe an encapsulation leak created by implementing all of the glob expansion logic in the body of `VFS`. - Create `glob_matching.rs`, exporting the `GlobMatching` trait, which exports the two methods `canonicalize` and `expand`, which call into methods in a private trait `GlobMatchingImplementation`. **Note:** `canonicalize` calls `expand`, and vice versa, which is why both methods were moved to `glob_matching.rs`. Orthogonal glob matching logic is made into a trait that is implemented for all types implementing `VFS`, removing the encapsulation leak. The `VFS` trait is now just four method signature declarations, making the trait much easier to read and understand. Enable fromfile support for --owner-of and increase test coverage (pantsbuild#5948) The new `--owner-of` option was broken in the context of `pantsd`, but didn't have test coverage due to the `--changed` and `--owner-of` tests not running under `pantsd`. Additionally, `fromfile` support is useful for this option, but was not enabled. Mark some integration tests as needing to run under the daemon, and enable `fromfile` support for `--owner-of`. [pantsd] Robustify client connection logic. (pantsbuild#5952) Fixes pantsbuild#5812 under full-on-assault stress testing via: $ watch -n.1 'pkill -f "pantsd \[" pantsd-runner' this will mostly behave like: WARN] pantsd was unresponsive on port 55620, retrying (1/3) WARN] pantsd was unresponsive on port 55620, retrying (2/3) WARN] pantsd was unresponsive on port 55626, retrying (3/3) WARN] caught client exception: Fallback(NailgunExecutionError(u'Problem executing command on nailgun server (address: 127.0.0.1:55630): TruncatedHeaderError(u"Failed to read nailgun chunk header (TruncatedRead(u\'Expected 5 bytes before socket shutdown, instead received 0\',)).",)',),), falling back to non-daemon mode 23:30:24 00:00 [main] (To run a reporting server: ./pants server) 23:30:38 00:14 [setup] 23:30:39 00:15 [parse] ... mid-flight terminations (simulated via single-shot pkill calls) also result in a more descriptive error with traceback proxying: 23:40:51 00:04 [zinc] 23:40:51 00:04 [javac] 23:40:51 00:04 [cpp] 23:40:51 00:04 [errorprone] 23:40:51 00:04 [findbugs]CRITICAL] CRITICAL] lost active connection to pantsd! Exception caught: (<class 'pants.bin.remote_pants_runner.Terminated'>) File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 73, in <module> main() File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 69, in main PantsLoader.run() File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 65, in run cls.load_and_execute(entrypoint) File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 58, in load_and_execute entrypoint_main() File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_exe.py", line 39, in main PantsRunner(exiter, start_time=start_time).run() File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_runner.py", line 39, in run return RemotePantsRunner(self._exiter, self._args, self._env, bootstrap_options).run() File "/Users/kwilson/dev/pants/src/python/pants/bin/remote_pants_runner.py", line 162, in run self._run_pants_with_retry(port) File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 221, in execute return self._session.execute(cwd, main_class, *args, **environment) File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 94, in execute return self._process_session() File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 69, in _process_session for chunk_type, payload in self.iter_chunks(self._sock, return_bytes=True): File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_protocol.py", line 206, in iter_chunks chunk_type, payload = cls.read_chunk(sock, return_bytes) File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_protocol.py", line 182, in read_chunk raise cls.TruncatedHeaderError('Failed to read nailgun chunk header ({!r}).'.format(e)) Exception message: abruptly lost active connection to pantsd runner: NailgunError(u'Problem talking to nailgun server (address: 127.0.0.1:55707, remote_pid: -28972): TruncatedHeaderError(u"Failed to read nailgun chunk header (TruncatedRead(u\'Expected 5 bytes before socket shutdown, instead received 0\',)).",)',) Re-shade zinc to avoid classpath collisions with annotation processors. (pantsbuild#5953) Zinc used to be shaded before the `1.x.y` upgrade (pantsbuild#4729), but shading was removed due to an overabundance of optimism. When testing the zinc upgrade internally, we experienced a classpath collision between an annotation processor and zinc (in guava, although zinc has many other dependencies that could cause issues). Shade zinc, and ensure that our annotation processor uses a very old guava in order to attempt to force collisions in future. Improve PythonInterpreterCache logging (pantsbuild#5954) When users have issues building their Python interpreter cache, they are often very confused because does not currently log much about the process to help users debug. Here we add log lines describing what/where Pants looks to build the interpreter cache, and the results of what it found. This should help users better understand/debug the process. use liblzma.dylib for xz on osx and add platform-specific testing to the rust osx shard (pantsbuild#5936) See pantsbuild#5928. The `xz` archiver wasn't tested on osx at all, and failed to find `liblzma.so` on osx (it should have been `liblzma.dylib`). There were additional errors with library search paths reported in that PR which I was not immediately able to repro. This PR hopefully fixes all of those errors, and ensures they won't happen again with the addition of platform-specific testing (see previous issue at pantsbuild#5920). - Switch to a statically linked `xz`. - Fix the incorrect key `'darwin'` in the platform dictionary in the `LLVM` subsystem (to `'mac'`). - Add the tag `platform_specific_behavior` to the new python target `tests/python/pants_test/backend/python/tasks:python_native_code_testing`, which covers the production of `python_dist()`s with native code. - Add the `-z` argument to `build-support/bin/ci.sh` to run all tests with the `platform_specific_behavior` tag. Also clean up old unused options in the getopts call, and convert echo statements to a simpler heredoc. - Change the name of the "Rust Tests OSX" shard to "Rust + Platform-specific Tests OSX", and add the `-z` switch to the `ci.sh` invocation. **Note:** the tests in `tests/python/pants_test/backend/native/subsystems` are going to be removed in pantsbuild#5815, otherwise they would be tagged similarly. `./pants test tests/python/pants_test/backend/python/tasks:python_native_code_testing` now passes on osx, and this fact is now being tested in an osx shard in travis. Support output directory saving for local process execution. (pantsbuild#5944) Closes pantsbuild#5860 reimplement a previous PR -- ignore this 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 create tasks and targets for c/c++ sources - refactors the "native toolchain" and introduces the "binaries" subdirectory of subsystems start by adding a new ctypes testproject add c/c++ sources add example BUILD file add some native targets add tasks dir remove gcc try to start adding tasks clean some leftover notes in BuildLocalPythonDistributions update NativeLibrary with headers move DependencyContext to target.py add native compile tasks houston we have compilation run: ./pants -ldebug --print-exception-stacktrace compile testprojects/src/python/python_distribution/ctypes: for an idea use the forbidden product request change target names to avoid conflict with cpp contrib and flesh out cpp_compile now we are compiling code we can link things now now we know how to infer headers vs sources fix the test case and fix include dir collection (un)?suprisingly, everything works, but bdist_wheel doesn't read MANIFEST.in houston we have c++ bring back gcc so we can compile halfway done with osx support now things work on osx???????? ok, now it works on linux again too first round of review - NB: refactors the organization of the `results_dir` for python_dist()! - move ctypes integration testing into python backend tests revert some unnecessary changes refactor native_artifact to be a datatype fix some copyright years add ctypes integration test add assert_single_element method in collections.py decouple the native tools for setup.py from the execution environment streamline the selection of the native tools for setup.py invocation make gcc depend on binutils on linux for the 'as' assembler fix logging visibility by moving it back into the task make the check for the external llvm url more clear refactor local dist building a bit - use SetupPyRunner.DIST_DIR as the source of truth - add a separate subdir of the python_dist target's results_dir for the python_dist sources - move shraed libs into the new subdir fix imports second round of review - fixed bugs - expanded error messages and docstrings make a couple docstring changes fix dist platform selection ('current' is not a real platform) lint fixes fix broken regex which modifies the `.dylib` extension for python_dist() fix the ctypes integration test respond to some review comments clear the error message if we can't find xcode cli tools refactor the compilation and linking pipeline to use subsystems - also add `PythonNativeCode` subsystem to bridge the native and python backends refactor the compilation and linking pipeline to use subsystems add some notes fix rebase issues add link to pantsbuild#5788 -- maybe use variants for args for static libs move `native_source_extensions` to a new `PythonNativeCode` subsystem update native toolchain docs and remove bad old tests move tgt_closure_platforms into the new `PythonNativeCode` subsystem remove unnecessary logging remove compile_settings_class in favor of another abstractmethod refactor `NativeCompile` and add documentation improve debug logging in NativeCompile document NativeCompileSettings refactor and add docstrings convert provides= to ctypes_dylib= and add many more docstrings remove or improve TODOs improve or remove FIXMEs improve some docstrings, demote a FIXME, and add a TODO link FIXMEs to a ticket add notes to the ctypes testproject update mock object for strict deps -- test passes fix failing integration test on osx add hack to let travis pass fix the system_id key in llvm and add a shameful hack to pass travis swap the order of alias_types remove unused EmptyDepContext class remove -settings suffix from compile subsystem options scopes add AbstractClass to NativeLibrary bump implementation_version for python_dist() build - we have changed the layout of the results_dir in this PR add ticket link and fix bug revert indentation changes to execute() method refactor `assert_single_element()` revert addition of `narrow_relative_paths()` add link to pantsbuild#5950 move common process invocation logic into NativeCompile revert an unnecessary change turn an assert into a full exception revert unnecessary change use get_local_platform() wherever possible delete duplicate llvm subsystem fix xcode cli tools resolution change `ctypes_dylib=` to `ctypes_native_library=` add a newline move UnsupportedPlatformError to be a class field remove unused codegen_types field fix zinc-compiler options to be valid ones Construct rule_graph recursively (pantsbuild#5955) The `RuleGraph` is currently constructed iteratively, but can be more-easily constructed recursively. Switch to constructing the `RuleGraph` recursively, and unify a few disparate diagnostic messages. Helps to prepare for further refactoring in pantsbuild#5788. Allow manylinux wheels when resolving plugins. (pantsbuild#5959) Also plumb manylinux resolution support for the python backend, on by default, but configurable via `python_setup.resolver_use_manylinux`. Fixes pantsbuild#5958 `exclude-patterns` and `tag` should apply only to roots (pantsbuild#5786) The `--exclude-patterns` flag currently applies to inner nodes, which causes odd errors. Moreover, tags should also apply only to roots. See pantsbuild#5189. - added `tag` & `exclude_patterns` as params to `Specs` - add tests for both - modify changed tests to pass for inner node filtering Fixes pantsbuild#5189. Remove value wrapper on the python side of ffi. (pantsbuild#5961) As explained in the comment in this change, the overhead of wrapping our CFFI "handle"/`void*` instances in a type that is shaped like the `Value` struct was significant enough to care about. Since the struct has zero overhead on the rust side, whether we represent it as typedef or a struct on the python side doesn't make a difference (except in terms of syntax). 6% faster `./pants list ::` in Twitter's repo. return an actual field use temporary native-compile goal Cobertura coverage: Include the full target closure's classpath entries for instrumentation (pantsbuild#5879) Sometimes Cobertura needs access to the dependencies of class files being instrumented in order to rewrite them (pantsbuild#5878). This patch adds an option that creates a manifest jar and adds an argument to the Cobertura call so that it can take advantage of it. class files that need to determine a least upper bound in order to be rewritten can now be instrumented. Fixes pantsbuild#5878 add ticket link fix xcode install locations and reduce it to a single dir_option return the correct amount of path entries Record start times per graph node and expose a method to summarize them. (pantsbuild#5964) In order to display "significant" work while it is running on the engine, we need to compute interesting, long-running leaves. Record start times per entry in the `Graph`, and add a method to compute a graph-aware top-k longest running leaves. We traverse the graph "longest running first", and emit the first `k` leaves we encounter. While this will almost certainly need further edits to maximize it's usefulness, visualization can begun to be built atop of it. Prepare the 1.8.0.dev4 release (pantsbuild#5969) Mark a few options that should not show up in `./pants help`. (pantsbuild#5968) `./pants help` contains core options that are useful to every pants command, and the vast majority of global options are hidden in order to keep it concise. A few non-essential options ended up there recently. Hide them. adding more documentation for python_app (pantsbuild#5965) The python_app target doesn't have the documentation specific for it and has a documentation that is specific to jvm_app. Added a few lines of documentation. There is no system-wide change, only a documentation change. Remove DeprecatedPythonTaskTestBase (pantsbuild#5973) Use PythonTaskTestBase instead. Fixes pantsbuild#5870 Chris first commit on fresh rebase Merge branch 'ctypes-test-project' of github.com:cosmicexplorer/pants into clivingston/ctypes-test-project-third-party unrevert reverted fix (NEEDS FOLLOWUP ISSUE!) put in a better fix for the strict_deps error until the followup issue is made add ticket link Merge branch 'ctypes-test-project' of github.com:cosmicexplorer/pants into clivingston/ctypes-test-project-third-party Shorten safe filenames further, and combine codepaths to make them readable. (pantsbuild#5971) Lower the `safe_filename` path component length limit to 100 characters, since the previous 255 value did not account for the fact that many filesystems also have a limit on total path length. This "fixes" the issue described in pantsbuild#5587, which was caused by using this method via `build_invalidator.py`. Additionally, merge the codepath from `Target.compute_target_id` which computes a readable shortened filename into `safe_filename`, and expand tests. This removes some duplication, and ensure that we don't run into a similar issue with target ids. The specific error from pantsbuild#5587 should be prevented, and consumers of `safe_filename` should have safe and readable filenames. Fixes pantsbuild#5587. Whitelist the --owner-of option to not restart the daemon. (pantsbuild#5979) Because the `--owner-of` option was not whitelisted as `daemon=False`, changing its value triggered unnecessary `pantsd` restarts. Whitelist it. Prepare the 1.8.0rc0 release. (pantsbuild#5980) Robustify test_namespace_effective PYTHONPATH. (pantsbuild#5976) The real problem is noted, but this quick fix should bolster against interpreter cache interpreters pointing off to python binaries that have no setuptools in their associated site-packages. Fixes pantsbuild#5972 make_target upgrades sources to EagerFilesetWithSpec (pantsbuild#5974) This better simulates how the engine parses BUILD files, giving a more faithful experience in tests. I'm about to make it a warning/error to pass a list of strings as the sources arg, so this will make tests which use make_target continue to work after that. Also, make cloc use base class scheduler instead of configuring its own. Lib and include as a dep-specifc location source attribute is automatically promoted to sources (pantsbuild#5908) This means that either the `source` or `sources` attribute can be used for any rule which expects sources. Places that `source` was expected still verify that the correct number of sources are actually present. Places that `sources` was expected will automatically promote `source` to `sources`. This is a step towards all `sources` attributes being `EagerFilesetWithSpec`s, which will make them cached in the daemon, and make them easier to work with with both v2 remote execution and in the v2 engine in general. It also provides common hooks for input file validation, rather than relying on them being done ad-hoc in each `Target` constructor. For backwards compatibility, both attributes will be populated on `Target`s, but in the future only the sources value will be provided. `sources` is guaranteed to be an `EagerFilesetWithSpec` whichever of these mechanisms is used. A hook is provided for rules to perform validation on `sources` at build file parsing time. Hooks are put in place for non-contrib rule types which currently take a `source` attribute to verify that the correct number of sources are provided. I imagine at some point we may want to add a "file type" hook too, so that rules can error if files of the wrong type were added as sources. This is a breaking change for rules which use both the `source` and `sources` attributes (and where the latter is not equivalent to the former), or where the `source` attribute is used to refer to something other than a file. `source` is now becoming a reserved attribute name, as `sources` and `dependencies` already are. This is also a breaking change for rules which use the `source` attribute, but never set `sources` in a Payload. These will now fail to parse. This is also a slightly breaking change for the `page` rule - before, omitting the `source` attribute would parse, but fail at runtime. Now, it will fail to parse. This is also a breaking change in that in means that the source attribute is now treated like a glob, and so if a file is specified which isn't present, it will be ignored instead of error. This feels a little sketchy, but it turns out we did the exact same thing by making all sources lists be treated like globs... Override get_sources for pants plugins (pantsbuild#5984) 1.7.0 release notes (pantsbuild#5983) No additional changes, so it's a very short release note. Fixups for native third party work hardcode in c/c++ language levels for now remove all the unnecessary code relating to file extensions Merge branch 'ctypes-test-project' of github.com:cosmicexplorer/pants into clivingston/ctypes-test-project-third-party Caching tests are parsed through the engine (pantsbuild#5985) reimplement a previous PR -- ignore this 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 create tasks and targets for c/c++ sources - refactors the "native toolchain" and introduces the "binaries" subdirectory of subsystems start by adding a new ctypes testproject add c/c++ sources add example BUILD file add some native targets add tasks dir remove gcc try to start adding tasks clean some leftover notes in BuildLocalPythonDistributions update NativeLibrary with headers move DependencyContext to target.py add native compile tasks houston we have compilation run: ./pants -ldebug --print-exception-stacktrace compile testprojects/src/python/python_distribution/ctypes: for an idea use the forbidden product request change target names to avoid conflict with cpp contrib and flesh out cpp_compile now we are compiling code we can link things now now we know how to infer headers vs sources fix the test case and fix include dir collection (un)?suprisingly, everything works, but bdist_wheel doesn't read MANIFEST.in houston we have c++ bring back gcc so we can compile halfway done with osx support now things work on osx???????? ok, now it works on linux again too first round of review - NB: refactors the organization of the `results_dir` for python_dist()! - move ctypes integration testing into python backend tests revert some unnecessary changes refactor native_artifact to be a datatype fix some copyright years add ctypes integration test add assert_single_element method in collections.py decouple the native tools for setup.py from the execution environment streamline the selection of the native tools for setup.py invocation make gcc depend on binutils on linux for the 'as' assembler fix logging visibility by moving it back into the task make the check for the external llvm url more clear refactor local dist building a bit - use SetupPyRunner.DIST_DIR as the source of truth - add a separate subdir of the python_dist target's results_dir for the python_dist sources - move shraed libs into the new subdir fix imports second round of review - fixed bugs - expanded error messages and docstrings make a couple docstring changes fix dist platform selection ('current' is not a real platform) lint fixes fix broken regex which modifies the `.dylib` extension for python_dist() fix the ctypes integration test respond to some review comments clear the error message if we can't find xcode cli tools refactor the compilation and linking pipeline to use subsystems - also add `PythonNativeCode` subsystem to bridge the native and python backends refactor the compilation and linking pipeline to use subsystems add some notes fix rebase issues add link to pantsbuild#5788 -- maybe use variants for args for static libs move `native_source_extensions` to a new `PythonNativeCode` subsystem update native toolchain docs and remove bad old tests move tgt_closure_platforms into the new `PythonNativeCode` subsystem remove unnecessary logging remove compile_settings_class in favor of another abstractmethod refactor `NativeCompile` and add documentation improve debug logging in NativeCompile document NativeCompileSettings refactor and add docstrings convert provides= to ctypes_dylib= and add many more docstrings remove or improve TODOs improve or remove FIXMEs improve some docstrings, demote a FIXME, and add a TODO link FIXMEs to a ticket add notes to the ctypes testproject update mock object for strict deps -- test passes fix failing integration test on osx add hack to let travis pass fix the system_id key in llvm and add a shameful hack to pass travis swap the order of alias_types remove unused EmptyDepContext class remove -settings suffix from compile subsystem options scopes add AbstractClass to NativeLibrary bump implementation_version for python_dist() build - we have changed the layout of the results_dir in this PR add ticket link and fix bug revert indentation changes to execute() method refactor `assert_single_element()` revert addition of `narrow_relative_paths()` add link to pantsbuild#5950 move common process invocation logic into NativeCompile revert an unnecessary change turn an assert into a full exception revert unnecessary change use get_local_platform() wherever possible delete duplicate llvm subsystem fix xcode cli tools resolution change `ctypes_dylib=` to `ctypes_native_library=` add a newline move UnsupportedPlatformError to be a class field remove unused codegen_types field fix zinc-compiler options to be valid ones return an actual field use temporary native-compile goal add ticket link fix xcode install locations and reduce it to a single dir_option return the correct amount of path entries unrevert reverted fix (NEEDS FOLLOWUP ISSUE!) put in a better fix for the strict_deps error until the followup issue is made add ticket link hardcode in c/c++ language levels for now remove all the unnecessary code relating to file extensions fix osx failures and leave a ticket link Add rang header-only lib for integration testing Merge branch 'ctypes-test-project' of github.com:cosmicexplorer/pants into clivingston/ctypes-test-project-third-party Fix TestSetupPyInterpreter.test_setuptools_version (pantsbuild#5988) Previously the test failed to populate the `PythonInterpreter` data product leading to a fallback to the current non-bare interpreter which allowed `setuptools` from `site-packages` to leak in. Fixes pantsbuild#5467 Refactor conan grab into subsystem Engine looks up default sources when parsing (pantsbuild#5989) Rather than re-implementing default source look-up. This pushes sources parsing of default sources through the engine, in parallel, rather than being later synchronous python calls. This also works for plugin types, and doesn't change any existing APIs. It updates the Go patterns to match those that the engine currently performs, rather than ones which aren't actually used by any code. Add unit tests, refactor C/C++ targets which can be compiled/linked and used in python_dist() with ctypes (pantsbuild#5815) It is currently possible to expose native code to Python by compiling it in a `python_dist()` target, specifying C or C++ source files as a `distutils.core.Extension` in the `setup.py` file, as well as in the target's sources. `python_dist()` was introduced in pantsbuild#5141. We introduced a "native toolchain" to compile native sources for this use case in pantsbuild#5490. Exposing Python code this way requires using the Python native API and `#include <Python.h>` in your source files. However, python can also interact with native code that does not use the Python native API, using the provided `ctypes` library. For this to work, the `python_dist()` module using `ctypes` needs to have a platform-specific shared library provided within the package. This PR introduces the targets, tasks, and subsystems to compile and link a shared library from native code, then inserts it into the `python_dist()` where it is easily accessible. - Introduce the `ctypes_compatible_c_library()` target which covers C sources (`ctypes_compatible_cpp_library()` for C++), and can specify what to name the shared library created from linking the object files compiled from its sources and dependencies. - Introduce `CCompile`, `CppCompile`, and `LinkSharedLibraries` to produce the shared libraries from the native sources. The compile tasks use options `CCompileSettings` or `CppCompileSettings` to define file extensions for "header" and "source" files. - Introduce the `CCompileSettings` and `CppCompileSettings` subsystems to control compile settings for those languages. - Convert `BuildLocalPythonDistributions` to proxy to the native backend through the new `PythonNativeCode` subsystem. - Move all the `BinaryTool` subsystems to a `subsystems/binaries` subdirectory, and expose them to the v2 engine through `@rule`s defined in the subsystem's file. - Move some of the logic in `pex_build_util.py` to `setup_py.py`, and expose datatypes composing the setup.py environment through `@rule`s in `setup_py.py`. `SetupPyRunner.for_bdist_wheel()` was created to set the wheel's platform, if the `python_dist()` target contains any native sources of its own, or depends on any `ctypes_compatible_*_library`s. **Note:** the new targets are specifically prefixed with `ctypes_compatible_` because we don't yet eclipse the functionality of `contrib/cpp`. When the targets become usable for more than this one use case, the name should be changed. To see how to link up native and Python code with `ctypes`, here's (most of) the contents of `testprojects/src/python/python_distribution/ctypes`: *BUILD*: ```python ctypes_compatible_c_library( name='c_library', sources=['some_math.h', 'some_math.c', 'src-subdir/add_three.h', 'src-subdir/add_three.c'], ctypes_dylib=native_artifact(lib_name='asdf-c'), ) ctypes_compatible_cpp_library( name='cpp_library', sources=['some_more_math.hpp', 'some_more_math.cpp'], ctypes_dylib=native_artifact(lib_name='asdf-cpp'), ) python_dist( sources=[ 'setup.py', 'ctypes_python_pkg/__init__.py', 'ctypes_python_pkg/ctypes_wrapper.py', ], dependencies=[ ':c_library', ':cpp_library', ], ) ``` *setup.py*: ```python setup( name='ctypes_test', version='0.0.1', packages=find_packages(), # Declare two files at the top-level directory (denoted by ''). data_files=[('', ['libasdf-c.so', 'libasdf-cpp.so'])], ) ``` *ctypes_python_pkg/ctypes_wrapper.py*: ```python import ctypes import os def get_generated_shared_lib(lib_name): # These are the same filenames as in setup.py. filename = 'lib{}.so'.format(lib_name) # The data files are in the root directory, but we are in ctypes_python_pkg/. rel_path = os.path.join(os.path.dirname(__file__), '..', filename) return os.path.normpath(rel_path) asdf_c_lib_path = get_generated_shared_lib('asdf-c') asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp') asdf_c_lib = ctypes.CDLL(asdf_c_lib_path) asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path) def f(x): added = asdf_c_lib.add_three(x) multiplied = asdf_cpp_lib.multiply_by_three(added) return multiplied ``` Now, the target `testprojects/src/python/python_distribution/ctypes` can be depended on in a BUILD file, and other python code can freely use `from ctypes_python_pkg.ctypes_wrapper import f` to start jumping into native code. 1. pantsbuild#5933 2. pantsbuild#5934 3. pantsbuild#5949 4. pantsbuild#5950 5. pantsbuild#5951 6. pantsbuild#5962 7. pantsbuild#5967 8. pantsbuild#5977 Add simple integration test Pull in master Minor cleanup Fix CI errors Debug log stdout Fix integration test Fix integration test Fix lint error on third party

… native backend subsystem tests (#5943) ### Problem The native backend subsystems tests introduced in #5490 are still skipped, complaining about `crti.o` on linux, which is part of libc. See #5662 -- `clang`'s driver will find the directory containing that file on travis, but `gcc` won't. We should make a way to find this file (which is necessary for creating executables) so we can unskip the native backend testing. ### Solution - Fix a mistake in #5780 -- we did not check the correct directory with `os.path.isdir()`, so we never found the `LLVM` BinaryTool when downloading it from the LLVM download page. - Find libc using the new `LibcDev` subsystem. This uses the option `--libc-dir`, if provided, or finds an installation of libc with `crti.o` by invoking `--host-compiler` on the host once to get its search directories (this is necessary on travis, due to ubuntu's nonstandard installation location). - Expand the definition of executables, compilers, and linkers in `src/python/pants/backend/native/config/environment.py` to cover everything needed to actually compile, and give them the ability to generate an environment suitable for passing into `subprocess.Popen()`. - Introduce `GCCCCompiler`, `GCCCppCompiler`, `LLVMCCompiler`, and `LLVMCppCompiler` to differentiate between the two different compilers we have available for each language. - Expose the libc lib directory to the compilers we create in `native_toolchain.py`. - Unskip tests in `test_native_toolchain.py` from #5490. - Make the default `CCompiler` and `CppCompiler` into clang, for no particular reason (it will pass CI with gcc as well). The different compilers we can use will likely be denoted using variants in the future, but this solution allows us to separate the resources generated from each subsystem (`GCC`, `LLVM`, `Binutils`, `XCodeCLITools`) from a fully-functioning compiler that can be invoked (because actually invoking either clang or gcc requires some resources from the other, e.g. headers and libraries). Now, each binary tool only does the job of wrapping the resources it contains, while `native_toolchain.py` does the job of creating either a clang or a gcc compiler that we can invoke on the current host (with all necessary headers, libs, etc). ### Result The native toolchain tests from #5490 can be unskipped, and we can invoke our toolchain on almost any linux installation without further setup. The tests in `test_native_toolchain.py` now pass in CI, and as we iterate on the native backend, we will continue to have end-to-end testing for both of our compilers, on osx as well, regardless of whichever one we choose to use for `python_dist()` compilation.

stuhood reviewed Feb 20, 2018

View reviewed changes

stuhood requested review from kwlzn, CMLivingston and UnrememberMe February 20, 2018 22:57

UnrememberMe reviewed Feb 21, 2018

View reviewed changes

CMLivingston reviewed Feb 21, 2018

View reviewed changes

UnrememberMe approved these changes Feb 23, 2018

View reviewed changes

cosmicexplorer changed the title ~~use the LLVM subsystem to invoke a provided clang to compile native code in python_dist()~~ [WIP, needs testing] use the LLVM subsystem to invoke a provided clang to compile native code in python_dist() Mar 21, 2018

cosmicexplorer mentioned this pull request Mar 21, 2018

add gcc and binutils packages pantsbuild/binaries#60

Closed

cosmicexplorer force-pushed the dmcclanahan/python-dist-c++-sources branch from 9ae7284 to 3c3c7d2 Compare March 22, 2018 11:26

cosmicexplorer changed the title ~~[WIP, needs testing] use the LLVM subsystem to invoke a provided clang to compile native code in python_dist()~~ [needs binaries, testing] combine Clang, GCC, Binutils to form NativeToolchain and consume it in BuildLocalPythonDistributions Mar 22, 2018

cosmicexplorer commented Mar 22, 2018

View reviewed changes

baroquebobcat reviewed Mar 22, 2018

View reviewed changes

stuhood approved these changes Mar 26, 2018

View reviewed changes

cosmicexplorer force-pushed the dmcclanahan/python-dist-c++-sources branch from 61dd30a to 61503e6 Compare March 27, 2018 17:40

cosmicexplorer added 8 commits March 29, 2018 11:03

add some more BUILD files

5bad1a8

reduce platform-specific toolchains to ones we can handle right now

5765442

delete setup.py modifications from an earlier iteration and clarify c…

34c5125

…omments

remove added lines in the centos6 Dockerfile

93e760d

flesh out XCodeCLITools and add "sanity test" with hello world

94eca72

refactor the fasthello testproject to test both C and C++

e93a158

cosmicexplorer mentioned this pull request Mar 30, 2018

add gcc 7.3.0 and clang 6.0.0 cross platform, binutils 2.30 for linux, edit clang 5.0.1 to conform to #59 pantsbuild/binaries#62

Merged

stuhood approved these changes Mar 30, 2018

View reviewed changes

cosmicexplorer added 3 commits March 30, 2018 12:22

fix import order with isort.sh

fbc0b99

add a dependency that was definitely there last time i checked

f2b31d1

...fix lint errors...

21ae076

cosmicexplorer added 3 commits April 3, 2018 10:25

fix toolchain unit test failures and probably fix CI integration

9b73f7b

fix error message format string in native toolchain test

55dab09

don't isolate the path when invoking the native toolchain

b4feaa2

cosmicexplorer added 3 commits April 3, 2018 13:59

import os when using os in native toolchain test

b7a6184

prepend to PATH instead of isolating it when using native toolchain

7053d48

cut back on our use of the toolchain subsystem to pass CI

c35ac05

stuhood merged commit a389bba into pantsbuild:master Apr 5, 2018

cosmicexplorer mentioned this pull request Apr 14, 2018

Remodel of node subsystem #5698

Merged

cosmicexplorer mentioned this pull request May 10, 2018

Allow alternate binaries download urls generation and convert GoDistribution and LLVM subsystems to use it #5780

Merged

This was referenced Jun 8, 2018

C/C++ targets which can be compiled/linked and used in python_dist() with ctypes #5815

Merged

Introduce libc subsystem to find crti.o on linux hosts and unskip the native backend subsystem tests #5943

Merged

cosmicexplorer mentioned this pull request Jun 15, 2018

deprecate the contents of contrib/cpp #5967

Closed

		from pants.util.memo import memoized_method


		class SandboxedInterpreter(PythonInterpreter):


		options_scope = 'native-toolchain'

		PLATFORM_SPECIFIC_TOOLCHAINS = {

combine Clang, GCC, Binutils, and XCodeCLITools to form the NativeToolchain subsystem and consume it in BuildLocalPythonDistributions for native code #5490

combine Clang, GCC, Binutils, and XCodeCLITools to form the NativeToolchain subsystem and consume it in BuildLocalPythonDistributions for native code #5490

Conversation

cosmicexplorer commented Feb 20, 2018 • edited Loading

Problem

Solution

Result

stuhood 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

cosmicexplorer Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuhood Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cosmicexplorer Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

UnrememberMe 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

CMLivingston left a comment

Choose a reason for hiding this comment

cosmicexplorer commented Feb 21, 2018

cosmicexplorer commented Feb 21, 2018 • edited Loading

UnrememberMe commented Feb 21, 2018

cosmicexplorer commented Mar 21, 2018 • edited Loading

cosmicexplorer commented Mar 21, 2018 • edited Loading

cosmicexplorer left a comment • 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

baroquebobcat left a comment

Choose a reason for hiding this comment

stuhood commented Mar 26, 2018

stuhood 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

cosmicexplorer Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cosmicexplorer Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

illicitonion commented Mar 27, 2018

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cosmicexplorer commented Apr 2, 2018

stuhood commented Apr 3, 2018

cosmicexplorer commented Apr 3, 2018

cosmicexplorer commented Apr 3, 2018

cosmicexplorer commented Apr 5, 2018

cosmicexplorer commented Feb 20, 2018 •

edited

Loading

cosmicexplorer Feb 20, 2018 •

edited

Loading

stuhood Feb 21, 2018 •

edited

Loading

cosmicexplorer Feb 21, 2018 •

edited

Loading

cosmicexplorer commented Feb 21, 2018 •

edited

Loading

cosmicexplorer commented Mar 21, 2018 •

edited

Loading

cosmicexplorer commented Mar 21, 2018 •

edited

Loading

cosmicexplorer left a comment •

edited

Loading

cosmicexplorer Mar 28, 2018 •

edited

Loading

cosmicexplorer Mar 28, 2018 •

edited

Loading