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

Simpler version matching #1053

Closed
seanlinsley opened this issue Oct 27, 2014 · 6 comments
Closed

Simpler version matching #1053

seanlinsley opened this issue Oct 27, 2014 · 6 comments

Comments

@seanlinsley
Copy link

Since Rubygems doesn't have a way to require optional runtime dependencies, when working on the Active Admin gem I've often been frustrated by the available methods to version-check a loaded gem.

It seems like the correct way is:

Gem::Requirement.create('> 3').satisfied_by? Gem.loaded_specs['rails'].version

Not the easiest thing to remember, which I assume is why I often see developers comparing against the split version constants Rails provides:

Rails::VERSION::MAJOR > 3 && Rails::VERSION::MINOR > 2

Or, for gems that only have a version string:

Draper::VERSION > '1.3.0'

Both approaches get more complex of you want to correctly deal with prerelease versions, or match a range of versions.

To make development easier I built an abstraction layer:

ActiveAdmin::Dependency.rails? '>= 3.2', '< 4.2'

ActiveAdmin::Dependency.rails >= 3.2

ActiveAdmin::Dependency.rails! '~> 4.1'
# raises an exception: You provided rails 3.2.18 but we need: ~> 4.1.

ActiveAdmin::Dependency.devise!
# when not provided, raises an exception: To use devise you need to specify it in your Gemfile.

ActiveAdmin::Dependency['jquery-ui-rails'] < 5

Would this general feature set be desirable for Rubygems itself?

@drbrain
Copy link
Member

drbrain commented Oct 27, 2014

Why not use gem 'rails', '>= 3.2', '< 4.2' which will activate the given version or raise an exception if it cannot be loaded or an incompatible version is loaded.

@seanlinsley
Copy link
Author

I wasn't aware it was considered acceptable for gems to load dependent gems at runtime that way.

Regardless, that doesn't handle:

  • soft dependencies (only run some integration code if the gem is available)
  • conditional behavior for different versions of a gem

@drbrain
Copy link
Member

drbrain commented Oct 27, 2014

For gems listed as dependencies in your specification there is no need to gem as the matching version will be activated at runtime. For optional dependencies you do need to activate a version and gem is the API for this.

I don't understand what your objections have to do with version matching via gem.

If you only want to run some code if some gem is available you still need to activate it via gem.

Changing the API for version matching doesn't handle conditional behavior for different versions of a gem either. The conditional code you need will likely dwarf the few characters of savings provided by a different API.

This is handled fairly cleanly by a case statement after you've activated a gem, though:

case Gem.loaded_specs['rake'].version
when Gem::Requirement.new '~> 10.0' then
  # …
else
  # …
end

I wouldn't mind changing satisfied_by? to accept a Version or a Specification as it seems redundant to supply the version. I wouldn't mind fixing Specification#satisfies_requirement? to accept a Requirement, Requirement string or a Dependency (Gem.loaded_specs['rake'].satisfies_requirement? '~> 10.0').

I am not particularly fond of method_missing and Matcher APIs as they create extra garbage that needs to be cleaned up.

I am not particularly fond of encouraging multiple version support as, in general, it does not encourage developers of libraries to commit to stable APIs. I prefer to follow in Matz's footsteps and make things you aren't supposed to do difficult.

I recognize for projects that extend Rails that this is an unfair, but I fear that adding this to make it easier to interact with Rails will lead developers to think API churn is OK for their library that is orders of magnitude smaller.

@drbrain drbrain modified the milestones: Future, 2.5 Nov 22, 2014
@drbrain
Copy link
Member

drbrain commented Nov 22, 2014

I will streamline satisfied_by? and satisfies_requirement? as described above.

@copiousfreetime
Copy link
Contributor

Proposal is:

  • update Gem::Requirement#satisfied_by? to accept a Gem::Version or Gem::Specification
  • update Gem::Specification#satisfies_requirement? to accept a Requirement, Requirement string or a Dependency

@bronzdoc
Copy link
Member

This issue hasbeen ide for too long and nobody else has requested this feature, i'm going to close it for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants