Add content addressable gems support - #9773
Conversation
4a5def3 to
d849a56
Compare
ce7e319 to
8d050c3
Compare
tenderlove
left a comment
There was a problem hiding this comment.
I didn't review the specs super closely, I trust they are covering useful scenarios.
It looks like we've got a lot of array / hash manipulation going on in this PR. Should we be thinking about making real, named objects?
The direction looks good here IMO
|
|
||
| def hash | ||
| @set.hash ^ @name.hash ^ @version.hash ^ @platform.hash | ||
| @set.hash ^ @name.hash ^ @version.hash ^ @platform.hash ^ @content_address.hash |
There was a problem hiding this comment.
No impact on this PR, but this is a bad hash and we should fix it upstream.
We should be doing [@set, @name, ...].hash
|
|
||
| def hash # :nodoc: | ||
| name.hash ^ version.hash | ||
| [name, version, platform, content_address].hash |
| end | ||
|
|
||
| def spec_platforms(entry, platforms) | ||
| platforms = platforms.transform_values(&:uniq) |
There was a problem hiding this comment.
This is because it's now a hash of hashes? What is the structure of platforms?
There was a problem hiding this comment.
platforms comes from line 213 in output_versions. It's a one-level hash
platforms = Hash.new {|h,version| h[version] = [] }
In reality it would look something like this:
{
Gem::Version.new("1.0.0") => [
Gem::Platform::RUBY,
Gem::Platform.new("x86_64-linux"),
Gem::Platform.new("x86_64-linux")
],
Gem::Version.new("0.9.0") => [
Gem::Platform::RUBY
]
}
And then `transform_values(&:uniq) would remove any dups in the values.
|
|
||
| Gem::NameTuple.new(name, version, platform || "ruby") | ||
| suffix ||= "ruby" | ||
| content_address = suffix if Gem::ContentAddress.match?(suffix) |
There was a problem hiding this comment.
This logic is because we're using this same loop with the CA and non-CA RubyGems endpoints?
There was a problem hiding this comment.
Yep. The compact-index /versions response uses the same suffix field for both formats, legacy entries contain a platform, while CA entries contain a content-address token. This handles both, so it detects CA suffixes and stores them as content_address. The actual platform is decoded later from the gem’s /info metadata!
| platform: platform, | ||
| ruby_abi: ruby_abi_from(requirements[:ruby]), | ||
| } | ||
| end |
There was a problem hiding this comment.
It feels like we should make a real object here. Just spitballing but like:
class GemInfo < Struct.new(:version, :suffix, :platform, :ruby_abi)
def hash; suffix; end
def eql?(other); other.version == version && other.suffix == suffix; end
endThough now that I type this out, it seems very similar to NameTuple? It feels like we could be doing more simple code here with set intersections. e.g. wanted_rows.map { make_obj(_1) } & compact_index_info_rows(name).map { make_object(_1) }
There was a problem hiding this comment.
@tenderlove have implemented a solution in this commit, interested in your thoughts!
2b193ae to
467a7a5
Compare
TestingWith: https://rubygems.org/gems/content_addressable_test 1.1
|
6dc3207 to
f854b32
Compare
Replace Object#present? (ActiveSupport) with a plain truthy check on content_address in CompactIndex::GemVersionMethods, so the vendored lib/compact_index* files no longer rely on Rails being loaded. content_address is either nil or a non-empty hex string (the Version model's CONTENT_ADDRESS_FORMAT validation rejects empty/other values, allow_nil: true), so a truthy check is equivalent to present? here. Ref: ruby/rubygems#9773 (comment)
Added a follow up issue to explore refactoring away the hashes and arrays #9834 - but it's not a blocker for this initial support as we discussed privately. cc: @tenderlove |
27472f4 to
8b25dd2
Compare
Assisted-By: devx/9c2464e2-74cd-4d36-a6c0-50aef8c2ad01
Assisted-By: devx/9c2464e2-74cd-4d36-a6c0-50aef8c2ad01
Assisted-By: devx/9c2464e2-74cd-4d36-a6c0-50aef8c2ad01
3ffc2fb to
f2c0576
Compare
|
Another round of testing after changes 1. Query commands:
|
|
@hsbt I tested this branch throughly with the new changes. The suffix widening is still WIP so it would be great to get this merged and we'll create a follow up PR when that's ready. I also added fixes for gem doctor and bundle clean wrt the new abi specification directories and adjusted the platform field in the compact index spec since the existing syntax had an |
hsbt
left a comment
There was a problem hiding this comment.
Thank you for working through the design and for implementing both the server and the client side 🙏
Merging this now, with time to stabilize it before the final release.
|
@jenshenny @OughtPuts Sorry for my late review. I think that
Users need to build If users just use |
|
Related: #9863 |
|
@kou yes, the |
|
Thanks for confirming my questions. Why do you choose this design? I feel that a tool can change |
|
I believe it was @hsbt 's suggestion as someone can compile the extension themselves and use btw @OughtPuts and @Edouard-chin are working on adding support to the available tooling right now and mentioned they possibly need to make some additional changes to the build command. |
|
Hey @hsbt - thank you for your support with this. I've been working on a solution to your comment:
Are you able to go into more detail about what you would like to guard against happening here? Having looked at it quite carefully it doesn't seem likely that we would end up with a conflict between two gems that are the same name-version-ruby-abi but have different content addresses (one widened, one not). The server only allows one gem name-version-ruby-abi combination, so in the majority of cases, if a user needs Apologies if I've missed something, I'd be grateful for your thoughts on this. We'd also be happy just to monitor the situation and see if problems arise. |
|
@jenshenny When I reviewed the RFC, I had cases like these in mind:
I am not sure the toolchain really needs this, but switching through every interpreter just to build the gems for the former is clearly cumbersome. @OughtPuts My note pointed at something the RFC defines, but I had missed that rubygems.org already widens colliding addresses. I briefly reconsidered how often widening would affect the client, and collisions are rare in practice, so personally I don't think the cleanup is needed. Please decide with @jenshenny. |
Thanks for sharing the context.
Yes. I feel that the BTW, we already have the
In this case, we don't need to switch Ruby. We can just set
In this case, I think that the "artifact" includes not only |
|
Somewhat off topic and a larger task, but is there any ongoing discussion about the gem namespace more broadly? For example adding 'orgs' like NPM have? ( If so, one could also consider folding content-addressable gems into that, with some other delimiter. For example It's quite a bit of work to change namespace delimiters, so if you are about to do it for orgs, maybe consider content-addressable gems too? |
Sorry I was away for a few days. Based on what was clarified above, I agree that the Changing the flag to be more content address oriented (
@hsbt @kou Unless other opinions come up, we'll open a PR for the flag change to the gem build command and make the modifications to the tooling (rake-compiler) etc. |
|
@sandstrom not that I'm aware of.. I believe the latest discussion for namespaces was being discussed here rubygems/rfcs-archived#40. Namespaces would still be very valuable but I don't think anyone has revived any concrete plans for it yet. The delimiters idea would be interesting to fold in! |
|
Thanks for considering my suggestion. If |
|
Thank you for the insightful feedback ❤️
Yeah I'm not 100% sure if we need the @hsbt do you have any objections to changing the |
|
Thanks for raising all these points around I see the merits of making the change to
I've noticed today that Appreciate that the use cases are different from each other but I wanted to double check that we're still happy for the |
#9654
TL;DR
Adds RubyGems and Bundler client support for content-addressable ("skinny") binary gems: one artifact per Ruby ABI, named with a SHA-256 prefix.
The branch covers build, discovery, install, display, yank, lockfiles, caching, and
bundle install --local. Existing source and platform gems are unchanged.Why
"Fat" binary gems contain every supported Ruby ABI and keep growing. Skinny binaries are smaller, but builds for the same gem, version, and platform need distinct filenames. A content-derived suffix gives each artifact a unique identity.
A content address must be 8–64 lowercase hexadecimal characters. RubyGems only treats it as one when the gem also has a non-Ruby platform and constrained
required_ruby_version, avoiding false matches with ordinary filenames.Follow ups
gem buildref: Shopify/rubygems#171.
User behaviour
gem build nokogiri.gemspec --ruby-abi 3.4builds a skinny gem namednokogiri-1.18.9-78be552b.gem, where the suffix is the SHA-256 digest of the gem contents.--ruby-abi, behaviour is unchanged.Details
--ruby-abivalidates the ABI format (X.Y), requires a non-Ruby platform to be set, and constrainsrequired_ruby_version: if unset it defaults to~> X.Y.0; a mismatched existing requirement is rejected.gem installref: Shopify/rubygems#172 (local) and #173 (remote).
Local
gem install --local GEMNAMEis content-addressable aware.Every install of a CA gem writes a gemspec stub whose
# stub:suffix is the hash (so thename-version-<sha>directory resolves). The real platform rides on a separate# stub-target:line that older RubyGems ignore — backwards compatible, while current RubyGems recover both. File:specifications/mygem-1.0-78be552b.gemspec:Remote
platform:=metadata separately. Distinct hashes remain distinct candidates.For example, a server
info/nokogiriresponse with two skinny variants (different Ruby ABIs) plus a platform fallback:The hash is carried in the version token (
1.18.9-78be552b); the real platform and Ruby requirement travel in theplatform:=/ruby:metadata. The two hashes stay distinct resolver candidates, and1.18.9-x86_64-linuxis the platform fallback.gem pushref: Shopify/rubygems#174.
User behaviour
gem push name-*.gem --platform x86_64-linux --ruby-abi 3.4reads the specs of the SHA-named files and pushes the single matching artifact.Details
--platformand--ruby-abiselectors. Given multiple SHA-named files, RubyGems reads each specification and selects the one whose platform andrequired_ruby_versionsatisfy both selectors.ruby_matches?does not check platform (a RUBY-platform gem with a matching~> X.Y.0can be selected by--ruby-abi); this is documented in the tests rather than special-cased.gem yankref: Shopify/rubygems#176.
User behaviour
gem yank mygem -v 1.0.0 --platform x86_64-linux --ruby-abi 3.4sends gem name, version, platform, and ABI so the server can select one skinny variant.--ruby-abi.Remote queries (
gem list/search/info -r)ref: Shopify/rubygems#175.
User behaviour
gem dependencyref: Shopify/rubygems#189.
User behaviour
gem dependency GEM --remotelists content-addressable gems with their content-addressed full name plus the real platform and Ruby ABI, instead of treating the hash as the platform:Details
fetch_remote_specsruns the detected tuples throughGem::SpecFetcher#decode_content_addressable_tuples, which splits the hash from the real platform via the source, fetches each spec, and carriescontent_addressonto it (dependency_command.rb:69-73).content_address_annotationappends(Platform: <plat>, Ruby ABI: <abi>)only whenGem::ContentAddress.content_addressed?(spec)is true (dependency_command.rb:155-165).bundle install(lockfile + local cache)ref: Shopify/rubygems#177 (remote) and #178 (lockfile + local cache).
Remote
Lockfile and local cache
Gemfile.lockand parses both on the next run.vendor/cachewithbundle install --local. Remote and local paths produce the same installed directory; checksums remain keyed by the platform lock name. A CA gem locks with the hash in the version and the real platform beside it.vendor/cache