Install content-addressable gemspecs under specifications/<ruby_abi>/ - #196
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Read-only directory reuse and in-memory cross-ABI activation remain incorrect.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves content-addressable gemspecs into Ruby ABI-scoped directories while preserving ordinary gem layouts.
Changes:
- Adds ABI-aware gemspec discovery, installation, and path handling.
- Updates uninstall and RequestSet behavior.
- Expands RubyGems and Bundler coverage.
File summaries
| File | Description |
|---|---|
lib/rubygems/specification_record.rb |
Defines ABI-scoped layout and discovery. |
lib/rubygems/specification.rb |
Resolves ABI-scoped specification paths. |
lib/rubygems/installer.rb |
Writes and discovers ABI-scoped gemspecs. |
lib/rubygems/request_set.rb |
Finds installed ABI-scoped specs. |
test/rubygems/test_gem_specification.rb |
Tests discovery and path semantics. |
test/rubygems/test_gem_installer.rb |
Tests installation and directory modes. |
test/rubygems/test_gem_uninstaller.rb |
Tests ABI-scoped uninstall cleanup. |
test/rubygems/test_gem_commands_update_command.rb |
Updates command installation expectations. |
spec/install/gemfile/content_addressable_spec.rb |
Verifies Bundler’s scoped layout. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
475b421 to
914dfb5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Directory-mode handling is incomplete, and one new test fails on Ruby 3.4 CI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/installer.rb:432
- The new ABI-scoped specifications directory never gets the requested
dir_mode:ensure_writable_dircreates it as0755, and onlygem_diris chmoded later. Thus--dir-mode 0555succeeds in the new test only because the shared ABI directory remains writable, contrary to the documented write-before-clamp behavior. Please apply the requested mode after writing while preserving a workable strategy for subsequent installs, and assert the ABI directory's mode.
spec_file = self.spec_file
spec_dir = File.dirname spec_file
ensure_writable_dir spec_dir unless File.directory? spec_dir
Gem.write_binary(spec_file, spec.to_ruby_for_cache)
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Balanced
914dfb5 to
6d929c6
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Bundler local-spec resolution and shared-directory permission handling have unresolved correctness and concurrency issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/installer.rb:442
- If writing the gemspec fails after the existing read-only ABI directory was temporarily changed to
0755(for example, due to ENOSPC), this final chmod is skipped and the shared directory remains more permissive than the requested mode. Restoredir_modein anensureblock so failed installs cannot leave permissions altered.
Gem.write_binary(spec_file, spec.to_ruby_for_cache)
File.chmod(dir_mode, spec_dir) if dir_mode && content_addressed
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Review effort level: Balanced
bf8f633 to
7d45f8e
Compare
4a19f2f to
af7b1b5
Compare
fa060b5 to
b6c4df8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The temporary permission change exposes private ABI directories to additional users during installation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Balanced
b6c4df8 to
9245c64
Compare
Assisted-By: devx/9c2464e2-74cd-4d36-a6c0-50aef8c2ad01
9245c64 to
590aea0
Compare
fee510c
into
feature-branch-ca-changes-rubygems
Content-addressable gemspecs install under
specifications/<ruby_abi>/instead of the flatspecifications/directory. Implements item 2 of hsbt's format review. Supersedes #195 with a deliberately smaller scope.Why
A CA gemspec in the flat directory poisons a shared GEM_HOME for older RubyGems: the stub line carries the content address where the platform used to be, so an older
StubLineparses it as platformunknownand derives agem_dirthat doesn't exist —gem listshows the gem whilegem "name"raisesMissingSpecError.Older RubyGems globs
specifications/*.gemspecnon-recursively, so a subdirectory is invisible by construction — nothing to parse, nothing to misinterpret.specifications/default/and #192'splugins/<ruby_abi>/are the existing precedents.Install
Installer#spec_filedelegates toSpecificationRecord.specification_dir_for(spec, @gemhome), which returns the ABI-scoped path for CA specs and the flat path for everything else. No branching in the installer itself — the record is the single source of layout truth.write_speccreates the ABI dir lazily via the existingensure_writable_dir(soGem::FilePermissionErrorand Bundler's override apply unchanged). It then writes the gemspec, and only after the write succeeds, clamps the directory to--dir-mode— matching thegem_dir/build_info_dirtwo-step. The chmod runs in anensureblock, so a failed write can't leave the directory more permissive than requested. For sequential installs into a read-only (0555) ABI dir, writability is restored to 0755 before the write and re-clamped after.Installer#installed_specsandRequestSet#specs_inglob throughdirs_from, so already-installed CA gems are recognized during install planning.Read
SpecificationRecord.dirs_fromexpands each gem path into two search dirs: the flatspecifications/andspecifications/<Gem.ruby_abi>/. Only the running Ruby's ABI is searched, so a CA build for another ABI sharing the GEM_HOME is never an activation candidate — this also fixes the measured case where two CA builds of one name-version had identicalsort_objvalues andDependency#to_speccould activate a~> 3.4.0build on a different Ruby.map_stubsstrips one extra path level for ABI-scoped dirs when derivingbase_dir/gems_dir, so stubs receive the correctbase_dirwithout any path inference on the stub side.Specification#base_dirstrips 3 levels for specs loaded from an ABI-scoped dir (parallel todefault_gem?), and#spec_dirdelegates tospecification_dir_for.Bundler's
EndpointSpecification#local_specification_pathdelegates tospecification_dir_forso post-install spec reload finds CA gemspecs in the ABI dir.RubygemsIntegration#spec_cache_dirsusesdirs_fromso the fetcher's local cache lookup includes ABI-scoped dirs. Both fall back to flat-only on older system RubyGems via shims inrubygems_ext.rb.Uninstall
Uninstall and pristine work with zero uninstaller changes.
Uninstallerusesspec.spec_file(which delegates tospecification_dir_for) to locate and delete the gemspec, andspec.gem_dir/spec.base_dirfor the payload. Since the path model already resolves correctly for ABI-scoped specs, the uninstaller removes the ABI-scoped gemspec + payload + cache without any awareness of the subdirectory layout.Out of scope (follow-ups)
gem doctorawareness of ABI dirs (conservative union-of-ABIs installed set)bundle clean(currently: kept CA gems fully protected viaspec.spec_file; stale CA gemspecs are conservatively never deleted)gem contents --spec-dirsort_objtie between two rebuilds (different hashes, same name-version-ABI) — survives this change, open question whethersort_objshould tie-break oncontent_address