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

Remove dependency on logger #1062

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

eregon
Copy link
Collaborator

@eregon eregon commented Sep 4, 2024

This should be fully compatible and address the warnings.
Only if one uses the long-deprecated Concurrent.create_stdlib_logger they might need to add logger to their Gemfile.

* To fix the Ruby 3.3.5 warnings:
  ruby-concurrency#1061
* concurrent-ruby only uses 7 constants from Logger, so just copy those over.
Copy link
Contributor

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

It looks okay to me, but I'd personally try to remove logging as a concern of this library.

@kbrock
Copy link
Contributor

kbrock commented Oct 7, 2024

@ioquatix What did you just ask?

  1. Can we drop the method that creates @logger It was introduced to prevent a deadlock and do we still need it? 1
  2. Can drop global logger and have log be a no-op if @logging does not exist? 2
  3. Can we drop all references to the log method? [3]
  4. Other.

@eregon eregon merged commit d7ce956 into ruby-concurrency:master Oct 7, 2024
14 checks passed
@ioquatix
Copy link
Contributor

ioquatix commented Oct 8, 2024

Probably (1) and (3) - basically remove it entirely.

naitoh added a commit to naitoh/doorkeeper that referenced this pull request Jan 16, 2025
> NameError:
>  uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger

See: ruby-concurrency/concurrent-ruby#1062
@Fryguy
Copy link

Fryguy commented Jan 16, 2025

The release of this seems to have broken Rails 7.0 codebases with

rake aborted!
NameError: uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError)

    Logger::Severity.constants.each do |severity|
    ^^^^^^
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:12:in `<module:LoggerThreadSafeLevel>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:9:in `<module:ActiveSupport>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:8:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_silence.rb:5:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger.rb:3:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support.rb:29:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/railties-7.0.8.7/lib/rails.rb:7:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/sprockets-rails-3.5.2/lib/sprockets/railtie.rb:1:in `<top (required)>'
/Users/jfrey/dev/manageiq-ui-classic/lib/manageiq/ui/classic/engine.rb:2:in `<top (required)>'

I think ActiveSupport::LoggerThreadSafeLevel happened to be depending on Logger implicitly, so the fix is likely to just update Rails to require 'logger' there. I'll send a PR over there for the fix. Workaround locally is to add the require early.

@Fryguy
Copy link

Fryguy commented Jan 16, 2025

See rails/rails#54264

@nytoan
Copy link

nytoan commented Jan 16, 2025

The release of this seems to have broken Rails 7.0 codebases with

rake aborted!
NameError: uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError)

    Logger::Severity.constants.each do |severity|
    ^^^^^^
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:12:in `<module:LoggerThreadSafeLevel>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:9:in `<module:ActiveSupport>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_thread_safe_level.rb:8:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger_silence.rb:5:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support/logger.rb:3:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/activesupport-7.0.8.7/lib/active_support.rb:29:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/railties-7.0.8.7/lib/rails.rb:7:in `<top (required)>'
/Users/jfrey/.gem/ruby/3.3.6/gems/sprockets-rails-3.5.2/lib/sprockets/railtie.rb:1:in `<top (required)>'
/Users/jfrey/dev/manageiq-ui-classic/lib/manageiq/ui/classic/engine.rb:2:in `<top (required)>'

I think ActiveSupport::LoggerThreadSafeLevel happened to be depending on Logger implicitly, so the fix is likely to just update Rails to require 'logger' there. I'll send a PR over there for the fix. Workaround locally is to add the require early.

same problem with cocoapods

naitoh added a commit to naitoh/doorkeeper that referenced this pull request Jan 17, 2025
## Ruby 3.4 is out
### 1. Add Bundled gems for Ruby 3.4

https://bugs.ruby-lang.org/issues/20187

It has already been backported to the 7-0-stable branch, but is not included in 7.0.8.7.
- https://github.com/rails/rails/blob/7-0-stable/activesupport/activesupport.gemspec#L41-L43
- https://github.com/rails/rails/blob/v7.0.8.7/activesupport/activesupport.gemspec

### 2. concurrent-ruby 1.3.5 don't work on Rails 7.0

> NameError:
>  uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger

See: ruby-concurrency/concurrent-ruby#1062
naitoh added a commit to naitoh/doorkeeper-openid_connect that referenced this pull request Jan 17, 2025
## Ruby 3.4 is out
### 1. concurrent-ruby 1.3.5 don't work on Rails 7.0

> NameError:
>  uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger

See: ruby-concurrency/concurrent-ruby#1062
naitoh added a commit to naitoh/doorkeeper-openid_connect that referenced this pull request Jan 17, 2025
## Ruby 3.4 is out
### 1. concurrent-ruby 1.3.5 don't work on Rails 7.0

> NameError:
>  uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger

See: ruby-concurrency/concurrent-ruby#1062
naitoh added a commit to naitoh/doorkeeper-openid_connect that referenced this pull request Jan 17, 2025
## Ruby 3.4 is out
### 1. concurrent-ruby 1.3.5 don't work on Rails 7.0

> NameError:
>  uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger

See: ruby-concurrency/concurrent-ruby#1062
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.

5 participants