search

conda-forge / openblas-feedstock

A conda-smithy repository for openblas.
BSD 3-Clause "New" or "Revised" License
9 stars 38 forks source link

compatibility with defaults #27

Closed jschueller closed 7 years ago

jschueller commented 7 years ago

in defaults they set TARGET=NEHALEM and the lib file is libopenblas_nehalemp-r0.2.19.dylib instead of libopenblasp-r0.2.19.dylib

maybe it could be renamed, or that option set ? see https://github.com/conda-forge/arpack-feedstock/issues/5 for the full report

isuruf commented 7 years ago

Adding the TARGET=NEHALEM is not the correct approach. In conda-forge recipe we have DYNAMIC_ARCH=1 set which means the code is compiled for different architectures including nehalem and then the correct code is dispatched at package load time.

jschueller commented 7 years ago

and renaming ?

isuruf commented 7 years ago

A soft link should work. Renaming would break all of the packages depending on openblas.

isuruf commented 7 years ago

Ignore my last comment. Renaming won't work because the dependent libraries will still be linked to libopenblasp-r0.2.19.dylib even if the dylib's name changed. (Dependent libraries store the install_name of the dylib, not the name of the file.)

jschueller commented 7 years ago

as @jakirkham, it's not really supported, in fact we want compatibility in the other way around

I could have added something like this though:

# compatibility with defaults, which uses TARGET=NEHALEM instead of DYNAMIC_ARCH
ln -fs $PREFIX/lib/libopenblas$SHLIB_EXT $PREFIX/lib/libopenblas_nehalemp-r${PKG_VERSION}${SHLIB_EXT}
jakirkham commented 7 years ago

Thanks @isuruf. Indeed defaults is building a suboptimal copy of openblas and have been for a long time. I don't know why they are continuing to do this. Maybe they are simply unaware. It might be worth raising this to them or PRing a fix. OTOH maybe they should just use our recipe. 😄

jakirkham commented 7 years ago

As a side note, this reminds me literally how much information we are carrying around in our heads about things like how OpenBLAS is built for instance. Maybe we need a README in the recipe to explain why we use the settings we do and what they mean.