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 unnecessary check on version #433

Merged

Conversation

deivid-rodriguez
Copy link
Contributor

I had a look at the warnings being fixed in #390 to try to come up with a better solution.

Initially, I was able to reproduce the issue by running bundle from the root of this repo. The issue happened because my jruby installation had a psych installation at: ~/.rbenv/versions/jruby-9.2.9.0/lib/ruby/gems/shared/gems, and that specific installation did not have the version guard in versions.rb that this repository included. So, when running bundle first the local gemspec would be loaded which loads the versions.rb file in the repository relatively, then bundler would load the "system" version of psych without the version guard, causing the redefinition warnings.

The changes in this PR fix the issue because the local version of psych is always loaded.

However, I cannot reproduce on a pristine installation of jruby, because psych is not included in the ~/.rbenv/versions/jruby-9.2.9.0/lib/ruby/gems/shared/gems, so now I'm not sure how to reproduce this anymore... :/

@deivid-rodriguez
Copy link
Contributor Author

Just for completeness, the warning can be reproduced by running bundle install under jruby on a checkout of this repo without a Gemfile.lock file, and while having the same version of psych also globally installed.

Removing it triggers the following warnings when running `bundle` under
jruby from the root of the `psych` repo prints the following warnings:

```
/path/to/jruby-9.2.9.0/lib/ruby/gems/shared/gems/psych-3.1.0-java/lib/psych/versions.rb:7: warning: already initialized constant VERSION
/path/to//jruby-9.2.9.0/lib/ruby/gems/shared/gems/psych-3.1.0-java/lib/psych/versions.rb:10: warning: already initialized constant DEFAULT_SNAKEYAML_VERSION
```

This is because bundler loads the versions file relatively from the
local gemspec, and then internally loads the psych gem, causing the
redefinition warnings.

Instead, we modify the $LOAD_PATH so that when working locally on the
`psych` repo, the local version of `psych` gets used.
@deivid-rodriguez deivid-rodriguez force-pushed the remove_unnecessary_check_on_version branch from 6f8bbf8 to a3fc819 Compare March 17, 2020 20:27
@tenderlove tenderlove merged commit d2deaa9 into ruby:master May 8, 2020
@deivid-rodriguez deivid-rodriguez deleted the remove_unnecessary_check_on_version branch May 8, 2020 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants