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

Figure out what to do when different versions of json and json_pure are loaded #650

Open
byroot opened this issue Oct 25, 2024 · 7 comments
Milestone

Comments

@byroot
Copy link
Member

byroot commented Oct 25, 2024

Ref: #646

There is no version constraints between the two, but they expose the same files in $LOAD_PATH, so if two different versions are loaded it can easily cause incompatibilities.

We should find a solution for this. Perhaps use require_relative? But even then, large parts of the implementation are in the global JSON module, so we can't load two namespaced versions of it.

@byroot byroot added this to the 2.8 milestone Oct 25, 2024
@headius
Copy link
Contributor

headius commented Oct 25, 2024

Normally I'd say the common parts should be in a separate gem json_base that both json and json_pure depend upon (with a strict version). If they tried to activate different versions it would be an error earlier in library management, and even better it would not silently appear to load ok and then break.

However it would mean another gem in stdlib.

The only other alternative is shipping the common parts namespaced too.

@Earlopain
Copy link

Earlopain commented Oct 25, 2024

Since this is an unsupported use-case, can they both define a version constant somewhere independent from each other?

begin
  require "json/pure/from_gem/version"
  require "json/ext/version"
  if JSON::Ext::VERSION != JSON::Pure::FromGem::VERSION
    raise "Some descriptive error"
  end
rescue LoadError
end

This leaves people with older versions out of luck but maybe its acceptable? OTOH, loading different versions probably has just worked for a long time.

byroot added a commit to byroot/json that referenced this issue Oct 25, 2024
Prior to 2.7.3, `JSON::Ext::Parser` would only take kwargs.
So if json_pure 2.7.4 is loaded with `json <= 2.7.2` (or stdlib)
it blows up.

Ref: ruby#650
Fix: ruby#651
byroot added a commit that referenced this issue Oct 25, 2024
Prior to 2.7.3, `JSON::Ext::Parser` would only take kwargs.
So if json_pure 2.7.4 is loaded with `json <= 2.7.2` (or stdlib)
it blows up.

Ref: #650
Fix: #651
@byroot
Copy link
Member Author

byroot commented Oct 25, 2024

Yet another such issue #652

if JSON::Ext::VERSION != JSON::Pure::FromGem::VERSION

Yeah, this is kinda the obvious / easy thing to do. But I wonder if it's really what should be done.

I also wonder if the solution isn't simply to deprecate and remove json_pure.

After all, now that json is in the stdlib, why would you need json_pure specifically?

@byroot
Copy link
Member Author

byroot commented Oct 25, 2024

When I have some time, I'd like to go over look more into https://rubygems.org/gems/json_pure/reverse_dependencies, because at first sight I don't see a lot of legitimate use.

So I think we could just push a json_pure that's just a gemspec that require json. The Pure implementation would remain inside json but we'd enforce that only one implementation is ever loaded (e.g. use the pure generator on truffle, etc).

@headius
Copy link
Contributor

headius commented Oct 25, 2024

why would you need json_pure specifically?

Didn't we just have an issue where someone wanted to use json_pure because they deployed on a system without build tools?

@byroot
Copy link
Member Author

byroot commented Oct 25, 2024

It was resolved by the fact that json is part of the stdlib, so there's no need to build it.

@eregon
Copy link
Member

eregon commented Oct 25, 2024

It was resolved by the fact that json is part of the stdlib, so there's no need to build it.

Mmh, but that requires to use the same version of json as in stdlib (which might not be an option if some bug fix or change is wanted) and recent Bundler always reinstalls default gems (json is a default gem): rubygems/rubygems#8180

Maybe we could have some documented env var (or a extconf.rb flag, should also work fine) to allow skipping the C extension when installing the json gem?

hsbt pushed a commit to hsbt/ruby that referenced this issue Oct 26, 2024
Prior to 2.7.3, `JSON::Ext::Parser` would only take kwargs.
So if json_pure 2.7.4 is loaded with `json <= 2.7.2` (or stdlib)
it blows up.

Ref: ruby/json#650
Fix: ruby/json#651

ruby/json@4d9dc98817
hsbt pushed a commit to hsbt/ruby that referenced this issue Oct 26, 2024
Prior to 2.7.3, `JSON::Ext::Parser` would only take kwargs.
So if json_pure 2.7.4 is loaded with `json <= 2.7.2` (or stdlib)
it blows up.

Ref: ruby/json#650
Fix: ruby/json#651

ruby/json@4d9dc98817
hsbt pushed a commit to ruby/ruby that referenced this issue Oct 26, 2024
Prior to 2.7.3, `JSON::Ext::Parser` would only take kwargs.
So if json_pure 2.7.4 is loaded with `json <= 2.7.2` (or stdlib)
it blows up.

Ref: ruby/json#650
Fix: ruby/json#651

ruby/json@4d9dc98817
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

No branches or pull requests

4 participants