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

[Definition] Filter out unneeded gem platforms after resolving#6495

Merged
bundlerbot merged 1 commit intomasterfrom
segiddins/6491-extra-gem-platform-in-lockfile
Jul 22, 2018
Merged

[Definition] Filter out unneeded gem platforms after resolving#6495
bundlerbot merged 1 commit intomasterfrom
segiddins/6491-extra-gem-platform-in-lockfile

Conversation

@segiddins
Copy link
Copy Markdown
Contributor

What was the end-user problem that led to this PR?

The problem was the lockfile would contain platform-specific gems that it didn't need

Closes #6491

What was your diagnosis of the problem?

My diagnosis was the resolver sometimes activates platforms it doesn't need, since it can't know it won't need them

What is your fix for the problem, implemented in this PR?

My fix is to use SpecSet#for to filter out gems that won't ever be used

Why did you choose this fix out of the possible options?

I chose this fix because it isn't re-inventing the wheel!

@segiddins segiddins force-pushed the segiddins/6491-extra-gem-platform-in-lockfile branch from 4029a98 to e2e8e5a Compare April 19, 2018 04:49
@segiddins segiddins requested a review from indirect May 1, 2018 05:15
@segiddins
Copy link
Copy Markdown
Contributor Author

@indirect please review ?

@ThomasKoppensteiner
Copy link
Copy Markdown

Any updates on this? Is this PR merge-ready?

@segiddins
Copy link
Copy Markdown
Contributor Author

It’s just awaiting review

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!


def platforms_for_dependency_named(dependency)
__dependencies.select {|_, deps| deps.map(&:name).include? dependency }.keys
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just dead code not meant to be used inside or outside of bundler... right?

@segiddins
Copy link
Copy Markdown
Contributor Author

@bundlerbot r+

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit e2e8e5a has been approved by segiddins

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit e2e8e5a with merge 7b603f3...

bundlerbot added a commit that referenced this pull request Jul 22, 2018
…ckfile, r=segiddins

[Definition] Filter out unneeded gem platforms after resolving

### What was the end-user problem that led to this PR?

The problem was the lockfile would contain platform-specific gems that it didn't need

Closes #6491

### What was your diagnosis of the problem?

My diagnosis was the resolver sometimes activates platforms it doesn't need, since it can't know it won't need them

### What is your fix for the problem, implemented in this PR?

My fix is to use `SpecSet#for` to filter out gems that won't ever be used

### Why did you choose this fix out of the possible options?

I chose this fix because it isn't re-inventing the wheel!
@bundlerbot
Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-travis
Approved by: segiddins
Pushing 7b603f3 to master...

@bundlerbot bundlerbot merged commit e2e8e5a into master Jul 22, 2018
@deivid-rodriguez deivid-rodriguez deleted the segiddins/6491-extra-gem-platform-in-lockfile branch July 22, 2018 18:11
@colby-swandale colby-swandale added this to the 1.16.6 milestone Oct 2, 2018
colby-swandale pushed a commit that referenced this pull request Oct 5, 2018
…ckfile, r=segiddins

[Definition] Filter out unneeded gem platforms after resolving

### What was the end-user problem that led to this PR?

The problem was the lockfile would contain platform-specific gems that it didn't need

Closes #6491

### What was your diagnosis of the problem?

My diagnosis was the resolver sometimes activates platforms it doesn't need, since it can't know it won't need them

### What is your fix for the problem, implemented in this PR?

My fix is to use `SpecSet#for` to filter out gems that won't ever be used

### Why did you choose this fix out of the possible options?

I chose this fix because it isn't re-inventing the wheel!

(cherry picked from commit 7b603f3)
colby-swandale pushed a commit that referenced this pull request Oct 5, 2018
…ckfile, r=segiddins

[Definition] Filter out unneeded gem platforms after resolving

### What was the end-user problem that led to this PR?

The problem was the lockfile would contain platform-specific gems that it didn't need

Closes #6491

### What was your diagnosis of the problem?

My diagnosis was the resolver sometimes activates platforms it doesn't need, since it can't know it won't need them

### What is your fix for the problem, implemented in this PR?

My fix is to use `SpecSet#for` to filter out gems that won't ever be used

### Why did you choose this fix out of the possible options?

I chose this fix because it isn't re-inventing the wheel!

(cherry picked from commit 7b603f3)
colby-swandale pushed a commit that referenced this pull request Oct 7, 2018
* 1-16-stable:
  Version 1.16.6 with changelog
  fix uninitialized @use_gvp instance var warning
  no longer test Ruby 1.9.3 against rubygems master
  Merge #6708
  Auto merge of #6697 - walf443:added_changelog_section, r=hsbt
  Merge #6687
  Merge #6686
  Auto merge of #6670 - bundler:colby/invite-stephanie-morillo, r=segiddins
  Auto merge of #6627 - agrim123:agr-fix-add-groups, r=deivid-rodriguez
  Auto merge of #6612 - hdf1986:readme-bundle-add, r=segiddins
  Auto merge of #6495 - bundler:segiddins/6491-extra-gem-platform-in-lockfile, r=segiddins
  Auto merge of #6493 - agrim123:agr-update-bundle-update-docs, r=colby-swandale
  Auto merge of #6310 - utilum:rescue_unspecified_exception, r=segiddins
  Auto merge of #6184 - arbonap:pa-check-in-gemfile-docs, r=indirect
  fix typo
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.

Strange lockfile behaviour with bundle update/install and platform-specific gems

5 participants