samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
185 stars 124 forks source link

Explicit gem dependency on rails versions (vs. many rails components) #1007

Closed dazza-codes closed 7 years ago

dazza-codes commented 7 years ago
$ bundle install
[EngineCart] Unable to find test application dependencies in /data/src/dlss/hydra/hyrax/.internal_test_app/Gemfile, using placeholder dependencies
Fetching gem metadata from https://rubygems.org/..........
Fetching version metadata from https://rubygems.org/...
Fetching dependency metadata from https://rubygems.org/..
Resolving dependencies..........................................................................................................................................................................................................................................................
Bundler could not find compatible versions for gem "activesupport":
  In Gemfile:
    hyrax was resolved to 2.0.0.alpha, which depends on
      active-fedora (>= 11.1.3) was resolved to 11.1.3, which depends on
        activesupport (< 6, >= 4.2.4)

    hyrax was resolved to 2.0.0.alpha, which depends on
      active-fedora (>= 11.1.3) was resolved to 11.1.3, which depends on
        active-triples (~> 0.11.0) was resolved to 0.11.0, which depends on
          activesupport (>= 3.0.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      active-fedora (>= 11.1.3) was resolved to 11.1.3, which depends on
        active-triples (~> 0.11.0) was resolved to 0.11.0, which depends on
          activesupport (>= 3.0.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      active_attr (~> 0.9.0) was resolved to 0.9.0, which depends on
        activesupport (< 5.1, >= 3.0.2)

    hyrax was resolved to 2.0.0.alpha, which depends on
      carrierwave (~> 1.0) was resolved to 1.1.0, which depends on
        activesupport (>= 4.0.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      carrierwave (~> 1.0) was resolved to 1.1.0, which depends on
        activesupport (>= 4.0.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      hydra-head (>= 10.4.0) was resolved to 10.4.0, which depends on
        hydra-access-controls (= 10.4.0) was resolved to 10.4.0, which depends on
          activesupport (< 6, >= 4)

    hyrax was resolved to 2.0.0.alpha, which depends on
      blacklight (~> 6.9) was resolved to 6.10.0, which depends on
        kaminari (>= 0.15) was resolved to 1.0.1, which depends on
          activesupport (>= 4.1.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    hyrax was resolved to 2.0.0.alpha, which depends on
      railties (~> 5.0) was resolved to 5.1.1, which depends on
        activesupport (= 5.1.1)

    rspec-rails (~> 3.1) was resolved to 3.6.0, which depends on
      activesupport (>= 3.0)

    hyrax was resolved to 2.0.0.alpha, which depends on
      active-fedora (>= 11.1.3) was resolved to 11.1.3, which depends on
        solrizer (< 5, >= 3.4) was resolved to 4.0.0, which depends on
          activesupport
dazza-codes commented 7 years ago

At first glance, there are some activesupport < 5.1 in these:

    hyrax was resolved to 2.0.0.alpha, which depends on
      active_attr (~> 0.9.0) was resolved to 0.9.0, which depends on
        activesupport (< 5.1, >= 3.0.2)
    hyrax was resolved to 2.0.0.alpha, which depends on
      active-fedora (>= 11.1.3) was resolved to 11.1.3, which depends on
        solrizer (< 5, >= 3.4) was resolved to 4.0.0, which depends on
          activesupport

In general, for a gem, hyrax seems to have some complex version dependencies on various hyrda and other gems related to rails or rails components, but not on rails itself, e.g.

$ grep rails hyrax.gemspec
  spec.add_dependency 'tinymce-rails', '~> 4.1'
  spec.add_dependency 'tinymce-rails-imageupload', '~> 4.0.17.beta'
  spec.add_dependency 'font-awesome-rails', '~> 4.2'
  spec.add_dependency 'select2-rails', '~> 3.5.9'
  spec.add_dependency 'jquery-ui-rails', '~> 5.0'
  spec.add_dependency 'flot-rails', '~> 0.0.6'
  spec.add_dependency 'almond-rails', '~> 0.1'
  spec.add_dependency 'jquery-datatables-rails', '~> 3.4.0'
  spec.add_dependency 'clipboard-rails', '~> 1.5'
  spec.add_dependency 'rails_autolink', '~> 1.1'
  spec.add_dependency 'breadcrumbs_on_rails', '~> 3.0'

  spec.add_development_dependency 'rspec-rails', '~> 3.1'
  spec.add_development_dependency "factory_girl_rails", '~> 4.4'
  spec.add_development_dependency 'rails-controller-testing', '~> 0'
mjgiarlo commented 7 years ago

@darrenleeweber just to clarify, by "clean clone," you mean you don't have a Gemfile.lock yet?

dazza-codes commented 7 years ago

Yes, a clean clone means something like this pseudo-code

cd ...
rm -rf hyrax
git clone {hyrax-repo}
cd hyrax
bundle install

But this bug might be fixed now. I guess the issue really is whether or not hyrax should have a devel-dependency on a range of rails versions. From slack discussions, probably not.

jrochkind commented 7 years ago

hyrax has a dependencies on railtiles, which currently allows any 5.x. https://github.com/projecthydra-labs/hyrax/blob/master/hyrax.gemspec#L56

railties always depends on activesupport, and actionpack with the same version number as the railties. https://rubygems.org/gems/railties/versions/5.0.0.1

So it effectively is expressing a dependency, with some requirements (currently 5.x including 5.1) on some parts of rails, although not all.

It's unclear to me what the benefit of avoiding dependency on the other parts of rails are (can hydra work without activerecord? I don't think so. But it doesn't declare a dependency on it). It's unclear to me what the argument, referenced here, would be for making hydra not express dependencies on any parts of Rails. It's unclear to me if hyrax ought to be allowing Rails 5.1 in it's dependency requirements, as it currently does does -- does hyrax actually work on Rails 5.1? (I see it's currently passing travis in master on 5.1, so great; but the dependency would also allow a future rails 5.2, which seems unwise).

If we know that hyrax requires rails to work, and certain versions of rails at that -- isn't that exactly what explicit machine-readable dependencies are for?

jcoyne commented 7 years ago

It's unclear to me what the benefit of avoiding dependency on the other parts of rails are (can hydra work without activerecord? I don't think so. But it doesn't declare a dependency on it). It's unclear to me what the argument, referenced here, would be for making hydra not express dependencies on any parts of Rails.

Presently there's no dependency on actioncable. So we should either require activerecord and activejob or we should just require rails and be okay with the fact that we're pulling down actioncable and not using it.

does hyrax actually work on Rails 5.1? (I see it's currently passing travis in master on 5.1, so great; but the dependency would also allow a future rails 5.2, which seems unwise).

Yes, Hyrax works with Rails 5.1. If Rails follows semver then we shouldn't have any problem working with Rails 5.2 when it comes out.

jrochkind commented 7 years ago

If Rails follows semver then we shouldn't have any problem working with Rails 5.2 when it comes out.

Rails does not follow semver, and does release backwards incompat changes in 'minor' releases -- as just about anyone who's upgraded a gem for a rails minor release probably knows! But those are not bugs, rails does not follow semver.

http://guides.rubyonrails.org/maintenance_policy.html

Minor Y

New features, may contain API changes (Serve as major versions of Semver). Breaking changes are paired with deprecation notices in the previous minor or major release.

That is, Rails treats the minor version bump as if it were semver major version bump. And has additional strict deprecation policy that I don't think is covered by semver.

jrochkind commented 7 years ago

Presently there's no dependency on actioncable. So we should either require activerecord and activejob or we should just require rails and be okay with the fact that we're pulling down actioncable and not using it.

I think almost every other gem that only works with rails takes the later path. Most of them don't actually use actioncable, most of them just declare dependency on 'rails'. There are few downsides to this.

mjgiarlo commented 7 years ago

Given what @jrochkind says above, I wonder if we should consider pinning Rails more tightly going forward. Either that or better document the RAILS_VERSION trick that we get from EngineCart.

dazza-codes commented 7 years ago

Given the ongoing discussion, I'm going to re-title this issue and re-open it under the new title.

mjgiarlo commented 7 years ago

Chatted on stand-up today, and @darrenleeweber will close this issue and create a new one to declare rails, rather than railties, as a dependency within Hyrax.

dazza-codes commented 7 years ago

Moved to https://github.com/projecthydra-labs/hyrax/issues/1028