-
Notifications
You must be signed in to change notification settings - Fork 188
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
Provide precompiled gem for Linux #575
base: master
Are you sure you want to change the base?
Provide precompiled gem for Linux #575
Conversation
the PR is almost ready.
|
You can use this parameter to specify a `freetds.conf` there. per default, it will use the build path, which will be some obscure thing on GitHub Actions. Although most people will customize this location at runtime using environment variables, it still makes sense to provide sensible defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, amazing 😮
It has been 8 years since the old build process was touched,
Yeah ugh, incredible that you had the time to pick this apart...
I tried that out, and it resulted in a massive gem file (60 MB compressed), since we had all the dependencies times 6, for each supported Ruby version. So this was not feasable.
I'm not understanding this, if we're shipping binaries now why is it different static vs dynamic, wouldn't the binary be different per-build-target? Or are you saying that approach builds/ships all targets in 1 gem? Also what do you mean by times 6?
Ports are now build as part of rake compile when cross-compiling instead of having a separate task.
Can you share more of your rationale here? It seems like having a separate task is useful to debug compiling tiny tds C vs dependency C. Also if you're choosing to build dynamic libraries doesn't that mean we'd have separate compile tasks regardless? So I don't understand merging the rake tasks like this
Windows & Linux
Lastly for the moment, (maybe you said this), why not also macOS? It seems virtuous to treat all OS's equally for compilation/distribution if possible
To say it... incredible...!
Rakefile
Outdated
CrossLibraries = [ | ||
['x64-mingw-ucrt', 'mingw64'], | ||
['x64-mingw32', 'mingw64'], | ||
['x86_64-linux-gnu', 'linux-x86_64'], | ||
['x86_64-linux-musl', 'linux-x86_64'], | ||
['aarch64-linux-gnu', 'linux-aarch64'], | ||
['aarch64-linux-musl', 'linux-aarch64'], | ||
].map do |platform, openssl_config| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some weird indentation here, is it tabs vs spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think introducing a formatter for my own sake would be good 😅
... #574 :)
lib/tiny_tds.rb
Outdated
# libpq is found by a relative rpath in the cross compiled extension dll | ||
# or by the system library loader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's libpq?
lib/tiny_tds.rb
Outdated
block.call | ||
end | ||
end | ||
|
||
# Temporary add bin directories for DLL search, so that freetds DLLs can be found. | ||
add_dll_paths.call( TinyTds::Gem.ports_bin_paths ) do | ||
# Add a load path to the one retrieved from pg_config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait did this get copy/pasted from pg 😆
ext/tiny_tds/extconf.rb
Outdated
@@ -115,6 +115,7 @@ def configure_defaults | |||
|
|||
recipe.configure_options << "--with-openssl=#{openssl_recipe.path}" | |||
recipe.configure_options << "--with-libiconv-prefix=#{libiconv_recipe.path}" | |||
recipe.configure_options << "--sysconfdir=#{MiniPortile.windows? ? "C:/Sites" : "/usr/local/etc"}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should this come from env var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A user can overwrite it at runtime:
https://www.freetds.org/userguide/freetdsconf.htm
It's maybe easier to show. This is the structure of a precompiled
Now if we look into the file size of one of these files when everything is statically compiled, it comes out as follows:
So it's 9.2 MB. For each supported Ruby version, each of these shared library is 9.2 MB since all dependencies are statically compiled into it. This is what I meant by times 6.
It was originally just to align with I just tried to separate the builds and I am not super happy about it. When we compile FreeTDS statically with OpenSSL and iconv, when compiling When both ports and
to be honest, installation of FreeTDS on Mac OS is really easy and I personally do not have a need for a precompiled version on Mac. If there is a need by the community, I am happy to ship it at a later point. |
so:
otherwise I think this PR is ready to merge. |
I'm still working through the implications here & running the branch locally. However I'm trying to remember what about this makes them different for different Ruby versions. I understand why tiny_tds itself needs to be compiled And then if I'm getting it, does the equation change here if we switched to dynamic linking (still providing the compiled dependencies blob in the gem)? freetds/libiconv/openssl won't have the Ruby dependency and could be compiled once (per target platform/architecture), and then we have that dependency blob + 6 tiny_tds.so's. And that tiny_tds.so would be really small (...tiny... 😆) so 6x doesn't seem so bad. I re-read your comments above but I'm still unclear on why you're preferring to avoid dynamic linking, can you expand on that more? If we did it that way, Windows & Linux would be more similar to macOS, right? Because in all cases the dependencies are dynamically linked on macOS (we are declining to provide a statically compiled fat gem for that OS) |
Okay, I built the gem from this branch okay but it failed to install. Does this seem like a "me problem" or something changed here?
(never mind, "me problem") |
well this is somewhat complicated to explain over text 😅.
Correct.
I think we have a misunderstanding here: I do not think linking everything statically in our case is better, because of the size of the dependencies.
This is now exactly how it works now with this PR. One large dependency blob, 6 tiny
Just for completeness / comparison: This is how the ports look for the current version of
basically, everything is a shared library and dynamically linked. When starting |
Closes #569.
Background
If I read that correctly from the project's history,
tiny_tds
used to ship with an extensivemini_portile
configuration prior to v2 to build FreeTDS. This has been removed with v2, see #345. Now you need to bring your own FreeTDS, whichtiny_tds
will find and compile against.Compiling FreeTDS yourself is not too complicated, especially since you can follow along with the README. However, I noted the general sentiment of the Ruby community of shipping gems precompiled to you, not only for Windows, but also Linux and Mac OS. nokogiri and sqlite3-ruby have been doing this for a while, and I saw that the pg gem prepared for a Linux build. So I feel it would be good to follow along and also provide a precompiled gem for Linux.
Approach
There are two approaches on how you can ship precompiled gems: static or dynamic. On that note, we also have to know that for each supported Ruby version of the precompiled gem, a separate compiled file has to be shipped. Right now, our Windows gem ships with 6 different compiled
tiny_tds
files for each supported Ruby version.If you would ship with a statically compiled
tiny_tds
file, all our dependencies (FreeTDS, iconv, OpenSSL) are squeezed into the resultingtiny_tds
files. I tried that out, and it resulted in a massive gem file (60 MB compressed), since we had all the dependencies times 6, for each supported Ruby version. So this was not feasable.Dynamic means: You compile a library with all its dependencies visible and tell it "you will find it at runtime" again, don't worry. It is a good way to safe space. This is also how the current
tiny_tds
build works. After the results of building the static version, I felt like this is the best way to go, considering our dependency size.Changes
First commit: Partially static build
I heavily looked into what has been done in the pg, and decided to replicate the changes in order to have it easier to implement the Linux support. So the first commit is mostly aligning the build system.
The gist of the changes is:
rake compile
when cross-compiling instead of having a separate task.Next 4 commits: Support for Linux
The first commit contains the most changes.
Generally, I decided to ship four different Linux gems:
Then most of it was declaring Windows and Linux specific compiler flags.
There is one interesting thing, which is this line in
extconf.rb
:$LDFLAGS << " '-Wl,-rpath=$$ORIGIN/../../../ports/#{gem_platform}/lib'"
When the linker links our precompiled
tiny_tds
gem against FreeTDS, it passes it "runtime paths", basically information where it find different required libraries, like FreeTDS. Of course, our gem could be installed anywhere, so we cannot use absolute paths here. This little flag tells the linker to tell the resulting file that it should look in our ports directory from its current location for the FreeTDS library.I also extended the test suite in this commit to test the resulting files and make sure they pass the tests. I do not run tests for anything else than the regular x86_64 GNU build, but I test the installation on the other platforms in Docker according to a template by flavorjones.
Last commit: Fix binstubs
tiny_tds
ships with binstubs to calldefncopy
andtsql
from FreeTDS.tsql
is for connection diagnostics,defncopy
is used by the SQL server adapter to dump schemas. I noted that these have been broken prior to my changes, so I thought it might be good to fix them.tsql
. On Windows, thetsql.exe
by themsys2
system did not pass thebinary?
check. I removed it altogether since I did not see an advantage of having it.tsql
anddefncopy
where to find itssybdb
library file with a runtime path, similar as with thetiny_tds
files.Consideration
While this PR will simply the installation for
tiny_tds
on Linux, and reduces the size for Windows users, it makes our build process more complicated and also larger because of many additional tests.As I have written in the initial paragraph, I would like to follow the example of other gems and simplify the installation process. Preparing this PR cost me a lot of time since I was not familiar with the C build system at all (and FreeTDS makes things extra difficult for a beginner with cmake and libtool). It has been 8 years since the old build process was touched, and I expect a similar life span for these changes. I do not really expect more maintenance effort from our side, but it will require some more frequent updates to keep the dependencies inside the gem up-to-date.