Warnings even if the right json version is available

[aerostitch]

Hi guys,

I'm hitting this warning:

WARNING: jmespath gem requires json gem >= 1.8.1; json 1.8.0 already loaded

Even if the right version of json/json_pure is available:

# gem list

*** LOCAL GEMS ***

aws-sdk (2.2.37)
aws-sdk-core (2.2.37)
aws-sdk-resources (2.2.37)
jmespath (1.2.4)
json (1.8.3)
json_pure (1.8.3)
root@maxscale01:~#

Current ruby version:

# ruby --version
ruby 1.9.3p484 (2013-11-22 revision 43786) [x86_64-linux]

Which comes from https://github.com/jmespath/jmespath.rb/blob/master/lib/jmespath.rb

Doing some testing I see that if I don't specify any gem, it shows a json version 1.8.0:

# irb
irb(main):001:0> require 'json'
=> true
irb(main):002:0> Object.const_defined?(:JSON)
=> true
irb(main):003:0> JSON::VERSION < '1.8.1'
=> true
irb(main):004:0> JSON::VERSION
=> "1.8.0"

The thing is that if I load json_pure, I get the right JSON::VERSION:

# irb
irb(main):001:0> gem('json_pure', '>= 1.8.1')
=> true
irb(main):002:0> require 'json'
=> true
irb(main):003:0> JSON::VERSION
=> "1.8.3"
irb(main):004:0>

And if I call for the json gem directly I also have the right version:

# irb
irb(main):001:0> gem('json', '>= 1.8.1')
=> true
irb(main):002:0> require 'json'
=> true
irb(main):003:0> JSON::VERSION
=> "1.8.3"
irb(main):004:0>

Shouldn't we try to load the json gem, fallback to json_pure and warn if both gem loads fail?

I can push a PR for that if you want, should be a quite easy fix, but I'd like your opinion 1st. :)

Thanks
Joseph

[aerostitch]

Proposed a patch in #28

[trevorrowe]

The JSON::VERSION check up front is intentional. In Ruby 1.9.3, it does not matter if you have json version 1.8.1+ installed. Calling a blind require 'json' will actually load an older version of json. This means if a user has the following code in Ruby 1.9.3:

require 'json'
require 'jmespath'

Then they will likely get a warning. Can you try adding the following to your Gemfile to see if this fixes the issue?

gem 'json', '>= 1.8.1'

[aerostitch]

Hi Trevor,

No I can't I'm using this in a script (via calls to the aws_sdk) to set a puppet fact.
I don't understand what's wrong with the PR. Can you develop please?
It was working on the different environments I tested it with and actually fixes the problem I explained in the description of this issue.

As I explained above I already have the json gen installed in the right version. Did you try to reproduce it?
I don't call a blind require 'json' in the PR, I call it by first trying to load the json jem in the right version (using gem('json', '>= 1.8.1') like you originally did) and fall back to json_pure if it cannot load and then give the warning if this cannot load either. What's wrong with that logic?

Thanks.

[trevorrowe]

With Ruby 1.9.3-p551 and the following gems installed:

json (1.8.3, 1.8.0)
json_pure (1.8.3, 1.8.0)

I ran the following code:

require 'json'
JSON::VERSION
#=> 1.5.5

Now, in the same process, I ran the code from the pull request:

def fallback_gem
  begin
    # Fallback on the json_pure gem dependency.
    gem('json_pure', '>= 1.8.1')
    require 'json'
  rescue Gem::LoadError
    if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
      warn("WARNING: jmespath gem requires json gem >= 1.8.1; json #{JSON::VERSION} already loaded")
    else
      warn("ERROR: unable to load any json library")
    end
  end
end

begin
  # Attempt to load the native version if available, not availble
  # by default for Ruby 1.9.3.
  gem('json', '>= 1.8.1')
  require 'json'
rescue Gem::LoadError
  fallback_gem
end

JSON::VERSION
#=> 1.5.5

The code fails to produce the desired warning because the code in the first (bottom) begin rescue block will not raise. It simply returns false because require 'json' has already happened. The result is version 1.5.5 JSON is loaded and no warning is emitted. This is not desired.

Also, care is required to ensure that you do not call require 'json/ext' or require 'json/pure' afterwards or things start to break.

[aerostitch]

Hi @trevorrowe,

Using a freshly installed 1.9.3 rvm environment:

$ ruby --version
ruby 1.9.3p551 (2014-11-13) [x86_64-linux]

I did 4 tests (and a code update proposition after that):

Content of the script used it's exactly the same version as you, just printing the last line:

def fallback_gem
  begin
    # Fallback on the json_pure gem dependency.
    gem('json_pure', '>= 1.8.1')
    require 'json'
  rescue Gem::LoadError
    if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
      warn("WARNING: jmespath gem requires json gem >= 1.8.1; json #{JSON::VERSION} already loaded")
    else
      warn("ERROR: unable to load any json library")
    end
  end
end

begin
  # Attempt to load the native version if available, not availble
  # by default for Ruby 1.9.3.
  gem('json', '>= 1.8.1')
  require 'json'
rescue Gem::LoadError
  fallback_gem
end

puts "Final version of the gem: #{JSON::VERSION}"

Test 1: No Json gem with the right version (only the 1.5.5 that comes with ruby) and no json_pure either

