From 5d1140656aefc0ed72b006d701aa60262a0e38fa Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Mon, 9 Nov 2026 19:39:04 -0500 Subject: [PATCH 1/2] Rebuild gem native extensions from a relative crawl root Agent-mode apply rebuilds a patched gem's C extension by running Ruby from the project dir, and RubyGems runs extconf.rb from the gem's ext dir. The copy's paths came from the crawl root as given, so with the default `--cwd .` (Bundler `path vendor/bundle`) GEM_PATH was a relative `./vendor/bundle/ruby/`. From the ext dir that names nothing, so an extconf.rb that activates a gem from the copy's own home failed. nokogiri's (`mini_portile2`) fails this way, and apply reported native_extension_build_failed and put the original back. A relative `--cwd proj` failed one step earlier: Ruby, run from `proj`, could not load `proj/vendor/.../x.gemspec`, so the patch was refused as native_extensions_unsupported. GemCopy::of now makes the copy's path absolute, so the spec path, GEM_PATH, gem dir and extension dirs are absolute for both the probe and the build. The marker still records extension dirs relative to the home. Found by smoke-testing 5.1.0. Before this fix, a hand-written patch to ext/nokogiri/nokogiri.c on nokogiri 1.19.7 (ruby platform, packaged libxml2) failed to rebuild under plain `socket-patch apply`. With it, the extension rebuilds in about 28s, Ruby loads the patched constant, `apply --check` is in sync, and rollback restores the original build. The new capstone native_patch_builds_from_a_relative_crawl_root patches extconf.rb to activate a helper gem installed in the same home, then applies with `--cwd proj` and with the default root. It fails without this change. Co-Authored-By: Claude Opus 5.5 --- .../tests/e2e_gem_native_ext_build.rs | 106 ++++++++++++++++++ crates/socket-patch-core/src/patch/gem_ext.rs | 7 ++ 2 files changed, 113 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs b/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs index 78cecd730..492873468 100644 --- a/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs +++ b/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs @@ -304,6 +304,112 @@ fn native_patch_that_does_not_build_is_put_back() { } /// `(relative path, bytes)` of every file under `dir`. +/// An `extconf.rb` that, like nokogiri's (which activates `mini_portile2`), +/// loads a helper gem installed beside the copy in the same home. +const EXTCONF_WITH_HELPER: &str = "gem \"sp_helper\"\nrequire \"sp_helper\"\n\ + require \"mkmf\"\ncreate_makefile(\"sp_native/sp_native\")\n"; + +/// Build the pure-Ruby `sp_helper` gem and install it into `home`. +fn install_helper_gem(home: &Path) { + let src = home.parent().unwrap().join("sp_helper-src"); + std::fs::create_dir_all(src.join("lib")).unwrap(); + std::fs::write(src.join("lib/sp_helper.rb"), "module SpHelper; end\n").unwrap(); + std::fs::write( + src.join("sp_helper.gemspec"), + "Gem::Specification.new do |s|\n s.name = \"sp_helper\"\n s.version = \"1.0.0\"\n \ + s.summary = \"t\"\n s.authors = [\"t\"]\n s.files = [\"lib/sp_helper.rb\"]\nend\n", + ) + .unwrap(); + for (args, dir) in [ + (vec!["build", "sp_helper.gemspec"], src.clone()), + ( + vec![ + "install", + "--local", + "--no-document", + "--install-dir", + home.to_str().unwrap(), + "sp_helper-1.0.0.gem", + ], + src.clone(), + ), + ] { + let out = Command::new("gem") + .args(&args) + .current_dir(&dir) + .env("HOME", home.parent().unwrap()) + .output() + .unwrap(); + assert!( + out.status.success(), + "gem {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + } +} + +/// A crawl root spelled relative (`--cwd proj`, or the default `.`) used +/// to reach the build as a relative spec path and `GEM_PATH`, which Ruby +/// resolves from the project dir and `extconf.rb` from the ext dir: the +/// build could not load a gem from the copy's own home, so nokogiri +/// (`mini_portile2`) failed with `native_extension_build_failed`. +#[test] +#[ignore = "host capstone: compiles a C extension with the real ruby/gem toolchain; \ + the unpinned `test` job skips it, the pinned-Ruby e2e leg runs it via --ignored"] +fn native_patch_builds_from_a_relative_crawl_root() { + let Some(abi) = toolchain() else { + return; + }; + let patched = c_source(2); + let fx = Fixture::new(&abi, &patched); + install_helper_gem(&fx.home); + let socket = fx.proj.join(".socket"); + let entries = [ + ("ext/sp_native/extconf.rb", EXTCONF, EXTCONF_WITH_HELPER), + (C_KEY, &c_source(1) as &str, &patched as &str), + ]; + let hashes: Vec<(String, String)> = entries + .iter() + .map(|(_, b, a)| (git_sha256(b.as_bytes()), git_sha256(a.as_bytes()))) + .collect(); + let files: Vec> = entries + .iter() + .zip(&hashes) + .map(|((key, _, _), (b, a))| PatchEntry { + file_name: key, + before_hash: b, + after_hash: a, + }) + .collect(); + // `rollback` drops the record and frees its blobs, so this runs twice. + let record = || { + write_minimal_manifest(&socket, PURL, UUID, &files); + for ((_, b, a), (bh, ah)) in entries.iter().zip(&hashes) { + write_blob(&socket, bh, b.as_bytes()); + write_blob(&socket, ah, a.as_bytes()); + } + }; + record(); + + // `--cwd proj` from the project's parent. + let parent = fx.proj.parent().unwrap(); + let (code, stdout, stderr) = common::run(parent, &["apply", "--cwd", "proj", "--json"]); + assert_eq!(code, 0, "apply failed:\n{stdout}\n{stderr}"); + assert_eq!(event_action(&stdout), "applied", "{stdout}"); + assert_eq!(fx.value(), "2", "Ruby must load the rebuilt extension"); + + let (code, stdout, stderr) = common::run(parent, &["rollback", "--cwd", "proj", "--json"]); + assert_eq!(code, 0, "rollback failed:\n{stdout}\n{stderr}"); + assert_eq!(fx.value(), "1"); + + // The default `.` crawl root, run from inside the project. + record(); + let (code, stdout, stderr) = fx.run(&["apply", "--json"]); + assert_eq!(code, 0, "apply failed:\n{stdout}\n{stderr}"); + assert_eq!(event_action(&stdout), "applied", "{stdout}"); + assert_eq!(fx.value(), "2", "Ruby must load the rebuilt extension"); +} + fn walk_files(dir: &Path) -> Vec<(String, Vec)> { let mut out = Vec::new(); let mut stack = vec![dir.to_path_buf()]; diff --git a/crates/socket-patch-core/src/patch/gem_ext.rs b/crates/socket-patch-core/src/patch/gem_ext.rs index a226eb24b..a5209504d 100644 --- a/crates/socket-patch-core/src/patch/gem_ext.rs +++ b/crates/socket-patch-core/src/patch/gem_ext.rs @@ -125,6 +125,13 @@ impl GemCopy { /// The copy at `pkg_path`, when it sits in a RubyGems `gems/` dir. /// Whether its spec exists is checked by the caller. fn of(pkg_path: &Path) -> Option { + // Absolute, because a build runs Ruby from the project dir and + // `extconf.rb` from the ext dir: a relative crawl root (`--cwd .`, + // `--cwd proj`) would leave the spec path and `GEM_PATH` pointing + // nowhere, and an `extconf.rb` that loads `mini_portile2` (nokogiri) + // from the copy's own home would fail to build. + let pkg_path = std::path::absolute(pkg_path).ok()?; + let pkg_path = pkg_path.as_path(); let full_name = pkg_path.file_name()?.to_str()?.to_string(); let gems = pkg_path.parent()?; if gems.file_name()? != "gems" { From 5b1417c31c2999121bc6d2aa38eb1ddee6f17a2c Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Mon, 9 Nov 2026 19:45:01 -0500 Subject: [PATCH 2/2] Address /code-review on the relative gem crawl root fix - Keep a rooted path as is, so Windows doesn't put a drive on `/h` and break the gem_ext unit tests. Make only a relative root absolute, with relpath::absolute_lexically, which drops `..` the way Ruby's File.expand_path does and falls back to the given path on error instead of reading the copy as "not a RubyGems install". - Unit test: a relative `..` path becomes absolute and normalized. - Capstone: build the helper gem outside the crawled vendor/bundle tree, and also run `apply --check` and `rollback` on the default root. - Move walk_files' doc comment back onto it, and drop the now-stale "relative crawl root" reason from ext_dir_label's doc. Co-Authored-By: Claude Opus 5.5 --- .../tests/e2e_gem_native_ext_build.rs | 20 +++++++++---- crates/socket-patch-core/src/patch/gem_ext.rs | 28 +++++++++++++++++-- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs b/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs index 492873468..531574a3b 100644 --- a/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs +++ b/crates/socket-patch-cli/tests/e2e_gem_native_ext_build.rs @@ -303,15 +303,16 @@ fn native_patch_that_does_not_build_is_put_back() { assert_eq!(walk_files(&ext_dir), built_before); } -/// `(relative path, bytes)` of every file under `dir`. /// An `extconf.rb` that, like nokogiri's (which activates `mini_portile2`), /// loads a helper gem installed beside the copy in the same home. const EXTCONF_WITH_HELPER: &str = "gem \"sp_helper\"\nrequire \"sp_helper\"\n\ require \"mkmf\"\ncreate_makefile(\"sp_native/sp_native\")\n"; -/// Build the pure-Ruby `sp_helper` gem and install it into `home`. -fn install_helper_gem(home: &Path) { - let src = home.parent().unwrap().join("sp_helper-src"); +/// Build the pure-Ruby `sp_helper` gem in `scratch` (outside the project) +/// and install it into `home`. +fn install_helper_gem(scratch: &Path, home: &Path) { + let src = scratch.join("sp_helper-src"); + let user_home = scratch.join("gem-user-home"); std::fs::create_dir_all(src.join("lib")).unwrap(); std::fs::write(src.join("lib/sp_helper.rb"), "module SpHelper; end\n").unwrap(); std::fs::write( @@ -337,7 +338,7 @@ fn install_helper_gem(home: &Path) { let out = Command::new("gem") .args(&args) .current_dir(&dir) - .env("HOME", home.parent().unwrap()) + .env("HOME", &user_home) .output() .unwrap(); assert!( @@ -362,7 +363,7 @@ fn native_patch_builds_from_a_relative_crawl_root() { }; let patched = c_source(2); let fx = Fixture::new(&abi, &patched); - install_helper_gem(&fx.home); + install_helper_gem(fx._tmp.path(), &fx.home); let socket = fx.proj.join(".socket"); let entries = [ ("ext/sp_native/extconf.rb", EXTCONF, EXTCONF_WITH_HELPER), @@ -408,8 +409,15 @@ fn native_patch_builds_from_a_relative_crawl_root() { assert_eq!(code, 0, "apply failed:\n{stdout}\n{stderr}"); assert_eq!(event_action(&stdout), "applied", "{stdout}"); assert_eq!(fx.value(), "2", "Ruby must load the rebuilt extension"); + let (code, stdout, stderr) = fx.run(&["apply", "--check"]); + assert_eq!(code, 0, "a rebuilt patch checks clean:\n{stdout}\n{stderr}"); + let (code, stdout, stderr) = fx.run(&["rollback", "--json"]); + assert_eq!(code, 0, "rollback failed:\n{stdout}\n{stderr}"); + assert_eq!(fx.value(), "1", "the original build is back"); + assert!(!fx.marker().exists()); } +/// `(relative path, bytes)` of every file under `dir`. fn walk_files(dir: &Path) -> Vec<(String, Vec)> { let mut out = Vec::new(); let mut stack = vec![dir.to_path_buf()]; diff --git a/crates/socket-patch-core/src/patch/gem_ext.rs b/crates/socket-patch-core/src/patch/gem_ext.rs index a5209504d..d37b3a6b9 100644 --- a/crates/socket-patch-core/src/patch/gem_ext.rs +++ b/crates/socket-patch-core/src/patch/gem_ext.rs @@ -129,8 +129,14 @@ impl GemCopy { // `extconf.rb` from the ext dir: a relative crawl root (`--cwd .`, // `--cwd proj`) would leave the spec path and `GEM_PATH` pointing // nowhere, and an `extconf.rb` that loads `mini_portile2` (nokogiri) - // from the copy's own home would fail to build. - let pkg_path = std::path::absolute(pkg_path).ok()?; + // from the copy's own home would fail to build. `..` is dropped + // lexically, as Ruby's `File.expand_path` drops it from the spec + // path, so both name the same gem dir. A rooted path is kept as is. + let pkg_path = if pkg_path.has_root() { + pkg_path.to_path_buf() + } else { + crate::utils::relpath::absolute_lexically(pkg_path) + }; let pkg_path = pkg_path.as_path(); let full_name = pkg_path.file_name()?.to_str()?.to_string(); let gems = pkg_path.parent()?; @@ -809,7 +815,7 @@ struct BackupInfo { /// [`BuildMarker::extension_dir`] for `ext_dir` (one of the copy's /// installed extension dirs, [`find_ruby`]): relative to `home`, compared /// canonically too (Ruby and the crawler may spell the same dir -/// differently: a relative crawl root, `/tmp` vs `/private/tmp`), so the +/// differently through a symlink: `/tmp` vs `/private/tmp`), so the /// record survives the project moving. fn ext_dir_label(home: &Path, ext_dir: &Path) -> String { let canonical_rel = || { @@ -1638,6 +1644,22 @@ mod tests { assert!(GemCopy::of(Path::new("/h/bundler/gems2/json-abc")).is_none()); } + /// A relative crawl root reaches Ruby (run from the project dir) and + /// `extconf.rb` (run from the ext dir) as absolute paths, `..` dropped + /// the way Ruby's `File.expand_path` drops it. + #[test] + fn gem_copy_of_a_relative_path_is_absolute() { + let copy = GemCopy::of(Path::new("proj/../vendor/ruby/3.4.0/gems/json-2.10.1")).unwrap(); + let expected = std::env::current_dir().unwrap().join("vendor/ruby/3.4.0"); + assert_eq!(copy.home, expected); + assert!(copy.spec_path.is_absolute()); + assert!(copy.gem_dir.is_absolute()); + assert!(!copy + .gem_dir + .components() + .any(|c| c == std::path::Component::ParentDir)); + } + #[test] fn gem_name_strips_version_and_platform() { assert_eq!(gem_name_of("json-2.10.1"), "json");