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 logger to runtime dependency #1546

Closed
wants to merge 1 commit into from

Conversation

koic
Copy link
Contributor

@koic koic commented Jun 17, 2024

This PR adds logger to runtime dependency to suppress the following warning:

$ ruby -v
ruby 3.4.0dev (2024-06-14T03:14:32Z master 2677ab1607) [x86_64-darwin23]
$ cd path/to/rubocop
$ bundle exec rake
/Users/koic/.rbenv/versions/3.4-dev/lib/ruby/gems/3.4.0+0/gems/yard-0.9.36/lib/yard/logging.rb:3:
warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0.
Add logger to your Gemfile or gemspec.

This change is aimed at Ruby 3.5 and will start showing warnings from Ruby 3.4: ruby/ruby@d7e558e

To maintain the following behavior, the dependency will be added in the gemspec to resolve this. https://github.com/lsegal/yard/blob/v0.9.36/lib/yard/logging.rb#L3-L9

Description

Describe your pull request and problem statement here.

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

This PR adds `logger` to runtime dependency to suppress the following warning:

```console
$ ruby -v
ruby 3.4.0dev (2024-06-14T03:14:32Z master 2677ab1607) [x86_64-darwin23]
$ cd path/to/rubocop
$ bundle exec rake
/Users/koic/.rbenv/versions/3.4-dev/lib/ruby/gems/3.4.0+0/gems/yard-0.9.36/lib/yard/logging.rb:3:
warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0.
Add logger to your Gemfile or gemspec.
```

This change is aimed at Ruby 3.5 and will start showing warnings from Ruby 3.4:
ruby/ruby@d7e558e

To maintain the following behavior, the dependency will be added in the gemspec to resolve this.
https://github.com/lsegal/yard/blob/v0.9.36/lib/yard/logging.rb#L3-L9
koic added a commit to koic/rubocop that referenced this pull request Jun 17, 2024
This is a workaround to prevent the following warning in YARD:

```console
$ ruby -v
ruby 3.4.0dev (2024-06-14T03:14:32Z master 2677ab1607) [x86_64-darwin23]
$ cd path/to/rubocop
$ bundle exec rake
/Users/koic/.rbenv/versions/3.4-dev/lib/ruby/gems/3.4.0+0/gems/yard-0.9.36/lib/yard/logging.rb:3:
warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0.
Add logger to your Gemfile or gemspec.
```

The fundamental resolution will likely be addressed in lsegal/yard#1546.
koic added a commit to koic/rubocop that referenced this pull request Jun 17, 2024
This is a workaround to prevent the following warning in YARD:

```console
$ ruby -v
ruby 3.4.0dev (2024-06-14T03:14:32Z master 2677ab1607) [x86_64-darwin23]
$ cd path/to/rubocop
$ bundle exec rake
/Users/koic/.rbenv/versions/3.4-dev/lib/ruby/gems/3.4.0+0/gems/yard-0.9.36/lib/yard/logging.rb:3:
warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0.
Add logger to your Gemfile or gemspec.
```

The fundamental resolution will likely be addressed in lsegal/yard#1546.
bbatsov pushed a commit to rubocop/rubocop that referenced this pull request Jun 17, 2024
This is a workaround to prevent the following warning in YARD:

```console
$ ruby -v
ruby 3.4.0dev (2024-06-14T03:14:32Z master 2677ab1607) [x86_64-darwin23]
$ cd path/to/rubocop
$ bundle exec rake
/Users/koic/.rbenv/versions/3.4-dev/lib/ruby/gems/3.4.0+0/gems/yard-0.9.36/lib/yard/logging.rb:3:
warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0.
Add logger to your Gemfile or gemspec.
```

The fundamental resolution will likely be addressed in lsegal/yard#1546.
@lsegal
Copy link
Owner

lsegal commented Aug 26, 2024

@koic correct me if I'm wrong but this looks like it will have the side effect of pulling the logger dependency in on all Ruby versions even if it exists in the system Ruby, in other words, adding this dep will override the existing Ruby logger lib, even if the system Ruby provides it. I consider this a stability problem a blocker for this project, as it would be a breaking change for YARD to change its supported Ruby versions.

For example, a cursory glance at the logger package on RubyGems shows that it does only supports Ruby >= 2.5 when YARD still provides support for Ruby 2.4 and below. Furthermore, what happens in a year from now when the logger gem is updated and drops Ruby 2.5 support? How can we ensure that YARD stays compatible with the versions we already support? tl;dr we absolutely need to be loading the system logger if available.

Does the Ruby team have a proposed plan to address the backward compatibility problem of overriding the system library in the case where the 'logger' (or other gem) gem is actually available and preferred? This seems like a looming compatibility nightmare.

@Earlopain
Copy link

Earlopain commented Aug 31, 2024

Yes, users will recieve the latest logger gem available, the system library is not used anymore.

If I run Ruby 2.4 (or something lower even) this will still work and simply pull in the last version claiming support for my ruby. For 2.4, that would be logger 1.5.3 with >= 2.3.0. Still older logger versions claim no ruby incompatibilities and will as such install on even Ruby 1.8.

I don't think this has ever come up as a point for ruby devs. Personally, I don't think its much of a problem. The same thing has happened in the past starting with Ruby 3.0 for base64, mutex_m, webrick and many others and I'm not aware of this having caused any problems, appart from a bunch of churn through PRs like this.

That said, https://bugs.ruby-lang.org/issues/20309 is the place to discuss this. It also contains links to issues for previous gems/ruby versions.

@lsegal lsegal closed this in a589b9a Sep 3, 2024
@lsegal
Copy link
Owner

lsegal commented Sep 3, 2024

Unfortunately there are too many moving parts to pulling this in from RubyGems, and too much risk to upstream regressions with a logger library that's disconnected from the predictability of Ruby's release cycle. We chose logger originally because it was tied to Ruby's major version releases and thus provided some semblence of API stability and backward compatibility. Now there is a much greater risk of major version changes in logger causing failures that YARD itself would be responsible for fixing.

It's worth noting that YARD does rely pretty heavily on the Logger API internals and even monkeypatch some features, so we are fairly susceptible to breaking changes. YARD is heavily used as a CLI and as such, we don't have the luxury of relying on gem lockfiles or version pinning-- much of our usage comes from system-wide gem installs where version pinning simply is not feasible.

The good news is that because we've done a good job of isolating the API, we simply don't need this complexity, and the solution here is simply to rip out the now less-stable logger library. Given the monkey patches and heavy modification of overall usage, this is probably long overdue anyway.

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