Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subclass distutils UnixCCompiler for more control over building native extensions in setup.py invocation #5661

Closed
cosmicexplorer opened this issue Apr 5, 2018 · 2 comments · Fixed by #7126

Comments

@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Apr 5, 2018

It's really hard to invoke a setup.py and have it "just work" when providing a toolchain as in #5490. See this FIXME -- it looks like I may have restarted all the failing builds that spawned this comment, so the gist is that setup.py will fail because it cannot be told not to use x86_64-linux-gnu-gcc to link native extensions (even with CC=clang, which makes compilation to object files succeed). The FIXME describes subclassing distutils's UnixCCompiler (also see parent CCompiler) to control the compiler that is invoked.

The implementation of this would be simple: just compile all the files and link into shared libraries. You can see that's just what UnixCCompiler is doing in the link above. This would need to be provided to a setup.py subject of a python_dist() target, and we should probably try to make it so the setup.py doesn't have to be modified.

@cosmicexplorer
Copy link
Contributor Author

Thinking about implementing this -- was struck that it might be useful to spin this off into its own package on pypi that doesn't depend on the pants package (something like pantsbuild.pants.distutils_extensions maybe?). The idea would be that this code would just expose a better interface to invoke native compilation in setup.py for a build tool, and the build tool would take care of making pantsbuild.pants.distutils_extensions available for import. It could also not be exposed as its own pypi package, and just generated by Pants and added to the PYTHONPATH when invoking setup.py (which is probably a good first step). Does this sound reasonable?

@cosmicexplorer
Copy link
Contributor Author

Discussed with @stuhood offline and decided that decoupling this from its usage in Pants would make this harder than necessary at first, but I definitely think that makes sense after we hammer it out here.

cosmicexplorer added a commit that referenced this issue Jan 25, 2019
…tensions in setup.py (#7126)

### Problem

Resolves #7016 (see [mirrored `pants-devel` post](https://groups.google.com/forum/#!topic/pants-devel/Y37d0tf4bKo)). This unblocks #6855 and #7046, as well as further work on #7122.

Closes #5661, closes #6841. I also made a long, long-overdue [github project for native code](https://github.com/pantsbuild/pants/projects/11).

### Solution

- Use the host environment to invoke compilers and linkers as desired in distutils, don't try to inject our own toolchain. See #5661 and #7016 for why this is extremely difficult to maintain. 

### Result

Further native backend iteration is unblocked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants