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

Add bigdecimal to runtime dependency #49039

Merged
merged 1 commit into from Aug 25, 2023

Conversation

koic
Copy link
Contributor

@koic koic commented Aug 25, 2023

Motivation / Background

Follow up ruby/ruby@1c93288.

This PR adds bigdecimal to runtime dependency of Active Support to suppress the following Ruby 3.3.0dev's warning.

/Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/bundler/gems/rails-a8871e6829e5/activesupport/lib/
active_support/core_ext/object/json.rb:5: warning: bigdecimal will be not part of the default gems since Ruby 3.4.0.
Add bigdecimal to your Gemfile. Also contact author of to add bigdecimal into its gemspec.

The grep yields the following results:

$ git grep 'require.*bigdecimal'
activejob/lib/active_job/arguments.rb:3:require "bigdecimal"
activejob/lib/active_job/serializers/big_decimal_serializer.rb:3:require "bigdecimal"
activejob/test/cases/argument_serialization_test.rb:3:require "bigdecimal"
activemodel/lib/active_model/type/decimal.rb:3:require "bigdecimal/util"
activemodel/lib/active_model/validations/numericality.rb:5:require "bigdecimal/util"
activemodel/test/cases/validations/numericality_validation_test.rb:8:require "bigdecimal"
activerecord/test/cases/adapters/sqlite3/quoting_test.rb:4:require "bigdecimal"
activerecord/test/cases/arel/visitors/to_sql_test.rb:4:require "bigdecimal"
activerecord/test/cases/migration_test.rb:5:require "bigdecimal/util"
activesupport/lib/active_support/core_ext/big_decimal/conversions.rb:3:require "bigdecimal"
activesupport/lib/active_support/core_ext/big_decimal/conversions.rb:4:require "bigdecimal/util"
activesupport/lib/active_support/core_ext/object/json.rb:5:require "bigdecimal"
activesupport/lib/active_support/message_pack/extensions.rb:3:require "bigdecimal"
activesupport/lib/active_support/xml_mini.rb:5:require "bigdecimal"
activesupport/lib/active_support/xml_mini.rb:6:require "bigdecimal/util"
activesupport/test/core_ext/hash_ext_test.rb:4:require "bigdecimal"
activesupport/test/core_ext/object/duplicable_test.rb:4:require "bigdecimal"
activesupport/test/hash_with_indifferent_access_test.rb:4:require "bigdecimal"
activesupport/test/json/encoding_test_cases.rb:3:require "bigdecimal"

By adding only to Active Support as a dependency, it should resolve the issue due to the dependency.

Detail

The warning is confirmed in the following step:

$ cat generic_main.rb
# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
end

require 'active_support'
require 'minitest/autorun'
# These gems will be bundled gems in Ruby 3.4
require 'bigdecimal'

Run generic_main.rb with Ruby 3.3.0dev below.

$ ruby -v
ruby 3.3.0dev (2023-08-25T17:47:04Z master 7d32011399) [x86_64-darwin22]

$ ruby generic_main.rb
Fetching gem metadata from https://rubygems.org/.......
Resolving dependencies...
/Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/bundler/gems/rails-a8871e6829e5/activesupport/lib/
active_support/core_ext/object/json.rb:5: warning: bigdecimal will be not part of the default gems since Ruby 3.4.0.
Add bigdecimal to your Gemfile. Also contact author of  to add bigdecimal into its gemspec.
Run options: --seed 39015

# Running:

Finished in 0.001313s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips

Additional information

It is essentially the same as #48907.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

## Motivation / Background

Follow up ruby/ruby@1c93288.

This PR adds bigdecimal to runtime dependency of Active Support to suppress the following Ruby 3.3.0dev's warning.

> /Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/bundler/gems/rails-a8871e6829e5/activesupport/lib/
> active_support/core_ext/object/json.rb:5: warning: bigdecimal will be not part of the default gems since Ruby 3.4.0.
> Add bigdecimal to your Gemfile. Also contact author of  to add bigdecimal into its gemspec.

The grep yields the following results:

```console
$ git grep 'require.*bigdecimal'
activejob/lib/active_job/arguments.rb:3:require "bigdecimal"
activejob/lib/active_job/serializers/big_decimal_serializer.rb:3:require "bigdecimal"
activejob/test/cases/argument_serialization_test.rb:3:require "bigdecimal"
activemodel/lib/active_model/type/decimal.rb:3:require "bigdecimal/util"
activemodel/lib/active_model/validations/numericality.rb:5:require "bigdecimal/util"
activemodel/test/cases/validations/numericality_validation_test.rb:8:require "bigdecimal"
activerecord/test/cases/adapters/sqlite3/quoting_test.rb:4:require "bigdecimal"
activerecord/test/cases/arel/visitors/to_sql_test.rb:4:require "bigdecimal"
activerecord/test/cases/migration_test.rb:5:require "bigdecimal/util"
activesupport/lib/active_support/core_ext/big_decimal/conversions.rb:3:require "bigdecimal"
activesupport/lib/active_support/core_ext/big_decimal/conversions.rb:4:require "bigdecimal/util"
activesupport/lib/active_support/core_ext/object/json.rb:5:require "bigdecimal"
activesupport/lib/active_support/message_pack/extensions.rb:3:require "bigdecimal"
activesupport/lib/active_support/xml_mini.rb:5:require "bigdecimal"
activesupport/lib/active_support/xml_mini.rb:6:require "bigdecimal/util"
activesupport/test/core_ext/hash_ext_test.rb:4:require "bigdecimal"
activesupport/test/core_ext/object/duplicable_test.rb:4:require "bigdecimal"
activesupport/test/hash_with_indifferent_access_test.rb:4:require "bigdecimal"
activesupport/test/json/encoding_test_cases.rb:3:require "bigdecimal"
```

