Skip to content

Suppress warning for intentional circular require#9556

Merged
haberman merged 1 commit intoprotocolbuffers:masterfrom
ntkme:ruby-suppress-warning
Mar 1, 2022
Merged

Suppress warning for intentional circular require#9556
haberman merged 1 commit intoprotocolbuffers:masterfrom
ntkme:ruby-suppress-warning

Conversation

@ntkme
Copy link
Copy Markdown
Contributor

@ntkme ntkme commented Feb 27, 2022 •
edited
Loading

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.

@acozzette acozzette requested a review from haberman February 27, 2022 20:12
@haberman
Copy link
Copy Markdown
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

Reviewers

@haberman haberman Awaiting requested review from haberman

Assignees

No one assigned

Labels

ruby

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ntkme @haberman @acozzette @protobuf-kokoro