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

Suppress warning for intentional circular require #9556

Merged
merged 1 commit into from Mar 1, 2022
Merged

Suppress warning for intentional circular require #9556

merged 1 commit into from Mar 1, 2022

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented Feb 27, 2022

If ruby was started in verbose mode, it prints this warning:

$ ruby -w -r google/protobuf -e ''
<internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85: warning: <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85: warning: loading in progress, circular require considered harmful - /usr/local/bundle/gems/google-protobuf-3.19.4/lib/google/protobuf.rb
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:149:in  `require'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:160:in  `rescue in require'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:160:in  `require'
        from /usr/local/bundle/gems/google-protobuf-3.19.4/lib/google/protobuf.rb:57:in  `<top (required)>'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'
        from /usr/local/bundle/gems/google-protobuf-3.19.4/lib/google/protobuf/descriptor_dsl.rb:5:in  `<top (required)>'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'
        from /usr/local/bundle/gems/google-protobuf-3.19.4/lib/google/protobuf/descriptor_pb.rb:4:in  `<top (required)>'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'
        from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:85:in  `require'

The circular require is:

protobuf.rb -> protobuf/descriptor_dsl.rb -> protobuf/descriptor_pb.rb -> protobuf.rb

It is very annoying that many ruby test framework (e.g. minitest) runs in ruby verbose mode by default and thus prints this message every time the test runs.

As far as I can see, this circular require is actually needed. It guarantees that if user requires any one of the three files, all three files will be loaded. Since we intentionally need this circular require, it's best to just suppress the warning here, rather than having the user dealing with figure out how to suppress this one warning with other warnings untouched.

@haberman
Copy link
Member

haberman commented Mar 1, 2022

Related: #9355

I think suppressing the warning makes sense until we can implement a more principled fix.

@haberman haberman merged commit 2a001f7 into protocolbuffers:master Mar 1, 2022
@ntkme ntkme deleted the ruby-suppress-warning branch March 1, 2022 23:25
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.

None yet

4 participants