The script cannot find the JSON constant, so the gem 1.5.5 is not loaded.

$ gem list

*** LOCAL GEMS ***

bigdecimal (1.1.0)
bundler-unload (1.0.2)
executable-hooks (1.3.2)
gem-wrappers (1.2.7)
io-console (0.3)
json (1.5.5)
minitest (2.5.1)
rake (0.9.2.2)
rdoc (3.9.5)
rubygems-bundler (1.4.4)
rvm (1.11.3.9)
$ ruby test.rb
ERROR: unable to load any json library
test.rb:32:in `<main>': uninitialized constant JSON (NameError)
$

Test 2: with json_pure 1.8.1 installed

The script loads the json_pure gem:

$ gem install json_pure -v 1.8.1
Fetching: json_pure-1.8.1.gem (100%)
Successfully installed json_pure-1.8.1
Installing ri documentation for json_pure-1.8.1
1 gem installed
$ gem list

*** LOCAL GEMS ***

bigdecimal (1.1.0)
bundler-unload (1.0.2)
executable-hooks (1.3.2)
gem-wrappers (1.2.7)
io-console (0.3)
json (1.5.5)
json_pure (1.8.1)
minitest (2.5.1)
rake (0.9.2.2)
rdoc (3.9.5)
rubygems-bundler (1.4.4)
rvm (1.11.3.9)
$ ruby test.rb
Final version of the gem: 1.8.1
$

Test 3: with json = 1.8.2

$ gem install json -v 1.8.2
Fetching: json-1.8.2.gem (100%)
Building native extensions.  This could take a while...
Successfully installed json-1.8.2
Installing ri documentation for json-1.8.2
1 gem installed
$ gem list

*** LOCAL GEMS ***

bigdecimal (1.1.0)
bundler-unload (1.0.2)
executable-hooks (1.3.2)
gem-wrappers (1.2.7)
io-console (0.3)
json (1.8.2, 1.5.5)
json_pure (1.8.1)
minitest (2.5.1)
rake (0.9.2.2)
rdoc (3.9.5)
rubygems-bundler (1.4.4)
rvm (1.11.3.9)
$ ruby test.rb
Final version of the gem: 1.8.2
$

Test 4: with a require json done for an older gem version 1st

Now, if in my script I load the gem 1.5.5 first, yes, I get a bad behavior. Is that the test case you are talking about?

Adding those 2 lines at the top of the script:

require 'json'
puts "Initially loaded version of gem: #{JSON::VERSION}"

Gives me indeed the result you were talking about:

$ ruby test.rb 
Initially loaded version of gem: 1.5.5
Final version of the gem: 1.5.5
$

Code update

But that's easily fixable. I just need to add the following code to unload the gem and corresponding constant if the module has been loaded in an older version (and add "in the right version >= 1.8.1 required" to the error message):

  if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
    if Object.const_defined?("JSON")
      Object.send(:remove_const, "JSON")
    end
    $".delete_if {|s| s.include?('json') }
  end

Which would give us something like:

def fallback_gem
  begin
    # Fallback on the json_pure gem dependency.
    gem('json_pure', '>= 1.8.1')
    require 'json'
  rescue Gem::LoadError
    if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
      warn("WARNING: jmespath gem requires json gem >= 1.8.1; json #{JSON::VERSION} already loaded")
    else
      warn("ERROR: unable to load any json library in the right version >= 1.8.1 required")
    end
  end
end

begin
  # Unload the json gem if already loaded in an older version
  if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
    if Object.const_defined?("JSON")
      Object.send(:remove_const, "JSON")
    end
    $".delete_if {|s| s.include?('json') }
  end
  # Attempt to load the native version if available, not availble
  # by default for Ruby 1.9.3.
  gem('json', '>= 1.8.1')
  require 'json'
rescue Gem::LoadError
  fallback_gem
end

Would you be ok with this behavior and code?
If so, can the PR be reopened so that I add this chunk of code to it?

Note that your gemspec requires json_pure so if the jmespath gem is installed correctly, you should at least have json_pure available in the right version.

Thanks
Joseph

[trevorrowe]

Yes, the scenario I'm concerned about is when JSON is loaded prior to jmespath being loaded. I'm not sure how I feel about removing a loaded version of JSON. I would prefer to avoid such tactics. These things tend to alway go bad in the end.

What issue do you have with the current behavior? Is it the warning that you concerned with? The warning can be improved with guidance on how to resolve this.

[aerostitch]

My issue is that now, when I'm running puppet, it loads custom facts that use the aws-sdk gem (which calls jmespath). Somewhere in the facts or in a 3rd party library, the require 'json' is called without caring about the version.
Because of that we now have warnings showing up every time we run puppet. Warnings we cannot do anything about it as it comes from one of the latest version of this lib. All the requirements are in place. I'm just trying to make sure the lib uses what is available properly as everything it requires is already there.

I don't see any other way than unloading the lib and reloading it with the right version. As I was saying earlier, the json_pure >= 1.8.1 is a prerequisite of this gem anyway, so it should be at least available.
There is really little chances that the require 'json' done previously will not work in the new version (If it's the case the lib in question should probably update).

[trevorrowe]

Closed by #35. We've removed the dependency on json_pure, and have added some additional code to deal with the old Ruby 1.9.3 & JSON 1.5.5 differences.