Skip to content
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.

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

set required_rubygems_version for native gems that specify the linux libc #236

Conversation

flavorjones
Copy link
Contributor

Problem I'm trying to solve

Rubygems does not correctly recognize -musl or -gnu platform suffixes until v3.3.22.

Solution

If rake-compiler is building a linux native gem that specifies a libc in its platform object, then add ">= 3.3.22" to the required_rubygems_version requirement.

https://github.com/rubygems/rubygems/blob/master/CHANGELOG.md#3322--2022-09-07

Context

While working on musl support in the precompilation toolchain:

I noticed that Ruby 3.0 is still shipping with Rubygems 3.2.33, which does not recognize these gem platforms.

Specifying the rubygems requirement changes the error experienced by users during gem installation from:

ERROR: While executing gem ... (Gem::Exception)
Unable to find spec for #<Gem::NameTuple rcee_precompiled, 0.6.test.2024.0128.1724, aarch64-linux>

to:

ERROR: Error installing rcee_precompiled-0.6.test.2024.0128.1735-x86_64-linux-musl.gem:
rcee_precompiled-0.6.test.2024.0128.1735-x86_64-linux-musl requires RubyGems version >= 3.3.22.
The current RubyGems version is 3.2.33. Try 'gem update --system' to update RubyGems itself.

/cc @deivid-rodriguez @segiddins

@flavorjones flavorjones force-pushed the flavorjones-set-req-rubygems-version-for-linux-libc-platforms branch 2 times, most recently from ee7aefb to b76b47a Compare January 28, 2024 19:10
Comment on lines 286 to 288
spec.required_rubygems_version = Gem::Requirement.new(gem_spec.required_rubygems_version)
spec.required_rubygems_version.concat([">= 3.3.22"])
Copy link
Member

Choose a reason for hiding this comment

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

Can we simplify this?

