Rename native extension and cross-compile via rb-sys-dock#23
Open
berkos wants to merge 12 commits into
Open
Conversation
The native-gem cross-compile step used gem-compiler, which unpacks the source gem and runs its extconf.rb in isolation — without rb_sys on the load path. After the move to the rb-sys mkmf build, this fails with `cannot load such file -- rb_sys/mkmf`. Drop the precompiled-native-gem matrix and publish the source gem only. The native extension is compiled at install time, so no functionality is lost.
Supersedes the gem-compiler-based approach. gem-compiler unpacks the source gem and runs extconf.rb in isolation, without rb_sys on the load path; after the move to the rb-sys mkmf build, that step fails with `cannot load such file -- rb_sys/mkmf`. Replace it with the rake-compiler + rb-sys-dock pattern used by the other in-house rb-sys gems (distributing_iterator, gems/fetlife_markdown_renderer in fetlife-web): - Rakefile: switch from Gem::PackageTask + gem-compiler to RbSys::ExtensionTask + a dock:build:* namespace that shells out to rb-sys-dock. Cross-platforms: x86_64-linux, aarch64-linux, x86_64-darwin, arm64-darwin. - Gemfile: pin rake-compiler ~> 1.3 and rb_sys >= 0.9.126 for the cross-compile path. - .github/workflows/release.yml: three-job pipeline mirroring distributing-iterator — build_source_gem, build_native_gems (matrix per platform via `bundle exec rb-sys-dock`), release (attach all gems to the tag). The opencv cargo feature stays optional/off-by-default; cross-compiled gems ship libfacedetection (vendored C++, built from source via cc-rs) without opencv. Consumers who need opencv keep building from source.
berkos
changed the title
The new Rakefile requires `bundler/setup` so it can use bundler-managed deps (rake-compiler, rb_sys) for the cross-compile path. The old run-tests workflow ran `rake gem` before `bundle install`, which now aborts with `Bundler::GemNotFound`. Modernize to match distributing-iterator/ci.yml: - ruby/setup-ruby@v1 with bundler-cache: true (auto-runs bundle install) - actions-rust-lang/setup-rust-toolchain@v1 for the native compile - bundle exec rake build (replaces rake gem) - Drop libopencv-dev — the default cargo feature set doesn't pull opencv, and the smoke test only uses ruby-vips - Drop the long commented-out compile_native_gem block; the real cross-compile lives in release.yml now
RbSys::ExtensionTask.new("libfacedetection", ...) looks up cargo
package metadata by exact name match against `[package].name`. The
crate was named `libfacedetection-ruby` (chosen in the original rb-sys
migration to differentiate from the upstream `libfacedetection-rs` git
dep), so rake build aborts with `RbSys::PackageNotFoundError`.
Rename our package to match the gem name and alias the upstream dep
under a non-conflicting name:
- ext/libfacedetection/Cargo.toml:
- [package].name: libfacedetection-ruby -> libfacedetection
- [lib].name: libfacedetection_ruby -> libfacedetection
- dep alias: libfacedetection_rs (package = "libfacedetection")
- default feature: libfacedetection_rs (was libfacedetection)
- src/lib.rs: libfacedetection::facedetect_cnn -> libfacedetection_rs::,
feature gates updated. Ruby-visible Symbol "libfacedetection" kept
as the public feature label.
- libfacedetection.gemspec: cargo_crate_name metadata -> libfacedetection
- Cargo.lock: regenerated
extconf.rb / install-time bundle install path is unaffected — it just
runs `cargo build` against whatever is in the manifest.
actions-rust-lang/setup-rust-toolchain@v1 defaults to exporting RUSTFLAGS=-D warnings, which turns the pre-existing magnus 0.8.2 deprecation warnings (Symbol::new, Integer::from_i64, RArray::new, exception::runtime_error) into hard compile errors at `gem install` time. Opt out via `rustflags: ""`. The deprecations are real tech debt to clean up later (every call site needs threading the Ruby handle through), but not in scope for fixing the release workflow. release.yml is unaffected: rb-sys-dock runs inside a docker container with its own Rust toolchain, not the runner's env.
When cargo's [lib].name changed from libfacedetection_ruby to libfacedetection, the generated Makefile target name didn't follow: extconf.rb still asked rb_sys/mkmf to build "libfacedetection_ruby" as the artifact, so make tried to copy a non-existent target/release/liblibfacedetection_ruby.so and failed. - ext/libfacedetection/extconf.rb: create_rust_makefile target -> "libfacedetection/libfacedetection" - lib/libfacedetection.rb: load paths -> "libfacedetection" (no _ruby) Mirrors gems/fetlife_markdown_renderer's loader pattern where gem name = lib name = file name throughout.
`ruby tests/test_detection.rb` ran in system Ruby and couldn't find ruby-vips, which is bundler-managed (vendor/bundle/, not system gems). Switch to: - `bundle exec rake compile` — builds the native extension into lib/libfacedetection/ in-place (loader picks it up via relative require). - `bundle exec ruby tests/test_detection.rb` — bundler activates libfacedetection from the local gemspec source plus ruby-vips/ffi dev deps. Add minitest to the Gemfile (test group) — the smoke test uses minitest/autorun but it wasn't listed anywhere, so it only worked when system Ruby happened to ship it as a bundled gem.
- Rakefile: drop the dock:build:* namespace + CROSS_PLATFORMS + the
resolve_platforms helpers. Cross-compile is invoked directly via
`bundle exec rb-sys-dock --platform X --build` from release.yml,
so the rake tasks were dead weight. Add `Rake::TestTask` so
`rake test` compiles via the `compile:dev` dep and runs the smoke
test in one step. Drop the eager `require "bundler/setup"` —
`bundler/gem_tasks` already pulls bundler in.
- Gemfile: revert to bare `source + gemspec`. All dev pins live in
the gemspec (single-source-of-truth, matches distributing-iterator).
- libfacedetection.gemspec:
- move minitest "~> 5.0" from Gemfile group :test
- loosen rb_sys from "~> 0.9.126" to "~> 0.9" (matches
distributing-iterator; lets future 0.9.x patches in)
- extconf.rb: add `config.profile = ENV.fetch("RB_SYS_CARGO_PROFILE",
"release").to_sym` so debug builds are toggleable via env (QoL).
- workflow.yml: replace the explicit build/compile/test trio with a
single `bundle exec rake test`.
berkos
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #22 migrated the gem to rb-sys while preserving the historical
libfacedetection_rubynative-library name. This PR deliberately aligns the gem, Cargo package, cdylib, extconf target, loader, and gem metadata onlibfacedetection.The upstream Rust crate is imported as
libfacedetection_rsto avoid colliding with the local package name, while the public Cargo feature remainslibfacedetection.Extension layout
The layout follows the root-crate pattern introduced in
distributing-iteratorPR #8:Cargo.tomlandsrc/lib.rsat the repository root.ext/libfacedetection/extconf.rb, loading a shared rootextconf.rb.libfacedetection.Two fixes are included beyond the original PR #8 implementation:
./usr/local/.../Cargo.tomlpath.extension_dir, because the original PR Rework API #8 loader cannot load its released ABI-versioned native artifact.Release workflow
The 0.4.0 release failed because
gem-compilerunpacked the source gem and ranextconf.rbwithoutrb_syson the load path (cannot load such file -- rb_sys/mkmf). Replace that path with Bundler +rb-sys-dock, matchingdistributing-iterator:x86_64-linux,aarch64-linux,x86_64-darwin, andarm64-darwin.Other changes
RbSys::ExtensionTaskandRake::TestTask.Cargo.lockin the source gem.Readme.mdtoREADME.mdso it is packaged on case-sensitive systems.&Rubyhandle APIs.-D warningspolicy so future warnings fail CI.Verification
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --locked -- -D warningsRUSTFLAGS="-D warnings" bundle exec rake teston Ruby 3.4 and Ruby 4.0bundle exec rake buildlibfacedetectionsuccessfully.bundle exec rb-sys-dock --platform x86_64-linux --buildlibfacedetectionsuccessfully.actionlintpasses for both workflows.