Modernize gdal-ruby packaging, multi-arch CI, and Fulcrum coverage

[treyhyde]

Why

This gem is an old SWIG dump that still blocks Fulcrum on modern Ruby/GDAL and multi-arch builds. Version drift (repo said 2.0.0 while Fulcrum locked 3.0.0), dead Travis CI, Ruby 3 SWIG allocator warnings in Rails logs, and GDAL 3.13 compile breaks all make it hard to keep as an internal dependency. We also need a real release path to GitHub Packages.

What changed

• Replaced Travis with GitHub Actions CI on Ubuntu amd64/arm64 and macOS arm64 (Ruby 3.2/3.3)
• Added release-please + gem publish to rubygems.pkg.github.com/fulcrumapp when a release PR merges to main
• Modernized gemspec/metadata, required Ruby >= 3.1, aligned version to 3.1.0
• Fixed Ruby 3.2+ SWIG T_DATA allocator warnings on require
• Fixed GDAL 3.13 compile issues (CSLConstList const-correctness, ABS shim, no -std=c++17 on pure-C gdalconst)
• Added fixture-based specs that mirror Fulcrum's Import::Formats::Shapefile call surface (open, OFT/WKB constants, field accessors, flatten_to_2d, export_to_json, point/line/polygon/multipoint)

Verification

Locally on arm64 / GDAL 3.13.1 / Ruby 3.3.4:

23 examples, 0 failures

Notes for reviewers

• This keeps the gem viable short-term. Long-term recommendation remains: migrate Fulcrum shapefile import to rgeo-shapefile and retire this dependency.
• Publishing a source gem to GitHub Packages does not by itself solve multi-arch; consumers still need arch-native libgdal at compile time.
• After merge, release-please should open/finish a release PR to publish gdal 3.1.0 to Packages.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Modernizes the gdal (gdal-ruby) gem for current Fulcrum usage by updating packaging/release automation, multi-arch CI, and adding fixture-based specs that mirror Fulcrum’s shapefile importer call surface.

Changes:

• Replaced Travis with GitHub Actions CI across Ubuntu amd64/arm64 and macOS arm64; added release-please + publish workflow to GitHub Packages.
• Modernized gemspec/metadata and aligned versioning to 3.1.0.
• Updated native extension build/config and SWIG runtime glue for Ruby 3.2+ warnings and GDAL 3.13 compilation; added Fulcrum-parity specs + fixtures.

Reviewed changes

Copilot reviewed 30 out of 46 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
spec/support/fulcrum_shapefile_importer.rb	Adds a small adapter module to mirror Fulcrum’s shapefile import call surface for specs.
spec/spec_helper.rb	Introduces shared RSpec setup and fixture path constant used across new specs.
spec/ruby3_warnings_spec.rb	Adds a regression spec to ensure Ruby 3.2+ SWIG allocator warnings are not emitted.
spec/gdal_spec.rb	Updates core gem specs for version reporting, WKT→GeoJSON, and require entrypoints.
spec/fulcrum_shapefile_import_spec.rb	Adds fixture-driven coverage approximating Fulcrum’s shapefile import usage.
spec/fixtures/shapefiles/polygons/polygons.prj	Adds projection fixtures for polygon shapefile tests.
spec/fixtures/shapefiles/points/points.prj	Adds projection fixtures for point shapefile tests (incl. 3D).
spec/fixtures/shapefiles/multipoint/multipoint.prj	Adds projection fixtures for multipoint shapefile tests.
spec/fixtures/shapefiles/lines/lines.prj	Adds projection fixtures for line shapefile tests.
spec/fixtures/shapefiles/flat/points.prj	Adds projection fixtures for “flat” point shapefile tests.
release-please-config.json	Configures release-please for Ruby releases and changelog generation.
lib/gdal-ruby/version.rb	Aligns in-repo version to 3.1.0 and adds sync guidance comments.
gdal.gemspec	Modernizes gemspec metadata, required Ruby version, files list, and dev deps.
ext/gdal-ruby/osr/osr.cpp	Updates SWIG runtime class allocation behavior and fixes GDAL 3.13 compile issues.
ext/gdal-ruby/osr/extconf.rb	Switches OSR extension build to shared extconf helper.
ext/gdal-ruby/ogr/ogr.cpp	Updates SWIG runtime class allocation behavior for Ruby 3.2+.
ext/gdal-ruby/ogr/extconf.rb	Switches OGR extension build to shared extconf helper.
ext/gdal-ruby/gdalconst/gdalconst.c	Updates SWIG runtime class allocation behavior for Ruby 3.2+.
ext/gdal-ruby/gdalconst/extconf.rb	Switches gdalconst extension build to shared extconf helper.
ext/gdal-ruby/gdal/gdal.cpp	Updates SWIG runtime class allocation behavior and applies GDAL 3.13 const-correctness workaround.
ext/gdal-ruby/gdal/extconf.rb	Switches GDAL extension build to shared extconf helper.
ext/gdal-ruby/extconf_helper.rb	Adds a unified extconf helper for GDAL detection/flags across extensions.
Rakefile	Updates spec task to ensure compilation and consistent RSpec options.
README.md	Updates docs for new CI, install path (GitHub Packages), and long-term migration guidance.
CHANGELOG.md	Adds 3.1.0 release notes and cleans up older entries formatting.
.travis.yml	Removes obsolete Travis CI configuration.
.tool-versions	Pins local Ruby version for asdf/tooling consistency.
.release-please-manifest.json	Adds release-please manifest tracking current version.
.gitignore	Ignores RSpec examples persistence file.
.github/workflows/release.yml	Adds release-please + gem publishing workflow and GitHub Release asset upload.
.github/workflows/ci.yml	Adds multi-arch CI workflow installing GDAL and running specs.

