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

Fix plugin installation from gemfile #6957

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Sep 13, 2023

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

This fixes #6630 and several related issues. Related to #6643, but a very different approach. Specs were copied from that PR though.

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

Instead of blindly calling the installer with the plugins from the gemfile, build a virtual lockfile from the plugin index. Then only call the installer if it needs to be resolved. This fixes several issues:

Make sure the following tasks are checked

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 13, 2023

I've confirmed this also fixes #6589

@ccutrer ccutrer mentioned this pull request Sep 14, 2023
Copy link
Contributor

@pboling pboling left a comment

Choose a reason for hiding this comment

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

This is awesome 🎸

bundler/lib/bundler/plugin.rb Outdated Show resolved Hide resolved
@ccutrer ccutrer force-pushed the plugin-install branch 4 times, most recently from 024ff9c to 7122f47 Compare October 11, 2023 16:43
@slaughter550
Copy link

@pboling anything else you wanted to see on this PR?

@pboling
Copy link
Contributor

pboling commented Dec 8, 2023

Moving the ball forward on plugins is amazing. There is probably still some gap before I can do what I was hoping with plugins, but this is a great step toward making them more of a first-class citizen within bundler.

@deivid-rodriguez
Copy link
Member

This is indeed pretty cool! @ccutrer can you rebase this?

@ccutrer
Copy link
Contributor Author

ccutrer commented Mar 29, 2024

There are several conflicts. I'll try to get them resolved later today.

@ccutrer ccutrer force-pushed the plugin-install branch 2 times, most recently from 31d3050 to c014b7f Compare March 29, 2024 19:53
@ccutrer
Copy link
Contributor Author

ccutrer commented Mar 29, 2024

Rebased and conflicts resolved; I didn't run rubocop or specs locally -- I'll let GitHub Actions handle that

@deivid-rodriguez
Copy link
Member

@ccutrer I assume test failures here mean this still needs some work? Happy to have a look if needed!

plugin_should_not_be_installed("foo")
end

it "upgrade plugins version listed in gemfile" do
Copy link
Member

Choose a reason for hiding this comment

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

Adding a review comment here so we don't forget when we work on this PR: we should add @dfop02 attribution to the commit adding these specs, since he is the original author.

@ccutrer
Copy link
Contributor Author

ccutrer commented May 3, 2024

Yes, this still needs attention. I'm sorry I haven't had time for it - have had other priorities at work.

@deivid-rodriguez
Copy link
Member

No problem at all!

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.

Bundle keep "installing" plugins from Gemfile
4 participants