Support Ruby 3.1 and 3.2 #533

andyundso · 2023-05-10T20:11:02Z

This PR introduces support for Ruby 3.1 and Ruby 3.2 to tiny_tds. Couple of obstacles:

The GCC compiler for x64 UCRT within the rake-compiler-dock environment is named the same as the one for x64 mingw32, but they yield completely different binaries. This introduces an issue with the port mechanic, where we used the host property from RbConfig to determine the compiler and where to save the ports. I introduced an additional parameter for passing the Ruby architecture to the ports compilation tasks. The Ruby architecture is used to still have a way to differ between the ports for UCRT and Mingw32 while they share the name for the GCC compiler.

The cross-compile job for Windows now runs in parallel for each platform. This was because of another effect: When trying to install the compiled gems for Windows to get the compiled libraries, it raised Unable to find spec for #<Gem::NameTuple tiny_tds, 2.1.5, x64-unknown>, but only for 2.7 and below. Turned out that when installing a gem locally, RubyGems parsed all gem specifications for any file named *.gem in the current directory. And since older versions of Rubygems don't know ucrt, it raised x64-unknown. I found it out by essentially running byebug /my/ruby/installation/bin/gem install tiny_tds, opening the source code of RubyGems to find out where it got the specs from and setting a breakpoint using an absolute path again.

The parallisation added a couple of other issues with CircleCI, namely:

The ports are cached for each Ruby version. I had a couple of faulty CI runs where compiling tiny_tds only worked for some Ruby versions when the ports were restored from cache. I assume there are differences in regards to the container environment in the different images.
As the precompiled gems are saved into "the workspace", the files need to be unique. As each job also added the non-native version into the workspace, it resulted in a conflict. I solved this by removing the non-native gem so only the precompiled versions are saved into the workspace.

On CircleCI, the Linux tests initially failed for Ruby 3.1 and 3.2 when they tried to compile OpenSSL (see here). I did not bother finding out why, but instead used the ports mechanic from the gem to get the necessary libraries to build tiny_tds.

Note that this PR does not address #515.

This is in preparation for the Ruby 3.1 update, as the UCRT environment is only available with rack-compiler-dock 1.2 and newer.

This will invalidate the cache in case platforms are added or removed.

ecentell-CPF · 2023-06-12T18:26:09Z

Any hopes to get this merged soon?

smtadaka · 2023-07-14T12:52:40Z

Hi, When can we expect this PR to be merged?

ecentell-CPF · 2023-08-21T16:38:37Z

Hoping to get this PR some attention for a merge

andyundso · 2023-08-28T12:38:57Z

since a couple of comments have been posted, @aharpervc would you mind to have a look at my PR?

pere73 · 2023-09-06T13:53:08Z

It would be great, if tiny_tds would support at least Ruby 3.1 on Windows.

aharpervc

This generally seems fine, although I left a few line comments to consider.

Also, I'd like to have the CI jobs run to validate the PR, but I don't see that they have. "Build forked pull requests" is turned on in the project settings. I see the jobs have run in your fork, though, so that's probably good enough:

aharpervc · 2023-09-06T14:33:53Z

tiny_tds.gemspec

-  s.add_development_dependency 'rake-compiler', '~> 1.1.0'
-  s.add_development_dependency 'rake-compiler-dock', '~> 1.1.0'
+  s.add_development_dependency 'rake-compiler', '~> 1.2.0'
+  s.add_development_dependency 'rake-compiler-dock', '~> 1.3.0'


I wonder if we even need the patch level specifier on these... maybe it'd be best as 'rake-compiler', '~> 1.2' so the minor version is floating?

I removed the minor version constraint for rake-compiler in 801f16c. I kept it for rake-compiler-dock to keep it aligned with the Docker image, as stated in #533 (comment).

aharpervc · 2023-09-06T14:38:10Z

Rakefile

