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

Make FFI work on heroku cedar-14 #594

Closed
wants to merge 5 commits into from
Closed

Conversation

glitch003
Copy link

This small change adds a search path to the library lookup logic. It specifically adds the "/lib/x86_64-linux-gnu/" path which is where libraries live on heroku cedar-14.

This change keeps backwards compatibility with the existing search paths that were already present :)

@trostli
Copy link

trostli commented Nov 2, 2017

nice. i just ran into this issue myself

@trostli
Copy link

trostli commented Nov 3, 2017

Actually the other place where libraries live on Heroku is /app/vendor. Can you add that path too? That is where they are installed when using buildpacks

This makes FFI work with heroku buildpacks
@glitch003
Copy link
Author

@trostli sure, done

@trostli
Copy link

trostli commented Nov 3, 2017

Thanks!

@tduehr
Copy link
Member

tduehr commented Nov 10, 2017

That needs to be created specific to the platform currently being used rather than being hard coded to x86.

@danielpclark
Copy link

Shouldn't a server environment use values provided by ENV? This would allow any server or system to provide proper configurations which is easy enough to document.

# # Example
# ENV['FFI_LIBS'] = "/app/vendor/;/lib/x86_64-linux-gnu/"
path = ['/usr/lib/','/usr/local/lib/']
path += ENV['FFI_LIBS'].to_s.split(/[;,]/)
# # Path is now
# ['/usr/lib/', '/usr/local/lib/', '/app/vendor/', '/lib/x86_64-linux-gnu/']
path.find do |pth|
  File.exist?(pth + libname)
end

@trostli
Copy link

trostli commented Nov 11, 2017

good suggestion @danielpclark

@gsdean
Copy link

gsdean commented Feb 17, 2018

is this getting merged?

@headius
Copy link
Contributor

headius commented Feb 20, 2018

@gsdean I see @tduehr comment above but no follow-up commits, so I'm not sure if this is ready or not.

@larskanis
Copy link
Member

larskanis commented Feb 22, 2018

There is another PR which modifies the search path logic: #616 . To be honest, I would rather remove these hardcoded paths than adding random new ones. See my comment here.

@tduehr
Copy link
Member

tduehr commented Jan 25, 2019

Please rebase this if it's still an issue.

@dmtask
Copy link

dmtask commented Feb 11, 2019

When will this be finished?

@larskanis
Copy link
Member

larskanis commented Feb 11, 2019

This will not be merged. We'll not add architecture specific paths that are wrong on all other architectures.

FFI is not responsible for fixing misconfigurations in the library setup. FFI respects the library paths that are provided by the operating system. This is /etc/ld.so.conf and LD_LIBRARY_PATH for instance on Linux. They should be used instead of hardcoded paths within FFI.

To the contrary we'll remove the static paths entirely in a future release or limit then to operating systems that really need them.

@larskanis larskanis closed this Feb 11, 2019
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.

None yet

8 participants