Comments suppressed due to low confidence (3)

spec/support/fulcrum_shapefile_importer.rb:1

• OGRLayer#get_feature expects a Feature ID (FID), not a positional “cursor index”. For shapefiles where FIDs aren’t 0..N-1 (or contain gaps), this can return nil or the wrong feature, causing failures or incorrect imports. Prefer iterating with reset_reading + get_next_feature (matching Fulcrum’s “next_feature loop”), or use set_next_by_index(index) + get_next_feature if you need index-based access.
  spec/support/fulcrum_shapefile_importer.rb:1
• each_feature typically implies yielding to a block (or returning an Enumerator), but this implementation builds and returns an Array. To avoid confusion for future readers/maintainers, either rename it to something like features/read_features, or implement block-yielding behavior (and return an Enumerator when no block is given).
  spec/support/fulcrum_shapefile_importer.rb:1
• Assigning local variables to nil in ensure doesn’t deterministically release native GDAL/OGR resources; it’s effectively a no-op unless you rely on a later GC cycle. If explicit cleanup is required, call the relevant close/destroy method exposed by the bindings; otherwise, removing this ensure block will reduce misleading “resource cleanup” signaling.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[treyhyde]

Also tightened the Fulcrum-shaped helper based on the low-confidence notes:

• index access now uses set_next_by_index + get_next_feature (FID-safe)
• each_feature renamed to read_features and uses reset_reading sequential scan
• removed the no-op ensure nil assignments

[Copilot]

Pull request overview

Copilot reviewed 30 out of 46 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

spec/support/fulcrum_shapefile_importer.rb:1

• Inside the feature loop, field_info(lyr) and field_count(lyr) are recomputed for every row even though layer schema is constant. Moving info = field_info(lyr) and count = field_count(lyr) outside the while loop will reduce repeated native calls and speed up larger shapefiles. This also mirrors the same field-extraction logic already present in read_feature, which could be factored into a shared helper to avoid drift between the two paths.
  ext/gdal-ruby/gdal/gdal.cpp:2184
• CSLDuplicate(...) allocates a new string list; these functions return the allocated char** without an obvious corresponding CSLDestroy(...) on the Ruby/SWIG side. If the SWIG typemap for returned char ** does not free with CSLDestroy, this becomes a per-call memory leak. Consider changing the wrapper/typemap so the duplicated list is destroyed after conversion to Ruby (or ensure the return typemap frees using the correct GDAL deallocator).

    /* GDAL 3+ returns CSLConstList. Duplicate so Ruby owns a mutable char**
       without aliasing GDAL's const storage (avoids const_cast UB on mutate). */
    return CSLDuplicate(GDALGetMetadata(self, pszDomain));
  }
SWIGINTERN char **GDALMajorObjectShadow_GetMetadata_List(GDALMajorObjectShadow *self,char const *pszDomain=""){
    return CSLDuplicate(GDALGetMetadata(self, pszDomain));
  }

gdal.gemspec:28

• RubyGems metadata fields like github_repo are typically expected to be an https://github.com/... URL; using an ssh:// URI may not be recognized or linked correctly by RubyGems consumers/tools. Prefer https://github.com/fulcrumapp/gdal-ruby (or a canonical git URL) for better compatibility.

  gem.metadata = {
    "bug_tracker_uri" => "https://github.com/fulcrumapp/gdal-ruby/issues",
    "changelog_uri" => "https://github.com/fulcrumapp/gdal-ruby/blob/main/CHANGELOG.md",
    "homepage_uri" => gem.homepage,
    "source_code_uri" => "https://github.com/fulcrumapp/gdal-ruby",
    "github_repo" => "ssh://github.com/fulcrumapp/gdal-ruby",
    "allowed_push_host" => "https://rubygems.pkg.github.com/fulcrumapp"
  }

README.md:69

• The README usage example requires gdal-ruby/ogr, but the rest of this PR adds coverage around require \"gdal\" as the primary entrypoint (and the gem name is gdal). Consider updating the example (or adding an additional example) to use require \"gdal\" so the docs match the supported/verified require path.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 46 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

spec/support/fulcrum_shapefile_importer.rb:1

• schema_columns computes info = field_info(layer) but then calls field_count(layer) which calls field_info(layer) again, doing extra native calls unnecessarily. Since info is already available, prefer using info.get_field_count.times here to avoid redundant layer-defn lookups.
  spec/support/fulcrum_shapefile_importer.rb:1
• The field extraction logic (type switch + accessor selection) is duplicated here and again in read_features. This duplication increases the chance of the Fulcrum-mirroring behavior drifting between the two code paths. Consider extracting a small helper (e.g., read_attrs(feature, layer_defn)) that takes info/field_count once and is used by both methods.
  spec/support/fulcrum_shapefile_importer.rb:1
• field_info(lyr) and field_count(lyr) are invariant for a layer and don’t need to be recomputed for every feature. Moving info = field_info(lyr) and count = info.get_field_count outside the while loop avoids repeated native calls and makes the loop intent clearer.
  spec/spec_helper.rb:1
• Defining FIXTURES as a top-level constant can cause accidental coupling across specs/support files and makes it easier to collide with other constants if the suite grows. Consider namespacing it (e.g., SpecSupport::FIXTURES) or exposing it via RSpec.configuration to keep global namespace clean.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 46 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

spec/ruby3_warnings_spec.rb:1

• layer.get_feature(0) assumes the first feature’s FID is 0, but OGR’s get_feature takes an FID (which is not guaranteed to be 0..N-1). This can make the warning-regression spec fail depending on how the fixture was created. Use reset_reading + get_next_feature, or set_next_by_index(0) + get_next_feature, to make the test index-based and stable.
  .github/workflows/release.yml:62
• The release pipeline publishes the gem immediately after gem build without running the specs (or even a compile verification via the Rake tasks). Given the native-extension surface and the goal of preventing Ruby/GDAL drift regressions, it’s safer to run bundle exec rake spec (or at least bundle exec rake compile) in the publish job before pushing, so broken releases don’t get published.

      - name: Build gem
        run: |
          gem build gdal.gemspec
          ls -la gdal-*.gem

      - name: Publish to GitHub Packages
        env:
          GEM_HOST_API_KEY: Bearer ${{ secrets.GITHUB_TOKEN }}
        run: |
          mkdir -p ~/.gem
          printf '%s\n' '---' ":github: ${GEM_HOST_API_KEY}" > ~/.gem/credentials
          chmod 0600 ~/.gem/credentials
          gem push --key github --host https://rubygems.pkg.github.com/fulcrumapp gdal-*.gem

[Copilot]

Pull request overview

Copilot reviewed 30 out of 46 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

spec/support/fulcrum_shapefile_importer.rb:1

• String#force_encoding("UTF-8") does not validate or transcode bytes; it can produce strings marked UTF-8 that still contain invalid UTF-8 sequences, which can later break JSON serialization, DB writes, or string operations. If the intent is “ensure valid UTF-8 text,” use a transcoding approach (e.g., encode("UTF-8", invalid: :replace, undef: :replace, replace: ...)) rather than force_encoding.
  ext/gdal-ruby/gdal/gdal.cpp:4953
• CSLDestroy(result) is only reached on the normal path. If any Ruby API call in the conversion loop raises (e.g., rb_hash_aset, rb_str_new2), the function can exit via exception and leak the duplicated CSL list. Consider wrapping the Ruby conversion in rb_ensure (or equivalent) so CSLDestroy(result) always runs, even when an exception is raised.

    /* GetMetadata_Dict duplicates via CSLDuplicate; free after Ruby conversion. */
    CSLDestroy(result);

ext/gdal-ruby/osr/osr.cpp:15

• This ABS macro evaluates x multiple times, which is unsafe if x has side effects (and can also overflow for INT_MIN). Prefer using an existing GDAL/CPL helper (e.g., CPL_ABS if available) or an inline function/template to ensure single evaluation and safer behavior.

#define ABS(x) ((x) < 0 ? -(x) : (x))

[Copilot]

Pull request overview

Copilot reviewed 30 out of 46 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

gdal.gemspec:46

• When git ls-files is unavailable and the gemspec falls back to Dir.glob(..., File::FNM_DOTMATCH), this will include hidden file entries that aren’t explicitly excluded (notably .git/* if present, plus other dotfiles). Consider explicitly rejecting .git/ (and possibly other common hidden paths) in the fallback list, or switching the fallback to an allowlist (e.g., lib/, ext/, README.md, LICENSE, etc.) to avoid accidental packaging of repository metadata.

        Dir.glob("**/*", File::FNM_DOTMATCH)
      end

    tracked
      .reject(&:empty?)
      .reject { |f| f == "." || f == ".." || f.end_with?("/.") || f.end_with?("/..") }
      .reject { |f| File.directory?(f) }
      .reject do |f|
        f.start_with?("spec/", "test/", ".github/", "tmp/", "pkg/") ||
          f == ".travis.yml" ||
          f.end_with?(".bundle", ".so", ".o", ".gem")
      end

ext/gdal-ruby/extconf_helper.rb:57

• If gdal-config fails, the raised error omits stderr, which can make diagnosing CI/build failures difficult (e.g., missing shared libs, bad shebang, permission issues). Consider using Open3.capture3 and including stderr (and possibly the exit status) in the exception message so failures are actionable.

    def gdal_config_output(gdal_config, *args)
      output, status = Open3.capture2(gdal_config, *args)
      raise "failed to run #{gdal_config} #{args.join(' ')}" unless status.success?

      output
    end