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.

Sign up for GitHub

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

Update dependencies for Windows build and drop 32-bit support #567

Merged
merged 9 commits into from
Dec 9, 2024
Merged

Update dependencies for Windows build and drop 32-bit support #567

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9226df1
Replace `setup-mssql` action
andyundso Nov 14, 2024
2bff820
Add new `force encryption` scenario on Windows
andyundso Nov 19, 2024
c96642a
Bump freetds to v1.4.23
andyundso Nov 19, 2024
645dd23
Bump libiconv to v1.17
andyundso Nov 19, 2024
aacd7ed
Fix cache restore keys for ports
andyundso Nov 19, 2024
c67e7c7
Bump OpenSSL to v3.4.0
andyundso Nov 19, 2024
60c6674
Drop support for 32-bit Windows
andyundso Nov 19, 2024
8deb5b2
Pass `force-encryption` as YAML boolean
andyundso Dec 9, 2024
681301c
`setup-mssql` is now owned by `rails-sqlserver`
andyundso Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ jobs:
matrix:
platform:
- "x64-mingw32"
- "x86-mingw32"
- "x64-mingw-ucrt"
name: cross-compile-windows
runs-on: ubuntu-22.04
Expand All @@ -35,10 +34,10 @@ jobs:
uses: actions/cache@v4
with:
path: ports
key: cross-compiled-${{ hashFiles('**/.ports_versions') }}
key: cross-compiled-v3-${{ matrix.platform }}-${{ hashFiles('**/.ports_versions') }}
restore-keys: |
cross-compiled-${{ hashFiles('**/.ports_versions') }}
cross-compiled-v2-
cross-compiled-v3-${{ matrix.platform }}-${{ hashFiles('**/.ports_versions') }}
cross-compiled-v3-${{ matrix.platform }}-

- name: Build gem
shell: bash
Expand Down Expand Up @@ -96,8 +95,11 @@ jobs:
strategy:
fail-fast: false
matrix:
force-encryption:
- "false"
- "true"
mssql-version:
#- 2017
- 2017
- 2019
- 2022
ruby-version:
Expand Down Expand Up @@ -138,12 +140,12 @@ jobs:
Copy-Item -Path ".\tmp\tiny_tds-$gemVersion-$rubyArchitecture\ports" -Destination "." -Recurse

- name: Setup MSSQL
uses: potatoqualitee/mssqlsuite@v1.7
uses: andyundso/setup-mssql@v1
andyundso marked this conversation as resolved.
Show resolved Hide resolved
with:
install: sqlengine, sqlclient
components: sqlcmd,sqlengine
version: ${{ matrix.mssql-version }}
sa-password: c0MplicatedP@ssword
show-log: true
force-encryption: ${{ matrix.force-encryption }}

- name: Setup MSSQL database
shell: pwsh
Expand Down Expand Up @@ -217,8 +219,11 @@ jobs:
strategy:
fail-fast: false
matrix:
force-encryption:
- "false"
- "true"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these strings instead of actual true/false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same reason as with the input array: since setup-mssql is a composite action, it will coerce any value to be a string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's on the receiver side though right? I'm thinking of it from our side, the user side, where normal values seem more ergonomic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, receiver's end.

mssql-version:
#- 2017
- 2017
- 2019
- 2022
ruby-version:
Expand Down Expand Up @@ -259,12 +264,12 @@ jobs:
Copy-Item -Path ".\tmp\tiny_tds-$gemVersion-$rubyArchitecture\ports" -Destination "." -Recurse

- name: Setup MSSQL
uses: potatoqualitee/mssqlsuite@v1.7
uses: andyundso/setup-mssql@v1
with:
install: sqlengine, sqlclient
components: sqlcmd,sqlengine
version: ${{ matrix.mssql-version }}
sa-password: c0MplicatedP@ssword
show-log: true
force-encryption: ${{ matrix.force-encryption }}

- name: Setup MSSQL database
shell: pwsh
Expand Down Expand Up @@ -345,10 +350,10 @@ jobs:
uses: actions/cache@v4
with:
path: ports
key: native-v2-${{ hashFiles('**/.ports_versions') }}
key: native-v3-${{ hashFiles('**/.ports_versions') }}
restore-keys: |
native-${{ hashFiles('* */.ports_versions') }}
native-v2-
native-v3-${{ hashFiles('* */.ports_versions') }}
native-v3-

- name: Build required libraries
run: |
Expand Down Expand Up @@ -388,20 +393,19 @@ jobs:
uses: actions/cache@v4
with:
path: ports
key: native-v2-${{ hashFiles('**/.ports_versions') }}
key: native-v3-${{ hashFiles('**/.ports_versions') }}
fail-on-cache-miss: true

- name: Build gem
run: |
bundle exec rake build

- name: Setup MSSQL
uses: potatoqualitee/mssqlsuite@v1.7
uses: andyundso/setup-mssql@v1
with:
install: sqlengine, sqlclient
components: sqlcmd,sqlengine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a yaml list instead of a comma delimited string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are different types of custom GitHub Actions you can build: Composite, Docker and JavaScript. The last one is the only one that supports other types than strings, see discussion here.

Means the setup-mssql action will receive this comma-separated list as string. The action is a composite action and uses PowerShell behind the scenes. This comma-separated list format is easier to parse from PowerShell.

version: ${{ matrix.mssql-version }}
sa-password: "c0MplicatedP@ssword"
show-log: true

- name: Setup MSSQL database
run: |
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
* Drop support for Ruby < 2.7
* Drop support for SQL Server < 2017
* Drop support for FreeTDS < 1.0
* No longer provide a 32-bit Windows build
* Raise error if FreeTDS is unable to sent command buffer to the server
* Use freetds v1.4.23, libiconv v1.17 and OpenSSL v3.4.0 for Windows builds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. extconts sets these versions for all platforms, right? And it's that their statically linked for Windows. But building for other platforms would also use these versions?
  2. Any concern doing this large of a version jump for OpenSSL?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extconts sets these versions for all platforms, right? And it's that their statically linked for Windows. But building for other platforms would also use these versions?

If you would build tiny_tds using our "ports" mechanic, you will also get these versions. Our Linux CI build actually also uses these versions. For installations outside of our CI, the discovered freetds version is used, which could be anything.

Any concern doing this large of a version jump for OpenSSL?

There is a chance that something breaks. As mentioned on the PR, I wanted to make sure that tiny_tds generally works when using encrypted connections by adding this new scenario on CI. But I can imagine that there are some configurations that will no longer work with this new OpenSSL version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you would build tiny_tds using our "ports" mechanic, you will also get these versions. Our Linux CI build actually also uses these versions. For installations outside of our CI, the discovered freetds version is used, which could be anything.

Right, exactly. That's what most people will be using across all OS's since we go from CI artifact -> rubygems.org, right? So it seems more accurate to drop the "for Windows" part of the sentence is what I'm saying

There is a chance that something breaks. As mentioned on the PR, I wanted to make sure that tiny_tds generally works when using encrypted connections by adding this new scenario on CI. But I can imagine that there are some configurations that will no longer work with this new OpenSSL version.

Is there any other consideration to add to the changelog? Minimum OS version, minimum glibc version, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what most people will be using across all OS's since we go from CI artifact -> rubygems.org, right?

Not exactly.

When you install tiny_tds on Linux or Mac OS, Rubygems will discover that we have some C code in the gem and will try to compile it. In order for tiny_tds to compile, it tries to discover an installation of freetds. This process does not invoke our ports mechanic in case freetds is missing, but fails instead.

So all people on non-Windows machines will compile freetds on their own first, likely according to our installation instructions.

Mid-term I would like to explore if we could provide precompiled versions of tiny_tds for all platforms, similar as Nokogiri did a while back. It would simplify the installation for the end-user.

Is there any other consideration to add to the changelog? Minimum OS version, minimum glibc version, etc?

Don't think so. I still would like to release the next version as v3 just to signal that there might be breaking changes.


## 2.1.7
* Add Ruby 3.3 to the cross compile list
Expand Down
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ $ apt-get install wget
$ apt-get install build-essential
$ apt-get install libc6-dev

