Skip to content

Use JSON for cargo metadata parsing #9373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hsbt merged 1 commit into from
Mar 9, 2026
+3 −4
Merged

Use JSON for cargo metadata parsing #9373

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/rubygems/ext/cargo_builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,9 @@ def cargo_crate_name(cargo_dir, manifest_path, results)
raise Gem::InstallError, "cargo metadata failed#{exit_reason}"
end

# cargo metadata output is specified as json, but with the
# --format-version 1 option the output is compatible with YAML, so we can
# avoid the json dependency
metadata = Gem::SafeYAML.safe_load(output)
# cargo metadata output is specified as json
require "json"
metadata = JSON.parse(output)
Comment on lines +230 to +232
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
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

Copilot uses AI. Check for mistakes.
package = metadata["packages"].find {|pkg| normalize_path(pkg["manifest_path"]) == manifest_path }
Comment on lines +230 to 233
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
unless package
found = metadata["packages"].map {|md| "#{md["name"]} at #{md["manifest_path"]}" }
Expand Down