By adding only to Active Support as a dependency, it should resolve the issue due to the dependency.

## Detail

The warning is confirmed in the following step:

```ruby
$ cat generic_main.rb
# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
end

require 'active_support'
require 'minitest/autorun'
# These gems will be bundled gems in Ruby 3.4
require 'bigdecimal'
```

Run generic_main.rb with Ruby 3.3.0dev below.

```console
$ ruby -v
ruby 3.3.0dev (2023-08-25T17:47:04Z master 7d32011399) [x86_64-darwin22]

$ ruby generic_main.rb
Fetching gem metadata from https://rubygems.org/.......
Resolving dependencies...
/Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/gems/3.3.0+0/bundler/gems/rails-a8871e6829e5/activesupport/lib/
active_support/core_ext/object/json.rb:5: warning: bigdecimal will be not part of the default gems since Ruby 3.4.0.
Add bigdecimal to your Gemfile. Also contact author of  to add bigdecimal into its gemspec.
Run options: --seed 39015

# Running:

Finished in 0.001313s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips
```

## Additional information

It is essentially the same as rails#48907.
@koic koic force-pushed the add_bigdecimal_to_runtime_dependency branch from a0729bc to a77535c Compare August 25, 2023 18:40
@rafaelfranca rafaelfranca merged commit fb48025 into rails:main Aug 25, 2023
4 checks passed
@koic koic deleted the add_bigdecimal_to_runtime_dependency branch August 25, 2023 22:21
yahonda added a commit to yahonda/rails that referenced this pull request Sep 5, 2023
To prepare Ruby 3.4 will ship `bigdecimal` as bundled gem,
`bigdecimal` has been added to Active Support runtime dependency
via rails#49039

It will install BigDecimal version 3.1.4 or higher,
so we just check the BigDecimal 3.1+ behavior at `NumericalityValidationTest`.

Refer to
rails#49039
ruby/bigdecimal#180
ruby/bigdecimal#70
yahonda added a commit to yahonda/rails that referenced this pull request Sep 19, 2023
Follow up rails#49152
rails#49039

- `bigdecimal_to_d.rb`
```
require 'bigdecimal/util'
p "Ruby version: #{RUBY_VERSION}"
p "BigDecimal version: #{BigDecimal::VERSION}"
p "123.455.to_d(5): #{123.455.to_d(5)}"
```

- Ruby 2.7 and 3.0 rounds down
```
$ ruby bigdecimal_to_d.rb
"Ruby version: 2.7.8"
"BigDecimal version: 2.0.0"
"123.455.to_d(5): 0.12345e3"
```

```
$ ruby bigdecimal_to_d.rb
"Ruby version: 3.0.6"
"BigDecimal version: 3.0.0"
"123.455.to_d(5): 0.12345e3"
```

- Ruby 3.1 and 3.2 rounds up

```
$ ruby bigdecimal_to_d.rb
"Ruby version: 3.1.4"
"BigDecimal version: 3.1.1"
"123.455.to_d(5): 0.12346e3"
```

```
$ ruby bigdecimal_to_d.rb
"Ruby version: 3.2.2"
"BigDecimal version: 3.1.3"
"123.455.to_d(5): 0.12346e3"
```
@zzak
Copy link
Member

zzak commented Dec 10, 2023

FYI: This error started re-appearing in CI on rubylang/ruby:master-nightly-jammy (3.3.0dev) builds:
https://buildkite.com/rails/rails/builds/102659#018c5108-8fcf-4cbb-b7d5-0986156576dd/1100-1105

@Earlopain Earlopain mentioned this pull request Jan 2, 2024
@Earlopain
Copy link
Contributor

Should this be backported to 7.0?

FYI: This error started re-appearing in CI on rubylang/ruby:master-nightly-jammy (3.3.0dev) builds:
https://buildkite.com/rails/rails/builds/102659#018c5108-8fcf-4cbb-b7d5-0986156576dd/1100-1105

I'd bet on this ruby/ruby@83bdf12, though I haven't checked if this is good now with a proper 3.3 release.

skipkayhil pushed a commit to skipkayhil/rails that referenced this pull request Jan 2, 2024
…pendency

Add bigdecimal to runtime dependency
skipkayhil pushed a commit to skipkayhil/rails that referenced this pull request Jan 3, 2024
…pendency

Add bigdecimal to runtime dependency
skipkayhil pushed a commit to skipkayhil/rails that referenced this pull request Jan 3, 2024
…pendency

Add bigdecimal to runtime dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants