Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Fix --source option causing incorrect gem unlocking #3763

Merged
merged 6 commits into from
Aug 24, 2015
Merged

Fix --source option causing incorrect gem unlocking #3763

merged 6 commits into from
Aug 24, 2015

Conversation

neoeno
Copy link
Contributor

@neoeno neoeno commented Jun 18, 2015

Fixes #3759 and #3761.

And also another issue I found where if:

  1. You have a Gemfile with a group,
  2. That group contains a gem with the same name as a source
  3. And you run bundle update --group group_name
  4. Then all gems associated with the source will be updated.

The 3 cases fixed here are all pretty subtle. The specs should make it clearer, and the github issues above add more context.

Note that this PR removes the ability to use bundle update --source gem_name as a hacky way to update a gem 'conservatively'. This is detailed in #3759.

@indirect
Copy link
Member

Thanks for the patch! As I commented over in #3759, we can't remove the --source hack until 2.0. If you remove that commit from this PR, I'd be happy to merge here. That said, I would also be super happy to merge that patch into 2-0-dev if you can submit your patch as a PR against that branch.

I'd also love to ship a documented --only option that limits updates to a single gem or gems. Since you're currently the world expert on this code, would you be willing to implement that in a PR?

Thanks!

@neoeno
Copy link
Contributor Author

neoeno commented Jun 19, 2015

Ha — I just realised why the tests are failing. Too much of the wrong kind of thinking! New commit to (hopefully) fix.

Anyway, thanks for the response! Hearing you on the backwards-compatibility thing. For this PR, I could have a go at fixing #3761 and the issue detailed in the OP — while preserving the undocumented --source behaviour. Does that sound like it'd be useful?

Also happy to take a look at 2-0-dev and look at applying the --source fix to there.

Piqued my curiosity on the potential --only flag so I'll take a look at that too.

@indirect
Copy link
Member

Awesome! That all sounds great. :)

@neoeno
Copy link
Contributor Author

neoeno commented Jul 13, 2015

Looks like I've still got plenty of work to do making this pass with old versions of RubyGems(?) but I think this is more or less in its final form.

It fixes #3761, and the --group issue detailed in this PR — along with adding tests for the behaviour. It preserves the --source behaviour detailed in #3759 and adds tests for it too (since it's now expected). There's also a test for the 'correct' behaviour which I've left pending with a comment to explain the situation.

I'll have a go at correcting the behaviour in the 2-0-dev branch next.

@indirect
Copy link
Member

This is super impressive, thanks! 😀

@neoeno
Copy link
Contributor Author

neoeno commented Jul 30, 2015

So I'm finding the tests here a little hard to figure out — seems like travis isn't running on this PR anymore? And also I'm not sure if master is green right now? Could you advise?

As far as I can work out the tests here pass (for me), except spec/commands/help_spec.rb:28 which fails for me on master too.

@segiddins segiddins closed this Aug 14, 2015
@segiddins segiddins reopened this Aug 14, 2015
@segiddins
Copy link
Member

@neoeno I had travis re-run the PR, it looks like the only failures are due to rubocop, bin/rake spec:deps rubocop:auto_correct

@indirect
Copy link
Member

Rebased and corrected styles at #3948, which will merge when the tests pass.

homu added a commit that referenced this pull request Aug 19, 2015
Rebase and correct styles on #3763

None
@neoeno
Copy link
Contributor Author

neoeno commented Aug 19, 2015

Ah — sorry about that! Thought I'd caught all those.

@neoeno
Copy link
Contributor Author

neoeno commented Aug 24, 2015

(CC #3948) OK —

  • Rebased in @indirect's rubocop fixes
  • Amended some testing message terminology
  • Moved some tests out of context blocks as re-building git repos in the same test run seemed to cause the issues in older versions of RubyGems.

One test fail remains: https://travis-ci.org/bundler/bundler/jobs/76903851 — is this expected or unexpected? Seems like an odd state it's got into.

@indirect
Copy link
Member

We've been fighting with that particular periodic test failure lately—just merged something to master that I think will help. In the meantime, I've just restarted that particular job and I expect it to pass. Thanks for sticking with this, and thanks again for all your work on this set of issues!

@indirect
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Aug 24, 2015

📌 Commit ac30a5b has been approved by indirect

@homu
Copy link
Contributor

homu commented Aug 24, 2015

⌛ Testing commit ac30a5b with merge c44fe31...

homu added a commit that referenced this pull request Aug 24, 2015
Fix `--source` option causing incorrect gem unlocking

Fixes #3759 and #3761.

And also another issue I found where if:

1. You have a Gemfile with a group,
2. That group contains a gem with the same name as a source
3. And you run `bundle update --group group_name`
4. Then all gems associated with the source will be updated.

The 3 cases fixed here are all pretty subtle. The specs should make it clearer, and the github issues above add more context.

Note that this PR removes the ability to use `bundle update --source gem_name` as a hacky way to update a gem 'conservatively'. This is detailed in #3759.
@homu
Copy link
Contributor

homu commented Aug 24, 2015

💔 Test failed - status

@neoeno
Copy link
Contributor Author

neoeno commented Aug 24, 2015

Looks like another strange result. Let me know if there's anything I should be doing!

@indirect
Copy link
Member

Ugh, that must be the RubyGems connectivity issues that are happening right now. :(

On Mon, Aug 24, 2015 at 2:47 AM, Caden Lovelace [email protected]
wrote:

Looks like another strange result. Let me know if there's anything I should be doing!

Reply to this email directly or view it on GitHub:
#3763 (comment)

indirect added a commit that referenced this pull request Aug 24, 2015
Fix `--source` option causing incorrect gem unlocking
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bundle update --source gem updates conservatively
5 participants