+    host: 'x86_64-w64-mingw32',
+    ruby_versions: ruby_cc_mingw32_versions
+  },
+  'x64-mingw-ucrt' => {


Looks like this commit is also introducing a ucrt based build, can you update the commit message with an additional explanation on this?

I added a reference to the commit in cae55bf.

aharpervc · 2023-09-06T14:40:43Z

test/gem_test.rb

@@ -9,11 +9,11 @@ class GemTest < MiniTest::Spec

    # We're going to muck with some system globals so lets make sure
    # they get set back later
-    original_host = RbConfig::CONFIG['host']
+    original_platform = RbConfig::CONFIG["arch"]


Prefer single quote string literals to match elsewhere in the file

addressed in cb98ec1.

aharpervc · 2023-09-06T14:42:57Z

ext/tiny_tds/extconf.rb

-host = RbConfig::CONFIG['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', host, 'freetds', FREETDS_VERSION)
+freetds_ports_dir = File.join(project_dir, 'ports', architecture, 'freetds', FREETDS_VERSION)


I don't completely understand the swap from "host" to "architecture", other than that I generally agree the latter makes sense. Is switching this matching rake compiler patterns, or is it something bespoke for this project's config?

not really on us, but rather on rake-compiler-dock.

I edited the description, so it's hopefully more clear.

aharpervc · 2023-09-06T14:46:12Z

.circleci/config.yml

+        type: string
+
+    docker:
+      - image: "ghcr.io/rake-compiler/rake-compiler-dock-image:1.3.0-mri-<< parameters.platform >>"


Switching the base image here is a little surprising, can you share more why this is needed? Also, rather than pinning to 1.3.0, is there a tag for the latest release?

Switching the base image here is a little surprising, can you share more why this is needed?

It's been a while since I implemented this. But basically, we previously had this one Ubuntu job which compiled the ports for x86-mingw32, x64-mingw32. Now with this PR, we need to introduce another platform with x64-mingw-ucrt.

To speed to process up, I instead opt to run a separate job for each architecture. I do not recall how much time it saves, but I assume we talk about ~10 minutes per pipeline run.

Also, rather than pinning to 1.3.0, is there a tag for the latest release?

Looking into GitHub packages, there is a snapshot version available, which might be unstable. Then 1.3.0 is already the latest release.

I think it makes sense to pin this version to align it with the rake-compiler-dock gem.

aharpervc · 2023-09-06T14:47:20Z

.circleci/config.yml

          command: |
-            docker exec cimg_ruby bash -c 'bundle install'
+            docker exec cimg_ruby bash -c 'bundle exec rake ports && bundle exec rake build'


I recommend keeping "rake ports" and "rake build" as separate CI steps unless they absolutely need to be combined

addressed in 69a6dbd.

aharpervc · 2023-09-06T14:48:22Z

.circleci/config.yml

@@ -247,6 +247,12 @@ jobs:
            mkdir -p artifacts-<< parameters.platform >>/gems
            mv pkg/*.gem artifacts-<< parameters.platform >>/gems

+      - run:
+          name: Remove non-native gem to avoid conflict in workspace


Does this mean there's unexpected shared state between the CI jobs? Addressing that might be a better approach than "delete everything that's not me" 🤔

It is a rather undesired side-effect of gem build: Each build yields its non-native version of the gem, on which CircleCI reports a conflict of resources. I did some research and there seems to be not a way to surpress the generation of the non-native gem.

ecentell-CPF · 2023-09-25T15:09:46Z

Bump for some hope to get this merged, thx,

Starting with Ruby 3.1, the Ruby installer for Windows switched their C-runtime from MSVCRT to UCRT. Therefore, this commit introduces a UCRT-based build. Reference: https://rubyinstaller.org/2021/12/31/rubyinstaller-3.1.0-1-released.html

The GCC for UCRT is named the same as in MingW. This was an issue with mini_portile who used the target host for directories. However, the two compiler yield two very different binaries, so I added a new mechanism based on `target_gem_platform` for saving the ports / final gem. Spend also some time to debug the UCRT build (always changed into `x86_64-linux` when trying to build `tiny_tds`, but adding Ruby 3.2 helped). So I assume it's an issue with rake-compiler?

andyundso · 2023-09-25T19:54:33Z

@aharpervc addressed your feedback, mind to have a second look?

aharpervc · 2023-09-26T13:54:23Z

@aharpervc addressed your feedback, mind to have a second look?

Seems fine. And the CircleCI integration started working for this PR, which is nice. Something I noticed is that despite having a step to upload artifacts, there isn't any files actually saved. Eg, should be a .gem file here: https://app.circleci.com/pipelines/github/simplificator/tiny_tds/137/workflows/5a2df471-ebdc-4a0c-9fdc-c87af53afb20/jobs/984/artifacts

Having that functional is a goal for this project so that folks can test gem's for PR's without building locally. I suppose it doesn't need to block this PR if there's capacity to investigate soon as a followup

andyundso · 2023-09-26T16:00:49Z

@aharpervc addressed your feedback, mind to have a second look?

Seems fine. And the CircleCI integration started working for this PR, which is nice. Something I noticed is that despite having a step to upload artifacts, there isn't any files actually saved. Eg, should be a .gem file here: https://app.circleci.com/pipelines/github/simplificator/tiny_tds/137/workflows/5a2df471-ebdc-4a0c-9fdc-c87af53afb20/jobs/984/artifacts

Having that functional is a goal for this project so that folks can test gem's for PR's without building locally. I suppose it doesn't need to block this PR if there's capacity to investigate soon as a followup

Will do so!

ecentell-CPF · 2023-09-26T16:10:46Z

Thanks for getting this merged! Can you tell me will there be a version increment or a related push to rubygems.org?

aharpervc · 2023-09-26T20:52:21Z

Yep, if nobody else has made the PR yet, go for it. I recommend this general pattern as a template: #486 (plus a commit that appends .pre to the version string, as a temporary commit to drop before merging).

However...

[re: assets from CI] Will do so!

I recommend basing the version prep PR on 👆, so someone with rubygems access can use the assets from CI for the rubygems release, so we can also validate that CI can produce release-able assets.

ecentell-CPF · 2023-09-28T14:33:26Z

Prepared PR #540 using #486 as a template.

andyundso added 2 commits April 4, 2023 20:02

Update rake-compiler and rake-compiler-dock

bdf0f40

This is in preparation for the Ruby 3.1 update, as the UCRT environment is only available with rack-compiler-dock 1.2 and newer.

Write platforms into ports_version file

f471526

This will invalidate the cache in case platforms are added or removed.

andyundso requested a review from aharpervc May 10, 2023 20:11

aharpervc approved these changes Sep 6, 2023

View reviewed changes

andyundso and others added 8 commits September 25, 2023 21:10

Support Ruby 3.1

cae55bf

Starting with Ruby 3.1, the Ruby installer for Windows switched their C-runtime from MSVCRT to UCRT. Therefore, this commit introduces a UCRT-based build. Reference: https://rubyinstaller.org/2021/12/31/rubyinstaller-3.1.0-1-released.html

Parallelize cross compilation

37b4572

Use ports mechanism for Linux tests

fb5b456

Avoid conflict of files in workspace

bbd8aee

Only specify minor version for Rake compiler

801f16c

Align quotes with rest of test file

cb98ec1

Separate compile ports and build gem

69a6dbd

andyundso force-pushed the ruby-3-1-3-2 branch from c27e7b8 to 69a6dbd Compare September 25, 2023 19:26

aharpervc self-requested a review September 26, 2023 13:54

aharpervc approved these changes Sep 26, 2023

View reviewed changes

andyundso merged commit fd0877b into rails-sqlserver:master Sep 26, 2023
17 checks passed

andyundso deleted the ruby-3-1-3-2 branch September 26, 2023 16:02

ecentell-CPF mentioned this pull request Sep 28, 2023

Prepare tiny_tds gem for Ruby v3.1 on Windows #532

Closed

ecentell-CPF mentioned this pull request Oct 13, 2023

Prepare for 2.1.6 #540

Merged

andyundso mentioned this pull request Oct 18, 2023

Update CI/Cross compile Ruby version support #509

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Ruby 3.1 and 3.2 #533

Support Ruby 3.1 and 3.2 #533

andyundso commented May 10, 2023 •

edited

Loading

ecentell-CPF commented Jun 12, 2023

smtadaka commented Jul 14, 2023

ecentell-CPF commented Aug 21, 2023

andyundso commented Aug 28, 2023

pere73 commented Sep 6, 2023

aharpervc left a comment

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

aharpervc Sep 6, 2023

andyundso Sep 25, 2023

ecentell-CPF commented Sep 25, 2023

andyundso commented Sep 25, 2023

aharpervc commented Sep 26, 2023

andyundso commented Sep 26, 2023

ecentell-CPF commented Sep 26, 2023

aharpervc commented Sep 26, 2023

ecentell-CPF commented Sep 28, 2023

Support Ruby 3.1 and 3.2 #533

Support Ruby 3.1 and 3.2 #533

Conversation

andyundso commented May 10, 2023 • edited Loading

ecentell-CPF commented Jun 12, 2023

smtadaka commented Jul 14, 2023

ecentell-CPF commented Aug 21, 2023

andyundso commented Aug 28, 2023

pere73 commented Sep 6, 2023

aharpervc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecentell-CPF commented Sep 25, 2023

andyundso commented Sep 25, 2023

aharpervc commented Sep 26, 2023

andyundso commented Sep 26, 2023

ecentell-CPF commented Sep 26, 2023

aharpervc commented Sep 26, 2023

ecentell-CPF commented Sep 28, 2023

andyundso commented May 10, 2023 •

edited

Loading