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

Fix warning: Kernel#open is deprecated #264

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

mttkay
Copy link
Contributor

@mttkay mttkay commented Jul 6, 2021

Description

Opening URIs by redirecting through Kernel#open is deprecated as of Ruby 2.7 and is dropped in Ruby 3.0.

To unblock applications that use Ruby 3, we need to replace these calls with URI.open.

See also:

Related Issue

n/a

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • [-] I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 7, 2021

Build failure look unrelated:

STDERR: /opt/asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.4/lib/bundler/spec_set.rb:87:in `block in materialize': Could not find ffi-1.10.0 in any of the sources (Bundler::GemNotFound)

It is also happening in other MRs, e.g.: https://buildkite.com/chef-oss/chef-license-scout-master-verify/builds/140#f51d32ff-3efe-4342-9c6b-dc773e962cc4

@mttkay mttkay marked this pull request as ready for review July 7, 2021 07:47
@mttkay
Copy link
Contributor Author

mttkay commented Jul 7, 2021

@tduffield Could you or another maintainer do a code review and assist with fixing the build? Build failures do not look related to recent changes in pull requests.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 7, 2021

There are also 3 tests that fail locally after a fresh checkout of the project when on master:

$ bundle exec rspec
...
Finished in 5.1 seconds (files took 0.3487 seconds to load)
157 examples, 3 failures

Failed examples:

rspec ./spec/license_scout/dependency_manager/bundler_spec.rb:134 # LicenseScout::DependencyManager::Bundler#dependencies bundler 2.x project returns an array of Dependencies found in the directory
rspec ./spec/license_scout/dependency_manager/cargo_spec.rb:86 # LicenseScout::DependencyManager::Cargo#dependencies cargo project returns an array of Dependencies found in the directory
rspec ./spec/license_scout/dependency_manager/mix_spec.rb:80 # LicenseScout::DependencyManager::Mix#dependencies returns an array of Dependencies found in the directory

testoutput.log

Will break out a separate issue for this.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 7, 2021

Will break out a separate issue for this.

#265

@mttkay
Copy link
Contributor Author

mttkay commented Jul 9, 2021

@tduffield Could you or another maintainer do a code review and assist with fixing the build? Build failures do not look related to recent changes in pull requests.

Or @danielsdeleo maybe?

@tduffield
Copy link
Contributor

@mttkay Apologies for the delay; I'm out on PTO this week. I'll take a look at this first thing when I get back (next Tuesday).

@mttkay
Copy link
Contributor Author

mttkay commented Jul 10, 2021

@mttkay Apologies for the delay; I'm out on PTO this week. I'll take a look at this first thing when I get back (next Tuesday).

No worries! I happen to be out on PTO myself all of next week -- enjoy your time off and thanks for looking into it!

@tduffield
Copy link
Contributor

Okay, I've fixed the issue in the pipeline. You should be able to rebase on the latest master branch.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 14, 2021

Excellent, thanks! I am traveling all day today, but should be able to get to this some time tomorrow 👍

@mttkay mttkay force-pushed the mk-fix-ruby3-deprecation-warning branch from c758222 to 9c83bca Compare July 15, 2021 07:33
Opening URIs by redirecting through Kernel#open is
deprecated as of Ruby 2.7 and is dropped in Ruby 3.0.

To unblock applications that use Ruby 3, we need to
replace these calls with URI.open.

Cf.:
https://rubyreferences.github.io/rubychanges/2.7.html#network-and-web
ruby/open-uri@53862fa

Signed-off-by: Matthias Kaeppler <[email protected]>
@mttkay mttkay force-pushed the mk-fix-ruby3-deprecation-warning branch from 9c83bca to e61f685 Compare July 15, 2021 07:43
@mttkay
Copy link
Contributor Author

mttkay commented Jul 15, 2021

Thanks @tduffield, build is green now.

@tduffield tduffield merged commit c801d12 into chef:master Jul 15, 2021
@mttkay
Copy link
Contributor Author

mttkay commented Jul 21, 2021

Thank you @tduffield for including this in 2.6.2 and tagging a new release. I am not familiar with the release cycle of Chef, but I noticed that the latest version on rubygems.org is still 2.5.1: https://rubygems.org/gems/license_scout

To adopt the new version, would we have to wait for a new Chef release, or is it possible to deploy 2.6.2 of license_scout to rubygems.org independently?

@nkierpiec
Copy link

@mttkay the new ruby gem (2.6.2) exists now

@mttkay
Copy link
Contributor Author

mttkay commented Jul 22, 2021

Thanks @nkierpiec -- sorry to be a bother, but I only now realized that chef-omnibus actually pins license_scout to 1.x, so I cannot declare a dependency on 2.6.2. We're on omnibus 7, but even on master this is still pinned to 1.x+: https://github.com/chef/omnibus/blob/master/omnibus.gemspec#L31

Should this be ~> 2.0, since otherwise, no one will ever pull in the 2.x releases via a bundle update? 🤔

I'm not very familiar with the chef ecosystem of libraries and dependencies so I could be missing something.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 22, 2021

To illustrate:

Fetching https://dev.gitlab.org/gitlab/omnibus.git
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Bundler could not find compatible versions for gem "license_scout":
  In Gemfile:
    license_scout (~> 2.6.2)

    omnibus was resolved to 7.0.10, which depends on
      license_scout (~> 1.0)

@nkierpiec
Copy link

Hey, @mttkay, not a bother, I am going to have to see if Tom has more insight on this since he has worked more closely with this product. @tduffield is this something you can clarify for @mttkay? Thanks!

@tduffield
Copy link
Contributor

@mttkay There are changes required to Omnibus in order to have it support LicenseScout 2.x. If you're having issues with 1.x, you can open a PR against the 1-stable branch, which is our release branch for 1.x.

@mttkay
Copy link
Contributor Author

mttkay commented Jul 23, 2021

Thanks all. I consulted with a co-worker as well, and it looks like we use a fork of Omnibus 7.0 so I was able to test these changes against our own repo; however, just requiring the newer licsense_scout version did not work because I ran into dependency errors with mixlib-shellout between what chef, omnibus and license_scout think it should be (chef and omnibus require or allow for 3.x, while license_scout pins it to 2.2).

I am now testing if we can relax that pin here: #269

Not sure I'm venturing in the right direction with this but I hope this will fix the problem.

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.

3 participants