-
-
Notifications
You must be signed in to change notification settings - Fork 399
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
Conversation
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
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.
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.
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 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. |
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 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 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. |
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. |
This PR adds
logger
to runtime dependency to suppress the following warning: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
bundle exec rake
locally (if code is attached to PR).