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

Do not change $LOAD_PATH in psych.gemspec #454

Closed
mame opened this issue Jun 8, 2020 · 4 comments
Closed

Do not change $LOAD_PATH in psych.gemspec #454

mame opened this issue Jun 8, 2020 · 4 comments

Comments

@mame
Copy link
Member

mame commented Jun 8, 2020

@deivid-rodriguez @tenderlove

The change for #433, which has been merged into ruby/ruby, produces dozens of warnings during make test-all in ruby master.

http://rubyci.s3.amazonaws.com/ubuntu2004/ruby-master/log/20200608T123004Z.log.html.gz

[15092/20234]
TestRDocOptions#test_init_with_encoding/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/ext/psych/lib/psych.rb:233: warning: already initialized constant Psych::LIBYAML_VERSION
/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/.ext/common/psych.rb:233: warning: previous definition of LIBYAML_VERSION was here
/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/ext/psych/lib/psych.rb:235: warning: already initialized constant Psych::NOT_GIVEN
/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/.ext/common/psych.rb:235: warning: previous definition of NOT_GIVEN was here
/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/ext/psych/lib/psych.rb:271: warning: method redefined; discarding old load
/home/chkbuild/chkbuild/tmp/build/20200608T123004Z/ruby/.ext/common/psych.rb:271: warning: previous definition of load was here
...

This is caused by double requiring ext/psych/lib/psych.rb and its copy file .ext/common/psych.rb.

A similar issue used to happen in fiddle. ruby/fiddle#34

I'm monitoring the number of warnings during test, so this is a bit noisy. Tentatively, I committed the same fix as fiddle directly into ruby master to make my monitoring easy. You can just import the fix if you like. If not, please fix it in your way. (I'm not particular about how to fix.)

Thanks!

@tenderlove
Copy link
Member

ah ya, that makes sense. I'll pull the fix from fiddle in to psych.

@deivid-rodriguez
Copy link
Contributor

I try to fix one redefinition warning and as a result, I create dozens of new instances of those 🤦. Thanks for fixing.

@tenderlove
Copy link
Member

@deivid-rodriguez lol, no problem! Don't worry about it! 😆

@tenderlove
Copy link
Member

I imported ruby/ruby@a3cc9b3 as 05d7e81, so this should be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants