Skip to content

Dont warn on prerelease dependencies with less than ("<")#2323

Closed
deivid-rodriguez wants to merge 2 commits intoruby:masterfrom
deivid-rodriguez:dont_warn_on_prerelease_plus_less_than
Closed

Dont warn on prerelease dependencies with less than ("<")#2323
deivid-rodriguez wants to merge 2 commits intoruby:masterfrom
deivid-rodriguez:dont_warn_on_prerelease_plus_less_than

Conversation

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Description:

I just saw this warning when installing my gem:

WARNING:  prerelease dependency on rails (< 6.0.x, >= 5.2) is not recommended

I can see the rationale for the "prerelease warning" but doesn't look reasonable when the requirement is using "<", since in that case we're filtering out prereleases. Right?


Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

@segiddins
Copy link
Copy Markdown
Contributor

In bundler, this sort of requirement will start allowing pre-releases

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

I don't understand, can you elaborate?

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

Elaborating myself, note that this is only removing a warning message in some cases where I think doesn't make sense. It's not changing behavior other than that.

@segiddins
Copy link
Copy Markdown
Contributor

Right, but if you have ">= 1", "< 2.pre" as a dependency, bundler will resolve that differently than "~> 1.0", even though they seem equivalent

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

You mean that they may get different, both correct, resolutions? I think that's fine, this is just a recommendation. In any case, that seems like a comment against introducing #2266 and #2242, not this PR. Or I'm missing something?

@segiddins
Copy link
Copy Markdown
Contributor

What I’m trying to say is that a prerelease requirement in a < is just as “harmful” as with any other operator

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

deivid-rodriguez commented Jun 27, 2018

Ok, so I don't understand then.

If I specify s.add_dependency "rails", "< 6.0.x", ">= 5.2" as a dependency of my gem, I don't think I'm depending on any pre-release version of rails, so the warning WARNING: prerelease dependency on rails (< 6.0.x, >= 5.2) is not recommended doesn't make sense to me. Are you saying that this requirement actually allows prereleases of rails, i.e., that #2266 is incorrect? That's definitely not my experience with bundler.

@segiddins
Copy link
Copy Markdown
Contributor

Are you saying that this requirement actually allows prereleases of rails

I believe that is the case, though I could be wrong

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

deivid-rodriguez commented Jun 28, 2018

I think you might be wrong, yeah. I just tried it and got:

Fetching gem metadata from https://rubygems.org/.........
Fetching gem metadata from https://rubygems.org/.
Resolving dependencies.....
Bundler could not find compatible versions for gem "decidim":
  In snapshot (Gemfile.lock):
    decidim (= 0.13.0.pre1)

  In Gemfile:
    decidim (= 0.13.0.pre1)

    my_test_gem was resolved to 0.1.0, which depends on
      decidim (< 0.13.x, >= 0.12)

Running `bundle update` will rebuild your snapshot from scratch, using only
the gems in your Gemfile, which may resolve the conflict.

And then running bundle update leads to

Fetching gem metadata from https://rubygems.org/.........
Fetching gem metadata from https://rubygems.org/.
Resolving dependencies...
Bundler could not find compatible versions for gem "decidim":
  In Gemfile:
    decidim (= 0.13.0.pre1)

    my_test_gem was resolved to 0.1.0, which depends on
      decidim (< 0.13.x, >= 0.12)

If I change the dependency to >= 0.12, < 0.13, it properly resolves and allows the prerelease.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

This warning might be confusing and not very easy to understand / agree with, so we might consider entirely dropping it as you suggested in #2228 (comment). But if the warning stays, I think this PR should go in.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

I opened #2351 as a (better, I think) alternative.

@bronzdoc
Copy link
Copy Markdown
Contributor

closing in favor of #2351

@bronzdoc bronzdoc closed this Jul 18, 2018
bundlerbot added a commit that referenced this pull request Jul 18, 2018
Remove semver gem build warning

# Description:

This is an alternative to #2323.

I think this warning is too brittle at the moment because `rubygems` has no way of knowing if a dependency is semantically versioned. If a dependency is not semantically versioned, I don't see the problem with using something like `~> 1.2.3` so suggesting a constraint that does the same thing in a more complicated way just because it does not warn seems no good.

In the future maybe rubygems could provide a `spec.versioning_policy =` accessor where gems can inform of their own versioning (`:semver`, or others) but currently I think this warning should just go.
______________

# Tasks:

- [x] Describe the problem / feature
- [ ] Write tests
- [x] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
@deivid-rodriguez deivid-rodriguez deleted the dont_warn_on_prerelease_plus_less_than branch July 18, 2018 23:30
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

Successfully merging this pull request may close these issues.

3 participants