rails-sqlserver / tiny_tds

TinyTDS - Simple and fast FreeTDS bindings for Ruby using DB-Library.
Other
606 stars 189 forks source link

On Windows add the msys path where freetds is automatically installed #536

Closed Largo closed 11 months ago

Largo commented 1 year ago

(C:/Ruby32-x64/msys64/ucrt64/include/freetds)

This should fix the issue where users on windows can't just run gem install tiny_tds

andyundso commented 1 year ago

Hi @Largo

Many thanks for the contribution! I quickly checked on a virtual machine how the current release behaves.

gem install .\tiny_tds-2.1.5.gem
Temporarily enhancing PATH for MSYS/MINGW...
Installing required msys2 packages: mingw-w64-x86_64-freetds
Building native extensions. This could take a while...
ERROR:  Error installing .\tiny_tds-2.1.5.gem:
        ERROR: Failed to build gem native extension.

    current directory: C:/Ruby30-x64/lib/ruby/gems/3.0.0/gems/tiny_tds-2.1.5/ext/tiny_tds
C:/Ruby30-x64/bin/ruby.exe -I C:/Ruby30-x64/lib/ruby/3.0.0 -r ./siteconf20230522-4676-4iwbhb.rb extconf.rb
looking for freetds headers in the following directories:
 - /opt/local/include
 - /opt/local/include/freetds
 - /usr/local/include
 - /usr/local/include/freetds
looking for freetds library in the following directories:
 - /opt/local/lib
 - /opt/local/lib/freetds
 - /usr/local/lib
 - /usr/local/lib/freetds
checking for sybfront.h... no
checking for sybdb.h... no
checking for tdsdbopen() in -lsybdb... yes
checking for dbanydatecrack() in -lsybdb... yes
Failed! Do you have FreeTDS 0.95.80 or higher installed?

Building a version with your code works.

gem install .\tiny_tds-2.1.5-fixed.gem
Temporarily enhancing PATH for MSYS/MINGW...
Using msys2 packages: mingw-w64-x86_64-freetds
Building native extensions. This could take a while...
Successfully installed tiny_tds-2.1.5
Parsing documentation for tiny_tds-2.1.5
Installing ri documentation for tiny_tds-2.1.5
Done installing documentation for tiny_tds after 1 seconds
1 gem installed

As I'm not really sure how to handle the interaction between Ruby Installer, especially the metadata property msys2_mingw_dependencies and extconf.rb I did some research on other projects that use C code and have Windows support. My conclusion is that everybody solves it different. Our extconf.rb is quite simple, while others have easily thousand lines of code. So I think your fix fits well to keep the code complexity low.

@aharpervc any opinions on this? otherwise I would approve the changes. @Largo in the meantime, would you mind to add an entry in the changelog to the unreleased section?

aharpervc commented 1 year ago

I don't have a specific opinion, but if we can repro the error AND this PR no longer has the error (AND doesn't introduce new errors, AND the CI jobs all run successfully), then it's fine with me.

That all seems to be the case for this PR, except now needing to rebase again because of the changelog. But that can easily be fixed and this PR merged.

andyundso commented 11 months ago

Closing this PR in favor of #544, which includes the same changes plus testing on CircleCI.