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

[WIP] Build system changes to allow pip installation from source #19

Closed

Conversation

tilmantroester
Copy link
Contributor

This PR attempts to unify the build process such that a single python setup.py install builds and installs the whole library. Ditto for pip.
To accomplish this, libchord.so is now being compiled at the build_py step (for stupid, setuptools-related reasons) and then copied into the package.
The extension is compiled with the right rpath, $ORIGIN, and install_name settings so that things work. Getting this to work cost me a good chunk of my sanity and will probably leave me with setuptools related PTSD for the foreseeable future.

To summarise:

  • python setup.py install, pip install ., pip install git+https://github.com/tilmantroester/PolyChordLite.git@compile_stuff_for_pip#egg=pypolychord seem to work on both Linux and macOS for MPI.
  • A no-MPI install is also possible by setting the --no-mpi flag. Note that the flag position is after setup.py, e.g., python setup.py --no-mpi install, for stupid, setuptools-related reasons.
  • There's now a python setup.py clean command that runs make veryclean

This should probably be tested in more detail. At the moment it is based of the setup_pip branch but I think it could easily be rebased to master.

Copy link
Member

@williamjameshandley williamjameshandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an extremely impressive piece of work, and seems to work very well on my machine. Let's try and get the build system working, and we may be able to get pypolychord working for macs as well from pip!

ifeq ($(MPI), 1)
FC = mpif90

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this removal of the FC variable is causing the continuous integration travis build to fail. Perhaps you could take a look to see if .travis.yml also needs to be updated (I think this would involve adjusting the build.sh script so it uses your new command, rather than make)? It also no longer works if we try to build it the old way, i.e. using make.

LDLIBS += -lstdc++
# clang uses a different libstdc++ than GCC
ifneq (,$(findstring clang,$(shell '$(CXX)' -v 2>&1)))
LDLIBS += -lc++

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice spot if this allows it to work for macs -- I think many people have run into problems with clang in the past, but nobody has suggested this as a solution

@williamjameshandley
Copy link
Member

williamjameshandley commented Apr 14, 2019

Potentially fixes #13 and #11

@tilmantroester
Copy link
Contributor Author

Should be replaced by PR #24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants