Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR switches Rust extension installation (Cargo metadata parsing) from YAML-based parsing to JSON parsing, aligning with the effort to reduce reliance on Psych/YAML internals within RubyGems.
Changes:
- Replace
Gem::SafeYAML.safe_loadwithJSON.parseforcargo metadataoutput. - Add a local
require "json"to support JSON parsing in the cargo builder.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| # cargo metadata output is specified as json | ||
| require "json" | ||
| metadata = JSON.parse(output) | ||
| package = metadata["packages"].find {|pkg| normalize_path(pkg["manifest_path"]) == manifest_path } |
There was a problem hiding this comment.
Open3.capture2e merges stdout+stderr, but cargo metadata writes JSON to stdout and can emit warnings/diagnostics to stderr even on success. With the new JSON.parse(output), any stderr content will make the string invalid JSON and break installs. Capture stdout separately (e.g., Open3.capture3 or capture2 with stderr captured independently) and parse only stdout; keep stderr for really_verbose/results output.
| # cargo metadata output is specified as json | ||
| require "json" | ||
| metadata = JSON.parse(output) |
There was a problem hiding this comment.
Now that cargo metadata is parsed as JSON, the earlier Gem.load_yaml in this method is unused and forces the YAML engine to be loaded during Rust extension installs. Please remove that call to avoid an unnecessary dependency/load cost (and to align with the goal of moving away from Psych).
| metadata = Gem::SafeYAML.safe_load(output) | ||
| # cargo metadata output is specified as json | ||
| require "json" | ||
| metadata = JSON.parse(output) |
There was a problem hiding this comment.
If JSON.parse fails (e.g., unexpected cargo output), it will currently raise JSON::ParserError and may surface as a noisy backtrace during gem install. Consider rescuing parse errors here and re-raising Gem::InstallError with a concise message (and include the raw output only when really_verbose or via results).
| metadata = JSON.parse(output) | |
| metadata = | |
| begin | |
| JSON.parse(output) | |
| rescue JSON::ParserError => error | |
| if Gem.configuration.really_verbose | |
| puts output | |
| else | |
| results << output | |
| end | |
| raise Gem::InstallError, "cargo metadata produced invalid JSON: #{error.message}" | |
| end |
Use JSON for cargo metadata parsing (cherry picked from commit c8694f0)
…ild failure Replace generic linux platforms (aarch64-linux, x86_64-linux) with explicit gnu/musl variants via `bundle lock --add-platform` so Bundler resolves precompiled binaries instead of building from source. This works around a RubyGems CargoBuilder bug where cargo deprecation warnings cause YAML parse errors during native extension builds. See: ruby/rubygems#9373
Backport the RubyGems 4.0.8+ fix for CargoBuilder#cargo_crate_name: replace SafeYAML.safe_load with JSON.parse and strip non-JSON stderr preamble (e.g. cargo deprecation warnings) before parsing. This replaces the precompiled binary approach from #73, which could not support aarch64-linux-musl where no binaries are published. See: ruby/rubygems#9373
from #9352
I'm working to replace Psych to pure Ruby implementation now. It's hard to support JSON at
YAMLSerializer.It's no problem to load
jsonat RubyGems becauselib/rubygems/ext/cargo_builder.rbis only called atgem i.