Suggested change
spec.required_rubygems_version = Gem::Requirement.new(gem_spec.required_rubygems_version)
spec.required_rubygems_version.concat([">= 3.3.22"])
spec.required_rubygems_version = spec.required_rubygems_version.requirements + [">= 3.3.22"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gem::Requirements does not support the #+ method. See https://docs.ruby-lang.org/en//3.2/Gem/Requirement.html for supported methods. This is the simplest form I can think of.

Copy link
Member

Choose a reason for hiding this comment

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

I think that spec.required_rubygems_version.requirements is an Array not a Gem::Requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed this to

spec.required_rubygems_version = Gem::Requirement.new(gem_spec.required_rubygems_version, ">= 3.3.22")

@kou What you suggest doesn't work. requirements is an array, but Gem::Specification#required_rubygems_version= does not accept it as an argument:

spec = Gem::Specification.new do |s|
  s.name = 'my_gem'
  s.platform = Gem::Platform::RUBY
  s.extensions = ['ext/somegem/extconf.rb']
  s.required_rubygems_version = "!= 1.0.0"
end

spec.required_rubygems_version.requirements + [">= 3.3.22"]
# => [["!=", Gem::Version.new("1.0.0")], ">= 3.3.22"]

spec.required_rubygems_version = spec.required_rubygems_version.requirements + [">= 3.3.22"]
# /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:107:in `parse': Illformed requirement ["!="] (Gem::Requirement::BadRequirementError)
# 
#       raise BadRequirementError, "Illformed requirement [#{obj.inspect}]"
#             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:139:in `block in initialize'
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:139:in `map!'
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:139:in `initialize'
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:64:in `new'
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/requirement.rb:64:in `create'
# 	from /home/flavorjones/.rbenv/versions/3.2.3/lib/ruby/3.2.0/rubygems/specification.rb:702:in `required_rubygems_version='
# 	from /home/flavorjones/bar.rb:15:in `<main>'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that Gem::Requirement could be improved in two ways that would make this feature simpler to implement:

  1. #dup should make a deep copy of @requirements. It does not, and so I am calling Gem::Requirement.new(gem_spec.required_rubygems_version) to duplicate it.
  2. #concat could return self. It does not, and so I am doing this across two statements.

If those things were fixed, this could become

spec.required_rubygems_version = spec.required_rubygems_version.dup.concat(">= 3.3.22")

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I see.

spec/lib/rake/extensiontask_spec.rb Outdated Show resolved Hide resolved
spec/lib/rake/extensiontask_spec.rb Outdated Show resolved Hide resolved
spec/lib/rake/extensiontask_spec.rb Outdated Show resolved Hide resolved
spec/lib/rake/extensiontask_spec.rb Outdated Show resolved Hide resolved
Copy link

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

👍

@flavorjones flavorjones force-pushed the flavorjones-set-req-rubygems-version-for-linux-libc-platforms branch 2 times, most recently from 9d0a762 to 972ef3c Compare January 29, 2024 17:03
@flavorjones flavorjones requested a review from kou January 29, 2024 17:07
@flavorjones flavorjones force-pushed the flavorjones-set-req-rubygems-version-for-linux-libc-platforms branch 3 times, most recently from fa8097d to d695891 Compare January 30, 2024 03:05
If rake-compiler is building a linux native gem that specifies a libc
in its platform object, then we add ">= 3.3.22" to the
required_rubygems_version requirement.

Rubygems does not correctly recognize `-musl` or `-gnu` platform
suffixes until v3.3.22.

https://github.com/rubygems/rubygems/blob/master/CHANGELOG.md#3322--2022-09-07
@flavorjones flavorjones force-pushed the flavorjones-set-req-rubygems-version-for-linux-libc-platforms branch from d695891 to 9143b1f Compare January 30, 2024 03:07
@kou kou merged commit 109fff5 into rake-compiler:master Jan 30, 2024
16 checks passed
@kou
Copy link
Member

kou commented Jan 30, 2024

Thanks!

@flavorjones flavorjones deleted the flavorjones-set-req-rubygems-version-for-linux-libc-platforms branch January 30, 2024 03:27
flavorjones added a commit to flavorjones/rubygems that referenced this pull request Jan 30, 2024
to avoid accidentally mutating the original's state when doing:

```ruby
spec2 = spec.dup
spec2.required_rubygems_version.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
flavorjones added a commit to flavorjones/rubygems that referenced this pull request Jan 30, 2024
to avoid accidentally mutating the original's state when doing:

```ruby
req2 = req.dup
req2.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
@flavorjones
Copy link
Contributor Author

See rubygems/rubygems#7439 which proposes changing Gem::Requirement and Gem::Specification to deep-copy requirements.

flavorjones added a commit to flavorjones/rubygems that referenced this pull request Feb 2, 2024
to avoid accidentally mutating the original's state when doing:

```ruby
req2 = req.dup
req2.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
flavorjones added a commit to flavorjones/rubygems that referenced this pull request Feb 2, 2024
to avoid accidentally mutating the original's state when doing:

```ruby
spec2 = spec.dup
spec2.required_rubygems_version.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 2, 2024
…@requirements

to avoid accidentally mutating the original's state when doing:

```ruby
req2 = req.dup
req2.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.

rubygems/rubygems@8e0c03144e
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 2, 2024
…ies requirements

to avoid accidentally mutating the original's state when doing:

```ruby
spec2 = spec.dup
spec2.required_rubygems_version.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.

rubygems/rubygems@c1d52389f0
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Feb 7, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Feb 7, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Feb 25, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
flavorjones added a commit to flavorjones/ruby-c-extensions-explained that referenced this pull request Mar 1, 2024
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Mar 24, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Mar 24, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
flavorjones added a commit to sparklemotion/sqlite3-ruby that referenced this pull request Apr 15, 2024
Rubygems 3.3.22 is the minimum needed to correctly detect and use
`-linux-musl` and `-linux-gnu` native gems.

rake-compiler/rake-compiler#236 introduced a
minimum rubygems version for these native platform gems to provide a
sensible error message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants