conda-forge / openblas-feedstock

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

Fix install name in OSX #32

Closed isuruf closed 7 years ago

isuruf commented 7 years ago

Fixes the install name of the dylib so that the downstream projects will link to libopenblas.dylib instead of libopenblasp-r0.2.20.dylib In Linux, SONAME is libopenblas.so.0 instead of libopenblasp-r0.2.20.so, so no change needed.

This change means the pinning doesn't have to be restrictive as before (https://github.com/conda-forge/conda-forge.github.io/pull/418) and can be as below

requirements:
   build:
     - openblas 0.2.20
   run:
     - openblas >=0.2.15

One other advantage is that this achieves compatibility with the defaults openblas and therefore a package compiled with conda-forge openblas can use the defaults' openblas at runtime.

cc @ocefpaf

conda-forge-linter commented 7 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

ocefpaf commented 7 years ago
   run:
     - openblas >=0.2.15

If this is true we can still build with openblas 0.2.19 as long as the same fixes are applied there, right?

(Just asking. I am +1 to build everything openblas 0.2.20.)

We also need finish the pin build/run PR to pin_slow_way.py to use this.

mingwandroid commented 7 years ago

Sounds perfectly reasonable. All downstreams will need to be rebuilt though to gain the benefit.

ocefpaf commented 7 years ago

Awesome! Thanks for the feedback and thanks @isuruf for the nice insight.

We'll pin to the latest version and I'll remove the previous pin number so everything uses this one from now on.

On Jul 26, 2017 6:36 AM, "Ray Donnelly" notifications@github.com wrote:

Sounds perfectly reasonable. All downstreams will need to be rebuilt though to gain the benefit.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conda-forge/openblas-feedstock/pull/32#issuecomment-318002686, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6BL5AtmoghQ3IoPxJPMclzo4aO-DNjks5sRwicgaJpZM4OjWcl .

isuruf commented 7 years ago

Thanks for merging.

If this is true we can still build with openblas 0.2.19 as long as the same fixes are applied there, right?

Yes, but it'll be hard to keep track of which build number of openblas it was built against, so a new version (0.2.20) is better.

ocefpaf commented 7 years ago

Yes, but it'll be hard to keep track of which build number of openblas it was built against, so a new version (0.2.20) is better.

Agreed. But then the pinning should be:

requirements:
   build:
     - openblas 0.2.20
   run:
     - openblas >=0.2.20

to be on the safe side, right?

isuruf commented 7 years ago

Advantage of openblas >=0.2.15 is that packages compiled with 0.2.20 can be used with packages compiled with 0.2.19. According to https://abi-laboratory.pro/tracker/timeline/openblas/ no symbols were added or removed from 0.2.15-0.2.20. (Ignore the changed soname in openblas 0.2.20, they have created a debug build). I think that means we have forward compatibility.

ocefpaf commented 7 years ago

(Ignore the changed soname in openblas 0.2.20, they have created a debug build). I think that means we have forward compatibility.

I am under the impression that the soname will be would be an issue with the 0.2.19 that we have in the channel. If not let's use openblas >=0.2.15.

isuruf commented 7 years ago

soname is libopenblas.so.0 in linux and libopenblas.dylib in osx with the 0.2.20 package. Even though the sonames are different in 0.2.19, both of these lbraries are available in the 0.2.19 package

ocefpaf commented 7 years ago

Got it ! Thanks for the explanation @isuruf. I'll start using that pinning scheme. (Although we need to improve the pinning script to do the same.)

jakirkham commented 7 years ago

Isn't libopenblas.dylib a symlink though? If so, I don't see how the older packages will work.

isuruf commented 7 years ago

@jakirkham, what I was saying was if you have A compiled with 0.2.19 and B compiled with 0.2.20 you can have A and B with 0.2.19 at runtime.

isuruf commented 7 years ago

@ocefpaf, you were right. Pinning should be following because abi-laboratory reports don't show the symbols created when compiling with DYNAMIC_ARCH=1 which may have more symbols when new CPU architectures are added.

requirements:
   build:
     - openblas 0.2.20
   run:
     - openblas >=0.2.20
ocefpaf commented 7 years ago

Cool. I'll update the PRs. Thanks @isuruf!

(But only numpy is using that now. We need to fix the SciPy issue first.)

jschueller commented 5 years ago

@isuruf Does this affect the rpath ? I'm investigating why delocate is not finding openblas dependencies on osx.