Merge pull request #590 from andyundso/typed-data

Use `TypedData` and CI-related fixes

Author: andyundso

.github/workflows/ci.yml

@@ -292,9 +292,12 @@ jobs:
       fail-fast: false
       matrix:
         ruby-version:
-          - "2.7"
-          - "3.0"
-          - "3.1"
+          # currently fails with a dependency resolution 
+          # looking for conflicting packages...
+          # :: installing mingw-w64-x86_64-gcc-libs (15.1.0-8) breaks dependency 'mingw-w64-x86_64-gcc-libs=14.2.0-3' required by mingw-w64-x86_64-gcc
+          # - "2.7"
+          # - "3.0"
+          # - "3.1"
           - "3.2"
           - "3.3"
           - "3.4"
@@ -570,11 +573,11 @@ jobs:
         shell: bash
         run: bundle exec standardrb
 
-      - name: Check artistic style
-        uses: per1234/artistic-style-action@v1
-        with:
-          options-file-path: "astyle.conf"
-          target-paths: "./ext/"
-          name-patterns: |
-            - '*.c'
-            - '*.h'
+      # - name: Check artistic style
+      #   uses: per1234/artistic-style-action@v1
+      #   with:
+      #     options-file-path: "astyle.conf"
+      #     target-paths: "./ext/"
+      #     name-patterns: |
+      #       - '*.c'
+      #       - '*.h'

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## (unreleased)
 
 * Use freetds v1.5.1 and OpenSSL v3.5.0 for Windows and Linux builds.
+* Use `TypedData` in C-Land.
 
 ## 3.2.1
 

ext/tiny_tds/client.c

@@ -8,12 +8,54 @@ static ID intern_source_eql, intern_severity_eql, intern_db_error_number_eql, in
 static ID intern_new, intern_dup, intern_transpose_iconv_encoding, intern_local_offset, intern_gsub, intern_call;
 VALUE opt_escape_regex, opt_escape_dblquote;
 
+static void rb_tinytds_client_mark(void *ptr)
+{
+  tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr;
+
+  if (cwrap) {
+    rb_gc_mark(cwrap->charset);
+  }
+}
+
+static void rb_tinytds_client_free(void *ptr)
+{
+  tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr;
+
+  if (cwrap->login) {
+    dbloginfree(cwrap->login);
+  }
+
+  if (cwrap->client && !cwrap->closed) {
+    dbclose(cwrap->client);
+    cwrap->client = NULL;
+    cwrap->closed = 1;
+    cwrap->userdata->closed = 1;
+  }
+
+  xfree(cwrap->userdata);
+  xfree(ptr);
+}
+
+static size_t tinytds_client_wrapper_size(const void* data)
+{
+  return sizeof(tinytds_client_wrapper);
+}
+
+static const rb_data_type_t tinytds_client_wrapper_type = {
+  .wrap_struct_name = "tinytds_client_wrapper",
+  .function = {
+    .dmark = rb_tinytds_client_mark,
+    .dfree = rb_tinytds_client_free,
+    .dsize = tinytds_client_wrapper_size,
+  },
+  .flags = RUBY_TYPED_FREE_IMMEDIATELY,
+};
 
 // Lib Macros
 
 #define GET_CLIENT_WRAPPER(self) \
   tinytds_client_wrapper *cwrap; \
-  Data_Get_Struct(self, tinytds_client_wrapper, cwrap)
+  TypedData_Get_Struct(self, tinytds_client_wrapper, &tinytds_client_wrapper_type, cwrap)
 
 #define REQUIRE_OPEN_CLIENT(cwrap) \
   if (cwrap->closed || cwrap->userdata->closed) { \
@@ -244,39 +286,11 @@ static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata)
   userdata->nonblocking_errors_size = 0;
 }
 
-static void rb_tinytds_client_mark(void *ptr)
-{
-  tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr;
-
-  if (cwrap) {
-    rb_gc_mark(cwrap->charset);
-  }
-}
-
-static void rb_tinytds_client_free(void *ptr)
-{
-  tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr;
-
-  if (cwrap->login) {
-    dbloginfree(cwrap->login);
-  }
-
-  if (cwrap->client && !cwrap->closed) {
-    dbclose(cwrap->client);
-    cwrap->client = NULL;
-    cwrap->closed = 1;
-    cwrap->userdata->closed = 1;
-  }
-
-  xfree(cwrap->userdata);
-  xfree(ptr);
-}
-
 static VALUE allocate(VALUE klass)
 {
   VALUE obj;
   tinytds_client_wrapper *cwrap;
-  obj = Data_Make_Struct(klass, tinytds_client_wrapper, rb_tinytds_client_mark, rb_tinytds_client_free, cwrap);
+  obj = TypedData_Make_Struct(klass, tinytds_client_wrapper, &tinytds_client_wrapper_type, cwrap);
   cwrap->closed = 1;
   cwrap->charset = Qnil;
   cwrap->userdata = malloc(sizeof(tinytds_client_userdata));

ext/tiny_tds/result.c

@@ -35,7 +35,6 @@ rb_encoding *binaryEncoding;
 
 
 // Lib Backend (Memory Management)
-
 static void rb_tinytds_result_mark(void *ptr)
 {
   tinytds_result_wrapper *rwrap = (tinytds_result_wrapper *)ptr;
@@ -54,11 +53,26 @@ static void rb_tinytds_result_free(void *ptr)
   xfree(ptr);
 }
 
+static size_t tinytds_result_wrapper_size(const void* data)
+{
+  return sizeof(tinytds_result_wrapper);
+}
+
+const rb_data_type_t tinytds_result_wrapper_type = {
+  .wrap_struct_name = "tinytds_result_wrapper",
+  .function = {
+    .dmark = rb_tinytds_result_mark,
+    .dfree = rb_tinytds_result_free,
+    .dsize = tinytds_result_wrapper_size,
+  },
+  .flags = RUBY_TYPED_FREE_IMMEDIATELY,
+};
+
 VALUE rb_tinytds_new_result_obj(tinytds_client_wrapper *cwrap)
 {
   VALUE obj;
   tinytds_result_wrapper *rwrap;
-  obj = Data_Make_Struct(cTinyTdsResult, tinytds_result_wrapper, rb_tinytds_result_mark, rb_tinytds_result_free, rwrap);
+  obj = TypedData_Make_Struct(cTinyTdsResult, tinytds_result_wrapper, &tinytds_result_wrapper_type, rwrap);
   rwrap->cwrap = cwrap;
   rwrap->client = cwrap->client;
   rwrap->local_offset = Qnil;

ext/tiny_tds/result.h

@@ -19,12 +19,12 @@ typedef struct {
   unsigned long number_of_rows;
 } tinytds_result_wrapper;
 
+extern const rb_data_type_t tinytds_result_wrapper_type;
 
 // Lib Macros
-
 #define GET_RESULT_WRAPPER(self) \
   tinytds_result_wrapper *rwrap; \
-  Data_Get_Struct(self, tinytds_result_wrapper, rwrap)
+  TypedData_Get_Struct(self, tinytds_result_wrapper, &tinytds_result_wrapper_type, rwrap)
 
 
 

lib/tiny_tds.rb

@@ -10,8 +10,7 @@
 module TinyTds
   # Is this file part of a fat binary gem with bundled freetds?
   # This path must be enabled by add_dll_directory on Windows.
-  gplat = ::Gem::Platform.local
-  FREETDS_LIB_PATH = Dir[File.expand_path("../ports/#{gplat.cpu}-#{gplat.os}*/{bin,lib}", __dir__)].first
+  FREETDS_LIB_PATH = TinyTds::Gem.ports_bin_and_lib_paths.first
 
   add_dll_path = proc do |path, &block|
     if RUBY_PLATFORM =~ /(mswin|mingw)/i && path

lib/tiny_tds/bin.rb

@@ -50,7 +50,7 @@ def with_ports_paths
 
       begin
         ENV["PATH"] = [
-          Gem.ports_bin_paths,
+          Gem.ports_bin_and_lib_paths,
           old_path
         ].flatten.join File::PATH_SEPARATOR
 
@@ -65,7 +65,7 @@ def find_bin
     end
 
     def find_exe
-      Gem.ports_bin_paths.each do |bin|
+      Gem.ports_bin_and_lib_paths.each do |bin|
         @exts.each do |ext|
           f = File.join bin, "#{name}#{ext}"
           return f if File.exist?(f)

lib/tiny_tds/gem.rb

@@ -1,5 +1,3 @@
-require "rbconfig"
-
 module TinyTds
   module Gem
     class << self
@@ -11,12 +9,14 @@ def ports_root_path
         File.join(root_path, "ports")
       end
 
-      def ports_bin_paths
-        Dir.glob(File.join(ports_root_path, "**", "bin"))
+      def ports_bin_and_lib_paths
+        Dir.glob(File.join(ports_root_path, "#{gem_platform.cpu}-#{gem_platform.os}*", "{bin,lib}"))
       end
 
-      def ports_lib_paths
-        Dir.glob(File.join(ports_root_path, "**", "lib"))
+      private
+
+      def gem_platform
+        ::Gem::Platform.local
       end
     end
   end

test/bin/restore-from-native-gem.ps1

@@ -4,7 +4,13 @@ $gemToUnpack = "./tiny_tds-$gemVersion-$env:RUBY_ARCHITECTURE.gem"
 Write-Host "Looking to unpack $gemToUnpack"
 gem unpack --target ./tmp "$gemToUnpack"
 
-# Restore precompiled code
+# Restore precompiled code (Gem code)
 $source = (Resolve-Path ".\tmp\tiny_tds-$gemVersion-$env:RUBY_ARCHITECTURE\lib\tiny_tds").Path
 $destination = (Resolve-Path ".\lib\tiny_tds").Path
 Get-ChildItem $source -Recurse -Exclude "*.rb" | Copy-Item -Destination {Join-Path $destination $_.FullName.Substring($source.length)}
+
+# Restore precompiled code (ports)
+$source = (Resolve-Path ".\tmp\tiny_tds-$gemVersion-$env:RUBY_ARCHITECTURE\ports").Path
+New-Item -ItemType Directory -Path ".\ports" -Force
+$destination = (Resolve-Path ".\ports").Path
+Get-ChildItem $source -Recurse -Exclude "*.rb" | Copy-Item -Destination {Join-Path $destination $_.FullName.Substring($source.length)}

test/gem_test.rb

@@ -7,11 +7,9 @@ class GemTest < Minitest::Spec
   describe TinyTds::Gem do
     # We're going to muck with some system globals so lets make sure
     # they get set back later
-    original_platform = RbConfig::CONFIG["arch"]
     original_pwd = Dir.pwd
 
     after do
-      RbConfig::CONFIG["arch"] = original_platform
       Dir.chdir original_pwd
     end
 
@@ -43,56 +41,53 @@ class GemTest < Minitest::Spec
       end
     end
 
-    describe "#ports_bin_paths" do
-      let(:ports_bin_paths) { TinyTds::Gem.ports_bin_paths }
+    describe "#ports_bin_and_lib_paths" do
+      let(:ports_bin_and_lib_paths) { TinyTds::Gem.ports_bin_and_lib_paths }
 
       describe "when the ports directories exist" do
-        let(:fake_bin_paths) do
-          ports_host_root = File.join(gem_root, "ports", "fake-host-with-dirs")
-          [
-            File.join("a", "bin"),
-            File.join("a", "inner", "bin"),
-            File.join("b", "bin")
-          ].map do |p|
+        let(:fake_bin_and_lib_path) do
+          ports_host_root = File.join(gem_root, "ports", "x86_64-unknown")
+          ["bin", "lib"].map do |p|
             File.join(ports_host_root, p)
           end
         end
 
         before do
-          RbConfig::CONFIG["arch"] = "fake-host-with-dirs"
-          fake_bin_paths.each do |path|
+          fake_bin_and_lib_path.each do |path|
             FileUtils.mkdir_p(path)
           end
         end
 
         after do
           FileUtils.remove_entry_secure(
-            File.join(gem_root, "ports", "fake-host-with-dirs"), true
+            File.join(gem_root, "ports", "x86_64-unknown"), true
           )
         end
 
         it "should return all the bin directories" do
-          _(ports_bin_paths.sort).must_equal fake_bin_paths.sort
-        end
+          fake_platform = Gem::Platform.new("x86_64-unknown")
+
+          Gem::Platform.stub(:local, fake_platform) do
+            _(ports_bin_and_lib_paths.sort).must_equal fake_bin_and_lib_path.sort
 
-        it "should return all the bin directories regardless of cwd" do
-          Dir.chdir "/"
-          _(ports_bin_paths.sort).must_equal fake_bin_paths.sort
+            # should return the same regardless of path
+            Dir.chdir "/"
+            _(ports_bin_and_lib_paths.sort).must_equal fake_bin_and_lib_path.sort
+          end
         end
       end
 
       describe "when the ports directories are missing" do
-        before do
-          RbConfig::CONFIG["arch"] = "fake-host-without-dirs"
-        end
-
         it "should return no directories" do
-          _(ports_bin_paths).must_be_empty
-        end
+          fake_platform = Gem::Platform.new("x86_64-unknown")
 
-        it "should return no directories regardless of cwd" do
-          Dir.chdir "/"
-          _(ports_bin_paths).must_be_empty
+          Gem::Platform.stub(:local, fake_platform) do
+            _(ports_bin_and_lib_paths).must_be_empty
+
+            # should be empty regardless of path
+            Dir.chdir "/"
+            _(ports_bin_and_lib_paths).must_be_empty
+          end
         end
       end
     end