manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
163 stars 50 forks source link

Pip fixes #209

Closed manodeep closed 4 years ago

manodeep commented 4 years ago

Addresses #208

manodeep commented 4 years ago

@lgarrison I have copied over the constructs within python(3)-config --includes/--prefix/--libs into the common.mk file. However, there is another potentially relevant construct corresponding to --ldflags. I am not sure whether or not we should be using --libs or --ldflags to get the python library link flags. Do you know?

(--ldflags contains all of --libs and then potentially some additional entries)

lgarrison commented 4 years ago

I'm not positive, but based on some quick searches I would guess --ldflags? Our intention is to pass these commands to the linker, after all.

On Mon, Jan 20, 2020 at 6:51 PM Manodeep Sinha notifications@github.com wrote:

@lgarrison https://github.com/lgarrison I have copied over the constructs within python(3)-config --includes/--prefix/--libs into the common.mk file. However, there is another potentially relevant construct corresponding to --ldflags. I am not sure whether or not we should be using --libs or --ldflags to get the python library link flags. Do you know?

(--ldflags contains all of --libs and then potentially some additional entries)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/pull/209?email_source=notifications&email_token=ABLA7S64HMPIUVVJV664SZDQ6Y2IFA5CNFSM4KJK5E42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJOCC6Y#issuecomment-576463227, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7S22QXFDXYB46ZMKRULQ6Y2IFANCNFSM4KJK5E4Q .

-- Lehman Garrison lgarrison@flatironinstitute.org Flatiron Research Fellow, Cosmology X Data Science Group Center for Computational Astrophysics, Flatiron Institute lgarrison.github.io

manodeep commented 4 years ago

@lgarrison My memory is vague but looking at what's commented out for PYTHON_LINK for Darwin, I suspect that we had --ldflags in the PYTHON_LINK.

Found it - committed in 2015 -- here

manodeep commented 4 years ago

If I am remembering correctly, the logic to removing the --ldflags was to remove any explicit links to the python shared library (following how conda works). Plus, the additional flags from --ldflags was causing issues on OSX and TRAVIS. I am for leaving the install as is (i.e., with --libs). I am happy to be convinced otherwise...

manodeep commented 4 years ago

@lgarrison Should we make a release and update on PyPI?

lgarrison commented 4 years ago

Maybe we should double check that the original issue is resolved. I'll ask the original reporter.