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

Resolve warnings for gems that are no longer default gems in Ruby 3.3.5 #2012

Closed
wants to merge 6 commits into from

Conversation

KaanOzkan
Copy link
Contributor

@KaanOzkan KaanOzkan commented Sep 5, 2024

Motivation

Fix CI https://github.com/Shopify/tapioca/actions/runs/10711399821/job/29700012289#step:6:68

cli
  version
    it must display the version when passing -v                     FAIL (0.82s)
        ########## STDOUT ##########
        Tapioca v0.16.2
        
        ########## STDERR ##########
        /opt/hostedtoolcache/Ruby/3.3.5/x[64](https://github.com/Shopify/tapioca/actions/runs/10711399821/job/29700012289#step:6:65)/lib/ruby/3.3.0/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
        You can add ostruct to your Gemfile or gemspec to silence this warning.
        logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
        You can add logger to your Gemfile or gemspec to silence this warning.
        
        ########## STATUS: true ##########
        .
        Expected "/opt/hostedtoolcache/Ruby/3.3.5/x64/lib/ruby/3.3.0/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.\nYou can add ostruct to your Gemfile or gemspec to silence this warning.\nlogger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.\nYou can add logger to your Gemfile or gemspec to silence this warning.\n" to be empty.
        /home/runner/work/tapioca/tapioca/spec/spec_with_project.rb:125:in `assert_empty_stderr'
        /home/runner/work/tapioca/tapioca/spec/tapioca/cli/version_spec.rb:23:in `block (2 levels) in <class:VersionSpec>'

Implementation

Doesn't look like we directly depend on it so I added it to Gemfile instead of gemspec

Tests

@paracycle
Copy link
Member

I don't think this needs fixing, since we don't (and shouldn't) have any dependency on ostruct. This is an upstream bug: https://bugs.ruby-lang.org/issues/20713

The fix is to be on json >= 2.7.2

@amomchilov amomchilov self-assigned this Sep 11, 2024
@amomchilov amomchilov changed the title Resolve warnings from ostruct Resolve warnings for gems that are no longer default gems in Ruby 3.3.5 Sep 11, 2024
@@ -24,6 +24,7 @@ Gem::Specification.new do |spec|
spec.metadata["allowed_push_host"] = "https://rubygems.org"

spec.add_dependency("bundler", ">= 2.2.25")
spec.add_dependency("logger")
Copy link
Contributor

Choose a reason for hiding this comment

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

This fixes:

logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add logger to your Gemfile or gemspec to silence this warning.

Copy link
Member

Choose a reason for hiding this comment

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

Where does Tapioca have a dependency on the logger gem? I am not sure if this is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

The yard gem needs it, and won't explicitly require it. They'll move to something else eventually, but until then we need to add the explicit dependency ourselves.

lsegal/yard#1546 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

This has since been resolved. Yard has dropped the logger dependency in lsegal/yard@a589b9a and released 0.9.37

@@ -38,6 +38,7 @@ def tapioca_gemfile
source("https://rubygems.org")

gemspec name: "tapioca", path: "#{TAPIOCA_PATH}"
gem "json", ">= 2.7.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Added this to fix this warning which occurs under Ruby 3.3.5:

/opt/rubies/3.3.5/lib/ruby/3.3.0/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add ostruct to your Gemfile or gemspec to silence this warning.
logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add logger to your Gemfile or gemspec to silence this warning.

This warning doesn't happen for the Tapioca project itself (whose Gemfile.lock locks to json 2.7.2), but it does happen for these mock projects, which end up with json 2.7.1 by default.

Is there a nicer way to fix this? cc @paracycle

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 not isolate this to just the tests that end up loading ostruct? I don't think we need to do this for all the bundles that we create.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah that's a good idea, though it does add more "noise" for this temporary requirement, which will be scattered more widely through the codebase

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'd prefer merging this to unblock PRs and keep working on the ideal solution if it's going to take longer.

@amomchilov amomchilov force-pushed the ko/openstruct branch 2 times, most recently from b35c8d6 to 261d456 Compare September 11, 2024 18:41
Comment on lines -1025 to -1026
# Just so that we have something else we can generate for.
@project.require_real_gem("json")
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed json as an example here, as it added a new gem "json" line to the Gemfile, which conflicts with the gem "json", ">= 2.7.2" line that gets added to every MockProject.

bundler: failed to load command: tapioca (/opt/hostedtoolcache/Ruby/3.2.5/x64/lib/ruby/gems/3.2.0/bin/tapioca)
        /opt/hostedtoolcache/Ruby/3.2.5/x64/lib/ruby/gems/3.2.0/gems/bundler-2.5.11/lib/bundler/dsl.rb:130:in `gem':  (Bundler::Dsl::DSLError)
        [!] There was an error parsing `Gemfile`: You cannot specify the same gem twice with different version requirements.
        You specified: json (>= 2.7.2) and json (>= 0). Bundler cannot continue.

@egiurleo
Copy link
Contributor

Superseded by #2021

@egiurleo egiurleo closed this Sep 25, 2024
@egiurleo egiurleo deleted the ko/openstruct branch September 25, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants