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

Fix: NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname) #438

Merged
merged 1 commit into from Feb 8, 2018

Conversation

onk
Copy link
Contributor

@onk onk commented Feb 6, 2018

Hashie::Mash.load require Pathname since v3.4.5.

see: bbafade

irb(main):001:0> require "hashie"
=> true
irb(main):002:0> Hashie::Mash.load("foo.yml")
Traceback (most recent call last):
        6: from /opt/rubies/2.6.0-dev/bin/irb:11:in `<main>'
        5: from (irb):2
        4: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/mash.rb:105:in `load'
        3: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `perform'
        2: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `new'
        1: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:9:in `initialize'
NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname)
irb(main):003:0> require "pathname"
=> true
irb(main):004:0> Hashie::Mash.load("foo.yml")
=> #<Hashie::Mash bar="baz">

@dblock
Copy link
Member

dblock commented Feb 6, 2018

This needs a test, please. If you cannot repro in a regular spec, you can always make an integration one.

CHANGELOG.md Outdated
@@ -30,8 +30,10 @@ scheme are considered to be bugs.

* [#435](https://github.com/intridea/hashie/pull/435): Mash `default_proc`s are now propagated down to nested sub-Hashes - [@michaelherold](https://github.com/michaelherold).
* [#436](https://github.com/intridea/hashie/pull/436): Ensure that `Hashie::Extensions::IndifferentAccess` injects itself after a non-destructive merge - [@michaelherold](https://github.com/michaelherold).
* [#438](https://github.com/intridea/hashie/pull/438): Require `pathname` stdlib that required in `Hashie::Mash.load` - [@onk](https://github.com/onk).
Copy link
Member

@dblock dblock Feb 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should list the problem here, so "Fix: NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname) in Mashie::Mash.load" instead of saying what the fix was, to help people with the same problem know that it was fixed.

@onk
Copy link
Contributor Author

onk commented Feb 7, 2018

This problem cannot be reproduced using bundler.
https://github.com/bundler/bundler/blob/v1.16.1/lib/bundler.rb#L6

It can be reproduced with the code below, but I think this is too technical.

Object.send(:remove_const, :Pathname)
$LOADED_FEATURES.reject! { |f| f.include?('/pathname') }

require 'pathname' #=> true

I would like to skip the repro test. WDYT?

@onk onk changed the title require pathname stdlib Fix: NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname) Feb 7, 2018
@michaelherold
Copy link
Member

You're right: Bundler requires pathname, so it's not easy to write a failing test within our test harness. Here's a reproduction script that I came up with:

$LOAD_PATH.unshift('lib')

require 'tmpdir'
require 'hashie'

def within_temporary_directory
  Dir.mktmpdir.tap do |dir|
    Dir.chdir(dir) do
      yield if block_given?
    end
  end
end

within_temporary_directory do
  File.write('foo.yml', 'foo: bar')
  Hashie::Mash.load('foo.yml')
end

#=> /Users/michael/.gem/ruby/2.4.2/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:9:in `initialize': uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname (NameError)
#=>         from /Users/michael/.gem/ruby/2.4.2/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `new'
#=>         from /Users/michael/.gem/ruby/2.4.2/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `perform'
#=>         from /Users/michael/.gem/ruby/2.4.2/gems/hashie-3.5.7/lib/hashie/mash.rb:105:in `load'
#=>         from test.rb:16:in `block in <main>'
#=>         from test.rb:9:in `block (2 levels) in within_temporary_directory'
#=>         from test.rb:8:in `chdir'
#=>         from test.rb:8:in `block in within_temporary_directory'
#=>         from test.rb:7:in `tap'
#=>         from test.rb:7:in `within_temporary_directory'
#=>         from test.rb:14:in `<main>'

I think the fix is the correct one (albeit with a missing empty line between the require and the class definition, but that can be fixed during the merge). I also think I'm good with adding the change without a test given the nature of the bug. What do you think, @dblock?

@@ -1,5 +1,6 @@
require 'yaml'
require 'erb'
require 'pathname'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets put back an empty line after this?

@dblock
Copy link
Member

dblock commented Feb 7, 2018

OK, this looks good, rebase and lets merge? Thanks.

…amlErbParser::Pathname)

`Hashie::Mash.load` require `Pathname` since v3.4.5.

see: bbafade

```
irb(main):001:0> require "hashie"
=> true
irb(main):002:0> Hashie::Mash.load("foo.yml")
Traceback (most recent call last):
        6: from /opt/rubies/2.6.0-dev/bin/irb:11:in `<main>'
        5: from (irb):2
        4: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/mash.rb:105:in `load'
        3: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `perform'
        2: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:19:in `new'
        1: from /Users/onaka/.gem/ruby/2.6.0/gems/hashie-3.5.7/lib/hashie/extensions/parsers/yaml_erb_parser.rb:9:in `initialize'
NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname)
irb(main):003:0> require "pathname"
=> true
irb(main):004:0> Hashie::Mash.load("foo.yml")
=> #<Hashie::Mash bar="baz">
```
@onk
Copy link
Contributor Author

onk commented Feb 8, 2018

Fixed the blank line and rebased. Thanks.

@michaelherold michaelherold merged commit 02cbc08 into hashie:master Feb 8, 2018
@michaelherold
Copy link
Member

Thanks for the contribution!

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 23, 2018
## [3.6.0] - 2018-08-13

[3.6.0]: hashie/hashie@v3.5.7...v3.6.0

### Added

* [#455](hashie/hashie#455): Allow overriding methods when passing in a hash - [@lnestor](https://github.com/lnestor).

### Fixed

* [#435](hashie/hashie#435): Mash `default_proc`s are now propagated down to nested sub-Hashes - [@michaelherold](https://github.com/michaelherold).
* [#436](hashie/hashie#436): Ensure that `Hashie::Extensions::IndifferentAccess` injects itself after a non-destructive merge - [@michaelherold](https://github.com/michaelherold).
* [#437](hashie/hashie#437): Allow codependent properties to be set on Dash - [@michaelherold](https://github.com/michaelherold).
* [#438](hashie/hashie#438): Fix: `NameError (uninitialized constant Hashie::Extensions::Parsers::YamlErbParser::Pathname)` in `Hashie::Mash.load` - [@onk](https://github.com/onk).
* [#457](hashie/hashie#457): Fix `Trash` to allow it to copy properties from other properties - [@michaelherold](https://github.com/michaelherold).

### Miscellaneous

* [#433](hashie/hashie#433): Update Rubocop to the most recent version - [@michaelherold](https://github.com/michaelherold).
* [#434](hashie/hashie#434): Add documentation around Mash sub-Hashes - [@michaelherold](https://github.com/michaelherold).
* [#439](hashie/hashie#439): Add an integration spec for Elasticsearch - [@michaelherold](https://github.com/michaelherold).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants