Install content-addressable gemspecs under specifications/<ruby_abi>/ - #195
Open
jenshenny wants to merge 21 commits into
Open
Install content-addressable gemspecs under specifications/<ruby_abi>/#195jenshenny wants to merge 21 commits into
jenshenny wants to merge 21 commits into
Conversation
Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
Co-authored-by: Harriet Oughton <harriet.oughton@shopify.com> Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
Co-authored-by: Gira Chawda <gira.chawda@shopify.com> Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
…al cache Co-authored-by: Jenny Shen <jenny.shen@shopify.com>
Gem::Indexer (rubygems-generate_index) loads the V1-only CompactIndex constants, either from the compact_index gem or from its own embedded copy, so the artifice's copy of rubygems.org's V2-only implementation can never share that name safely. Check in a copy of rubygems.org's lib/compact_index, renamed to the VendoredCompactIndex namespace, under spec/support/vendor/compact_index, and load it from the artifice with a plain require. Checking the copy in (rather than downloading it during the test run) keeps the suite hermetic and offline, makes the namespace rewrite visible in review, and lets parallel workers and CI runners that skip the test-deps setup load it without falling back to the incompatible gem. Refresh the copy with `rake vendor:compact_index`; `rake vendor:compact_index_check` fails if the checked-in copy drifts from the pinned upstream ref.
Assisted-By: devx/7307f7b7-fac2-4c30-9651-f8ce7643f65c
A content-addressable gem was locked inline as name (version-address)
platform. Bundler 4.0 parses the token after the version as the platform,
gets unknown, materializes name-version-unknown, and fails: it then either
re-resolves and rewrites the lockfile or fails outright under frozen.
Lockfiles are committed and read by many Bundler versions during
co-publication, so the inline form breaks the exact clients the transition
is supposed to protect.
The spec line is now an ordinary platform pin, and the content address moves
to its own section:
GEM
specs:
nokogiri (1.19.4-x86_64-linux)
CONTENT ADDRESSES
nokogiri (1.19.4-x86_64-linux) 86e5e59f sha256=<content-addressable build's sha>
CHECKSUMS
nokogiri (1.19.4-x86_64-linux) sha256=<platform build's sha>
Verified against the released Bundler 4.0.9:
- unknown unindented sections set @parse_method = nil and are skipped
silently (lockfile_parser.rb:137)
- Definition#lockfiles_equal? subtracts unknown sections before comparing
(definition.rb:1196), so a plain 4.0 bundle install does not rewrite the
lockfile and the section survives; it is only dropped on a genuine 4.0
re-lock, and restored on the next 4.1 re-lock
- 4.0 classifies a locked spec with a missing or empty CHECKSUMS entry as a
lockfile change (definition.rb:619-620): a plain install re-resolves and
frozen mode hard-fails. The platform lock name line must therefore carry a
real checksum, and it must be the platform build's, since 4.0 attributes
it to the artifact it installs for that lock name
That last point drives the checksum rules:
- the checksum store is keyed by full name instead of lock name, because a
content-addressable build and the platform build of the same name,
version, and platform share a lock name while being different files.
Store#register accepts anything responding to full_name and lock_name, so
the parser registers CHECKSUMS entries under the name tuple parsed from
the line rather than the (possibly content-addressed) spec object
- the content-addressable build's checksum is serialized next to its
address in CONTENT ADDRESSES, the only place older Bundler never parses,
and verifies the downloaded gem via the existing install-time registration
- CHECKSUMS carries the platform build's checksum, which the compact index
supplies for every row the fetcher sees, so it is captured during
resolution without downloading the platform gem
- when no platform build exists (skinny-only publication), the CHECKSUMS
line is omitted entirely rather than written bare, so every CHECKSUMS
line describes an artifact installable by its lock name
The section registers in SECTIONS_BY_VERSION_INTRODUCED under 4.1.0, and
its line format requires a platform, since content addressing only applies
to platformed gems.
Assisted-By: devx/01a05e2e-d421-7e27-b54e-abee85203da5
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
2 times, most recently
from
September 2, 2026 03:50
2b525ea to
d1a2493
Compare
Skinny gems built with --ruby-abi did not carry a required_rubygems_version constraint, so older RubyGems clients (4.0.x) could install them through the local path (gem install ./file.gem, vendor/cache, bundle install --local) with no warning. The content address was silently discarded and a gem scoped to one Ruby ABI would activate on an incompatible Ruby sharing that GEM_HOME. The remote path was safe only by accident: the version token parsed to platform=unknown which failed Gem::Platform.match_spec?. Now the protection is the one the RFC describes -- older clients reject the gem because their RubyGems version doesn't satisfy >= 4.1.0.a. Published artifacts are immutable, so any skinny gem released without this constraint stays visible to old clients permanently. Gem::Package.build now derives the required_rubygems_version for the built gem from the gemspec's own requirement: - unset requirements become >= 4.1.0.a - requirements that already satisfy that floor are left untouched - weaker requirements are rewritten to the floor plus any declared upper bounds and exclusions, with a warning since the built gem's metadata will differ from the gemspec - requirements that cap below the floor (e.g. < 4.0, ~> 3.5) raise at build time, since combined with the floor no RubyGems version could ever install the resulting gem As with required_ruby_version, the derived value is propagated back to the original spec only after a successful build. Assisted-By: devx/01a05e2e-d421-7e27-b54e-abee85203da5
There was a problem hiding this comment.
🟡 Changes recommended
Legacy uninstall paths, plugin isolation, doctor validation, and explicit specification-directory lookup 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 to isolate incompatible installations and older RubyGems versions.
Changes:
- Adds ABI-scoped gemspec installation and discovery.
- Updates RubyGems and Bundler consumers for the new layout.
- Extends doctor, clean, and installation tests.
File summaries
| File | Description |
|---|---|
test/rubygems/test_gem_specification.rb |
Tests scoped path discovery and ABI isolation. |
test/rubygems/test_gem_installer.rb |
Tests installation paths and permissions. |
test/rubygems/test_gem_doctor.rb |
Tests preservation of ABI-scoped artifacts. |
test/rubygems/test_gem_commands_update_command.rb |
Updates expected gemspec path. |
spec/install/gemfile/content_addressable_spec.rb |
Tests Bundler installation and cleaning. |
lib/rubygems/specification.rb |
Models scoped specification paths. |
lib/rubygems/specification_record.rb |
Discovers current-ABI specifications. |
lib/rubygems/resolver/lock_specification.rb |
Checks scoped paths for installed gems. |
lib/rubygems/request_set.rb |
Loads specs from scoped directories. |
lib/rubygems/installer.rb |
Writes gemspecs under ABI directories. |
lib/rubygems/doctor.rb |
Preserves ABI-scoped installations. |
lib/rubygems/commands/contents_command.rb |
Includes scoped specification paths. |
lib/bundler/runtime.rb |
Cleans scoped gemspecs. |
lib/bundler/rubygems_integration.rb |
Adds scoped Bundler cache directories. |
lib/bundler/endpoint_specification.rb |
Resolves scoped local gemspec paths. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
OughtPuts
reviewed
Sep 2, 2026
OughtPuts
left a comment
OughtPuts
left a comment
There was a problem hiding this comment.
Looks good! Thanks for putting this together!
It would be good to also have some test coverage for fallback branches where Gem.respond_to?(:ruby_abi) is false. (e.g. endpoint_specification).
OughtPuts
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 2, 2026 15:42
3b28e20 to
f38bfa4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Legacy flat gemspecs survive reinstalls, and numeric custom specification directories receive an incorrect base path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/installer.rb:431
- Reinstalling a content-addressed gem from the legacy flat layout leaves the old
specifications/<full_name>.gemspecin place. The new scoped copy is added, but the flat copy remains visible to older RubyGems (preserving the split-brain behavior this change is intended to eliminate) and produces duplicate stubs in current RubyGems. After the scoped write succeeds, remove the legacy flat file and cover this migration in the reinstall test.
Gem.write_binary(spec_file, spec.to_ruby_for_cache)
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Balanced
OughtPuts
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 2, 2026 16:42
f38bfa4 to
bf5a4ca
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Doctor can traverse ABI-directory symlinks, and bundle clean can orphan sibling-ABI gemspecs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/doctor.rb:148
- This also preserves numeric symlinks as if they were valid ABI directories. Excluding symlinks here restores the doctor's prior behavior of removing stray links and complements the traversal guard when enumerating ABI directories.
next if %w[specifications plugins].include?(sub_directory) &&
File.directory?(child) && /\A\d+\.\d+\z/.match?(File.basename(child))
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Balanced
…ute every eligibility, naming, lockfile, plugin directory, and spec construction decision through its shared predicates Assisted-By: devx/1070d399-4c30-4b1e-b5e5-dd5f191f26cf
…t carry the content address the lockfile or index declared Assisted-By: devx/1070d399-4c30-4b1e-b5e5-dd5f191f26cf
…verything is content addressed for another Ruby, so resolution reports the Ruby version requirement instead of no gem found Assisted-By: devx/1070d399-4c30-4b1e-b5e5-dd5f191f26cf
…ersion that pre-4.1 RubyGems versions do not satisfy Assisted-By: devx/1070d399-4c30-4b1e-b5e5-dd5f191f26cf
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
2 times, most recently
from
September 3, 2026 01:23
b0ab784 to
103cd1d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Older RubyGems can still discover scoped gemspecs through Bundler, and doctor can retain invalid scoped duplicates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/doctor.rb:124
- Passing ABI directories through
doctor_childdoes not reliably remove invalid scoped gemspecs because that method first accepts any basename present in the globalinstalled_specslist. For example, with a valid flata-1.gemspecand a non-content-addressedspecifications/9.9/a-1.gemspec,installed_specs.include?("a-1")causes the invalid scoped copy to be retained. Scoped entries need path-specific validation rather than global full-name membership.
abi_scoped_specification_dirs.each do |dir|
doctor_child File.join("specifications", File.basename(dir)), ".gemspec"
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 01:31
103cd1d to
7d1ed61
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Doctor can preserve invalid current-ABI entries, and the documented fallback behavior conflicts with the tests and implementation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Balanced
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 02:03
7d1ed61 to
670d4c3
Compare
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
2 times, most recently
from
September 3, 2026 02:07
0ec94e8 to
59a4338
Compare
There was a problem hiding this comment.
🟡 Changes recommended
One added test cannot pass, and direct --spec-dir inputs still miss ABI-scoped gemspecs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/commands/contents_command.rb:207
- Passing the actual
.../specificationsdirectory to--spec-dirstill searches that directory for flat gemspecs, but the ABI path generated here is.../specifications/specifications/<abi>, so CA gemspecs in.../specifications/<abi>remain undiscoverable. Preserve direct specification-directory inputs by adding their current-ABI child as well.
options[:specdirs].flat_map do |i|
[i, *Gem::SpecificationRecord.specification_dirs_in(i)]
end
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 02:11
59a4338 to
16d3726
Compare
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 02:15
16d3726 to
58a0bac
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Doctor currently preserves malformed ABI-scoped installations, and one legacy-uninstall test does not construct a content-addressed spec.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/rubygems/doctor.rb:62
- ABI-scoped specs are not validated consistently: the current ABI is skipped because
Gem::Specificationalready loaded it, but that generic record accepts ordinary specs in the ABI directory; other directories also accept a CA spec whoseruby_abidoes not match their basename. Doctor consequently preserves misplaced gemspecs and payloads that are not valid installations for that directory. Exclude scoped specs from the generic list, validate every ABI directory, and require the pinned ABI to match.
next [] if File.basename(dir) == Gem.ruby_abi
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 02:20
58a0bac to
b97db81
Compare
A CA gemspec written into the flat specifications/ directory poisons the GEM_HOME for older RubyGems sharing it. The stub line carries the content address in the field where the platform used to be, so an older StubLine reads it as a platform, gets unknown, and derives a gem_dir that does not exist: gem list shows the gem while activating it raises MissingSpecError. Older RubyGems globs specifications/*.gemspec non-recursively through Gem::Util.glob_files_in_dir, so a subdirectory is invisible to it by construction (specifications/default/ is the existing precedent). Installing CA gemspecs under specifications/<ruby_abi>/ means older clients never see them at all: nothing to parse, nothing to misinterpret. It also scopes visibility on 4.1 itself: the read side only adds the current Ruby's ABI subdirectory, so a CA build for another Ruby ABI sharing the same GEM_HOME is not a candidate for activation. This fixes the measured ambiguity where a ~> 3.4.0 build activated on a different Ruby with no warning. Two CA builds of the same name, version, and ABI can still tie in sort_obj; making that activation deterministic is left for a follow-up. Write side: - Installer#spec_file places CA gemspecs in specifications/<abi>/, using the gem's ABI and falling back to the installing Ruby's ABI when the gem's required_ruby_version is not a single-ABI pin - Installer#write_spec creates the subdirectory honoring --dir-mode Read side: - SpecificationRecord.dirs_from emits specifications/<current abi> next to each specifications dir, feeding stubs/stubs_for/all, so gem list, activation, and Bundler inherit the new location - SpecificationRecord#map_stubs derives base_dir correctly for the extra directory level - Installer#installed_specs and RequestSet#specs_in glob both directories Path model: - Specification#base_dir strips the extra level for CA specs loaded from an ABI-shaped subdirectory - Specification#spec_dir points at the ABI subdirectory for CA specs, so uninstall and pristine keep working unchanged Tooling: - gem doctor no longer deletes specifications/<ruby_abi>/ and plugins/<ruby_abi>/ as stray entries (the latter protects the plugin stubs introduced in 1821756, which doctor would otherwise remove) - bundle clean also globs specifications/<ruby_abi>/*.gemspec, so stale content-addressable gemspecs are cleaned up and current ones kept Assisted-By: devx/01a05e2e-d421-7e27-b54e-abee85203da5
jenshenny
force-pushed
the
ca-abi-scoped-specifications
branch
from
September 3, 2026 02:25
b97db81 to
2247583
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Directory snapshotting breaks live search-path updates, and doctor can preserve invalid ABI-scoped duplicates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Balanced
|
|
||
| def installed_specs # :nodoc: | ||
| @installed_specs ||= Gem::Specification.map(&:full_name) | ||
| @installed_specs ||= flat_installed_specs + abi_scoped_installed_specs |
| @active_stub_with_requirable_file = {} | ||
|
|
||
| @dirs = dirs | ||
| @dirs = dirs.uniq |
jenshenny
force-pushed
the
feature-branch-ca-changes-rubygems
branch
3 times, most recently
from
September 5, 2026 23:16
3ffc2fb to
f2c0576
Compare
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
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.
What
Implements item 2 of hsbt's format review: content-addressable gemspecs install under
specifications/<ruby_abi>/instead of the flatspecifications/directory.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 (Gem::Util.glob_files_in_dir), so a subdirectory is invisible by construction — nothing to parse, nothing to misinterpret.specifications/default/is the existing precedent.It also scopes visibility on 4.1 itself: the read side only adds the current Ruby's ABI subdirectory, so a CA build for another ABI sharing the GEM_HOME is never an activation candidate. This fixes the measured case from the review (a
~> 3.4.0build activating on a different Ruby with no warning).Changes
Write side
Installer#spec_fileplaces CA gemspecs inspecifications/<abi>/(the ABI pinned byrequired_ruby_version; only~> X.Y.0qualifies a gem for content addressing — gems without a single-ABI pin are not content-addressed and remain in the flat layout)Installer#write_speccreates the subdirectory through the existingensure_writable_dirhelper (permission failures surface asGem::FilePermissionError, and Bundler's override applies) and clamps it to--dir-modeafter writing, matching thegem_dir/build_info_dirtwo-stepRead side
SpecificationRecord.dirs_fromemitsspecifications/<current abi>next to each specifications dir —stubs/stubs_for/allinherit it, sogem list, activation, and Bundler all work unchangedSpecificationRecord.specification_dirs_in(path)is the single source of truth for "where gemspecs live";dirs_from,Installer#installed_specs,RequestSet#specs_in,gem contents --spec-dir, andResolver::LockSpecificationall go through itSpecificationRecord#map_stubsderivesbase_dircorrectly through the extra level, keyed on the directory's own ABI-shaped basename so a GEM_PATH entry named.../specificationscan't misclassifyPath model
Specification#base_dirstrips the extra level for CA specs loaded from an ABI-shaped subdir;#spec_dirpoints at it — uninstall and pristine work unchangedTooling
gem doctorno longer deletesspecifications/<abi>/andplugins/<abi>/as stray entries (the latter protects the plugin stubs from 1821756, which doctor currently removes)gem doctor's notion of "installed" now includes gemspecs from every ABI subdirectory, not just the running Ruby's: other-ABI installs are invisible toGem::Specificationbut theirgems/,cache/, and other artifacts belong to valid installations and must not be treated as stray (previouslygem check --doctordeleted a sibling Ruby's CA gem payload, orphaning its install). Also requiresfileutils, which doctor used but never loadedbundle cleanalso globsspecifications/<abi>/*.gemspec; the kept-gems list resolves through the ABI-scopedSpecification#spec_file, so in-use CA gemspecs are protectedBundler on older RubyGems
Gem.ruby_abi(Runtime#clean,RubygemsIntegration#spec_cache_dirs,EndpointSpecification#local_specification_path) are guarded withGem.respond_to?(:ruby_abi), so Bundler running against released RubyGems neither crashes nor manages ABI directories it cannot correctly reason about (its keep-list paths would be flat there) — consistent with theGem::ContentAddressshim philosophy that old stacks don't see CA artifacts at allVerified against released RubyGems 4.0.16 (shared GEM_HOME)
gem listafter CA installMissingSpecErrorwhile listed (split-brain)gem check --doctoron shared home--dir-mode 0700specifications/<abi>/is 0700