Add documentation about linking in Docker

rails-sqlserver / tiny_tds

TinyTDS - Simple and fast FreeTDS bindings for Ruby using DB-Library.

Other

607 stars 189 forks source link

Add documentation about linking in Docker #420

Closed mlh758 closed 5 years ago

mlh758 commented 5 years ago

Closes #419

I removed documentation that said TinyTDS would install FreeTDS for you since that seems to have been removed in version 2. I can remove that change if it's unwanted.

aharpervc commented 5 years ago

Questions raised purely from reading this readme and not closely investigating the other issue:

if freetds is normally in /usr/local/lib, and tinytds already knows how to load from /usr/local/lib, what is the purpose of setting LD_LIBRARY_PATH to /usr/local/lib?
when talking about how to set this up with a multistage docker build, wouldn't it be better to install to another directory than default, so your COPY command is more obviously useful (I get that it's copying from a prior stage to the current stage, but when it's the same folder path in both stages it's less obviously interesting)
should the url to extconf.rb be to this repo rather than your fork?

mlh758 commented 5 years ago

The extconf.rb file knows where to look, but that only helps you at gem build time. When you move it to another container, that link breaks and the OS will try to fix it, but LD_LIBRARY_PATH needs to be updated to show it where to look.
You could, but then your builder container has to be a little more complex and set the Free TDS location environment variable which doesn't really get you anything in this case
It definitely should be, yes. I'll fix that now.

mlh758 commented 5 years ago

Are you okay with the changes now?

coderjoe commented 5 years ago

Just re-read it. Should we really be suggesting a complete overwrite of LD_LIBRARY_PATH? We should really just be adding /usr/local/lib to the end of the existing path right?

Something like export LD_LIBRARY_PATH=/usr/local/lib:${LD_LIBRARY_PATH} maybe?

coderjoe commented 5 years ago

Alternatively we could suggest installing FreeTDS to /usr/lib instead of /usr/local/lib and then maybe you don't have to change LD_LIBRARY_PATH at all?

mlh758 commented 5 years ago

export LD_LIBRARY_PATH=/usr/local/lib:${LD_LIBRARY_PATH} would be safer, it's just normally blank and I assumed the reader would know to preserve it if necessary. I'll change it just to be safer by default.

I don't know why free tds uses /usr/local/lib instead, but tiny_tds also knows to look there and I'm not sure if changing that on the installed system would cause any other weird side effects.

coderjoe commented 5 years ago

Looks good to me. Thanks. 👍

mlh758 commented 5 years ago

Thank you for merging!