$ wget http://www.freetds.org/files/stable/freetds-1.4.10.tar.gz
$ tar -xzf freetds-1.4.10.tar.gz
$ cd freetds-1.4.10
$ wget http://www.freetds.org/files/stable/freetds-1.4.23.tar.gz
$ tar -xzf freetds-1.4.23.tar.gz
$ cd freetds-1.4.23
$ ./configure --prefix=/usr/local --with-tdsver=7.4
$ make
$ make install
Expand Down
4 changes: 0 additions & 4 deletions Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ ruby_cc_ucrt_versions = "3.3.0:3.2.0:3.1.0".freeze
ruby_cc_mingw32_versions = "3.0.0:2.7.0".freeze

GEM_PLATFORM_HOSTS = {
'x86-mingw32' => {
host: 'i686-w64-mingw32',
ruby_versions: ruby_cc_mingw32_versions
},
'x64-mingw32' => {
host: 'x86_64-w64-mingw32',
ruby_versions: ruby_cc_mingw32_versions
Expand Down
1 change: 0 additions & 1 deletion ext/tiny_tds/extconf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def do_help

# Make sure to check the ports path for the configured host
architecture = RbConfig::CONFIG['arch']
architecture = "x86-mingw32" if architecture == "i386-mingw32"

project_dir = File.expand_path("../../..", __FILE__)
freetds_ports_dir = File.join(project_dir, 'ports', architecture, 'freetds', FREETDS_VERSION)
Expand Down
6 changes: 3 additions & 3 deletions ext/tiny_tds/extconsts.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@

ICONV_VERSION = ENV['TINYTDS_ICONV_VERSION'] || "1.15"
ICONV_VERSION = ENV['TINYTDS_ICONV_VERSION'] || "1.17"
ICONV_SOURCE_URI = "http://ftp.gnu.org/pub/gnu/libiconv/libiconv-#{ICONV_VERSION}.tar.gz"

OPENSSL_VERSION = ENV['TINYTDS_OPENSSL_VERSION'] || '1.1.1s'
OPENSSL_VERSION = ENV['TINYTDS_OPENSSL_VERSION'] || '3.4.0'
OPENSSL_SOURCE_URI = "https://www.openssl.org/source/openssl-#{OPENSSL_VERSION}.tar.gz"

FREETDS_VERSION = ENV['TINYTDS_FREETDS_VERSION'] || '1.1.24'
FREETDS_VERSION = ENV['TINYTDS_FREETDS_VERSION'] || '1.4.23'
FREETDS_SOURCE_URI = "http://www.freetds.org/files/stable/freetds-#{FREETDS_VERSION}.tar.bz2"
4 changes: 2 additions & 2 deletions tasks/ports.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ namespace :ports do
if libraries_to_compile[:openssl]
# freetds doesn't have an option that will provide an rpath
# so we do it manually
ENV['OPENSSL_CFLAGS'] = "-Wl,-rpath -Wl,#{libraries_to_compile[:openssl].path}/lib"
ENV['OPENSSL_CFLAGS'] = "-Wl,-rpath -Wl,#{libraries_to_compile[:openssl].path}/lib64"
# Add the pkgconfig file with MSYS2'ish path, to prefer our ports build
# over MSYS2 system OpenSSL.
ENV['PKG_CONFIG_PATH'] = "#{libraries_to_compile[:openssl].path.gsub(/^(\w):/i) { "/" + $1.downcase }}/lib/pkgconfig:#{ENV['PKG_CONFIG_PATH']}"
ENV['PKG_CONFIG_PATH'] = "#{libraries_to_compile[:openssl].path.gsub(/^(\w):/i) { "/" + $1.downcase }}/lib64/pkgconfig:#{ENV['PKG_CONFIG_PATH']}"
libraries_to_compile[:freetds].configure_options << "--with-openssl=#{libraries_to_compile[:openssl].path}"
end

Expand Down
1 change: 0 additions & 1 deletion test/gem_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ class GemTest < MiniTest::Spec
{
'x64-mingw-ucrt' => 'x64-mingw-ucrt',
'x64-mingw32' => 'x64-mingw32',
'x86-mingw32' => 'x86-mingw32',
'x86_64-linux' => 'x86_64-linux',
}.each do |host,expected|
describe "on a #{host} architecture" do
Expand Down