-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
require reloads file, not honoring partial path in LOADED_FEATURES #2477
Comments
This would probably be fixed if we did the same porting required for #2510. MRI has extensive logic during load to look for loaded features with no extension, with and without various load path entries, and so on. We have none of that logic in either 1.7 or 9k. |
Due to some issues in the way that Puppet loads code, and some differences in how that works in JRuby vs MRI Ruby, doing a `require` on the `puppet/type/file` code multiple times in a single JRuby interpreter will cause problems. Since this code automatically gets loaded by Puppet before any custom type/provider is loaded, the `require` statement is unnecessary. This is related to the following Puppet Jira tickets: https://tickets.puppetlabs.com/browse/SERVER-64 https://tickets.puppetlabs.com/browse/SERVER-40 and also to the following puppet-logstash PR: voxpupuli/puppet-logstash#196 Hopefully there will be a more permanent fix in JRuby at some point: jruby/jruby#2477 But in the meantime, this simple workaround makes `file_concat` compatible with Puppet Server.
Note that #2510 has been fixed for 1.7.20 and 9k.pre2, but this is still an outstanding issue because we don't have the same heuristic for managing load path. |
This is not going to get fixed in the 1.7 line. There's a separate issue for 9k to implement the path caching logic from MRI, which would include this partial-path logic. |
This is related to a discussion in #1941.
In MRI Ruby 1.9, if a partial (relative?) path to a file is included in LOADED_FEATURES, an argument to 'require' matching that path will not result in the file being reloaded. In JRuby 1.7.18 and earlier, however, the same 'require' statement would cause the file to be reloaded.
For example, if "test/testmod.rb" were added to the LOADED_FEATURES, a subsequent "require 'test/testmod'" statement on MRI Ruby 1.9 would not reload the "test/testmod.rb" file whereas the same require statement would reload the "test/testmod.rb" file on JRuby 1.7.18.
Here are a couple of test Ruby files I used to observe this:
main.rb
test/testmod.rb
On MRI Ruby 1.9.3, I ran "ruby -I_base_dir_ main.rb" and saw the following output:
On JRuby 1.7.18, I ran "jruby -I_base_dir_ main.rb" and saw the following output:
This issue affects Puppet's puppet_server project - https://github.com/puppetlabs/puppet-server. In Puppet Ruby code, there is some logic which manipulates LOADED_FEATURES directly in some cases where Puppet needs to manually manage the load process. For example, see https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/autoload.rb#L32-L37. The differences in MRI Ruby vs. JRuby interpretation of LOADED_FEATURES when a "require" is performed causes code to be reloaded unexpectedly. For cases where the loaded code is not idempotent - e.g., in various places where Puppet uses metaprogramming - the reload can lead to bad side effects like previously defined symbols later being undefined, etc. For example, see https://tickets.puppetlabs.com/browse/SERVER-40.
To more closely match MRI Ruby behavior, I'm thinking that the logic behind 'require' in JRuby would need to insert each entry into LOADED_FEATURES using the partial path loaded - e.g., 'test/testmod.rb' instead of '/full/path/to/test/testmod.rb' for a require of 'test/testmod' - and also match up the path supplied to each require against the partial path to see if the file has already been loaded - i.e., if a second "require 'test/testmod'" were encountered and "test/testmod.rb" were already in LOADED_FEATURES, the "test/testmod.rb" file would not be reloaded.
Hope this makes sense. Please let me know if there are more specific examples that would be helpful to demonstrate this issue.
The text was updated successfully, but these errors were encountered: