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

Gemspec should use runtime dependencies #169

Closed
atombender opened this issue Mar 19, 2012 · 11 comments
Closed

Gemspec should use runtime dependencies #169

atombender opened this issue Mar 19, 2012 · 11 comments

Comments

@atombender
Copy link

Short version: WebMock's gemspec file declares its runtime dependencies as development dependencies. Therefore, gem install webmock (or via Bundler) will not enforce any version dependencies.

Long version: I had a problem where WebMock did not enable the Excon adapter. It turns out I was using an older version of Excon than the one required by WebMock. But the reason that WebMock used the old version -- and even installed -- was that the Excon dependency in the gemspec file are declared as development dependencies:

  s.add_development_dependency 'excon',           '>= 0.9.5'

My Gemfile had:

gem 'excon' , '~> 0.7.5'
gem 'webmock'

Development dependencies are for the development of the gem itself. For example, rdoc could be a development dependency, but excon should be a runtime dependency.

@bblimke
Copy link
Owner

bblimke commented Mar 19, 2012

Excon can't be WebMock's runtime dependency nor any other http client lib.
WebMock works with "any" of the http client libraries included in a project, but it can't enforce your project to depend on any http lib.

One thing that WebMock should probably do, is print a warning if http library is not compatible with WebMock, since there is no way to specify these incompatibilities in a gemspec itself.

@atombender
Copy link
Author

Considering that WebMock is run only for tests, I don't see a problem depending on libraries that people don't actually use.

But if you want to be strict about it, the "real" solution is to split the gem into multiple gems with appropriate dependencies. webmock for the core, webmock-excon for the Excon support, etc.

That's also a fairly logical way to structure a gem such as this, otherwise you are merely circumventing RubyGems' dependency support.

@myronmarston
Copy link
Collaborator

I'm with @bblimke. If we follow your suggestion, gem install webmock will install virtually every HTTP client library available in ruby. WebMock currently attempts to require them all (under the assumption that the only installed ones are the ones you are interested in using), so going this route would lead to lots of bloat. Adding them as runtime dependencies will also add time to any bundler operation in a project involving WebMock.

But if you want to be strict about it, the "real" solution is to split the gem into multiple gems with appropriate dependencies. webmock for the core, webmock-excon for the Excon support, etc.

This is a pretty neat idea, but I think it would add a significant maintenance burden. @bblimke would have to maintain and release 8 different gems (core, plus each gem for the 7 different HTTP libraries webmock supports). This would make contributing to WebMock harder, too: when I add a new feature that requires changes to the HTTP library adapters, I'd have to check out and make changes in 8 different gems.

One thing that WebMock should probably do, is print a warning if http library is not compatible with WebMock, since there is no way to specify these incompatibilities in a gemspec itself.

This is a good idea. This is similar to how VCR handles this same issue. I've got a VersionChecker, and each library VCR hooks into checks the version just after being loaded. I've never had any complaints about this.

@bblimke
Copy link
Owner

bblimke commented Mar 19, 2012

Indeed, it will be a pain to maintain 8 gems. I like the fact I just include WebMock in the project,
and I don't need to worry about specific libraries.
Have a look at Faraday gem. It can be used with many different http libs.
Faraday doesn't enforce any of these libs to be required though.

I wish gemspec would support compatibility declarations.

I like the VersionChecker solution. @myronmarston, may I copy/paste it to WebMock, with appropriate information,
that it's copied from VCR? :)

@myronmarston
Copy link
Collaborator

I like the VersionChecker solution. @myronmarston, may I copy/paste it to WebMock, with appropriate information,
that it's copied from VCR? :)

Of course :). There's specs as well if you want to copy them, too.

One thought, though: VCR raises an error if the version is too low. Your initial idea above was to print a warning instead. I think the warning makes sense for WebMock, because of the fact that WebMock attempts to require all installed HTTP libraries, and the person may not need webmock to integrate into one or more of them...so if the version was too low it would be OK. For example, excon is one of the HTTP libraries supported by the riak ruby client to talk to riak, and a user would probably not want to use WebMock for excon in that case--so if the version was too low, it'd be OK. A warning would be better than an error in cases like these.

@phiggins
Copy link
Contributor

I know I'm just jumping on the pile at this point, but I also agree that Webmock should not require all of its dependencies at runtime. This would require Webmock to depend on libcurl through curb, making Webmock all but unusable on Windows machines, for example.

👍 I like the idea of Webmock warning about incompatible versions. It might even be handy to have Webmock warn about incompatible versions upon gem installation as well.

@bblimke
Copy link
Owner

bblimke commented Mar 19, 2012

Is there a "after_install" hook in rubygems? :)

@phiggins
Copy link
Contributor

I thought there was, but after a little research it looks like there's not. You can set a post-install message, but not run arbitrary code for obvious abuse reasons.

@bblimke
Copy link
Owner

bblimke commented Mar 19, 2012

Warning message solution is implemented in version 1.8.4. @alexstaubo thank you for reporting this issue!

@bblimke bblimke closed this as completed Mar 19, 2012
@phiggins
Copy link
Contributor

Maybe VersionChecker should be a gem. :trollface:

@bblimke
Copy link
Owner

bblimke commented Mar 20, 2012

cool, I didn't know you can use :trollface: on github :)

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