Skip to content

Conversation

@sarahsehr
Copy link
Contributor

@sarahsehr sarahsehr commented May 8, 2024

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

When starting to work on my first issue, I found that rake setup failed for me. This was due to having previously installed a plugin, whose files had since been deleted (presumably when I uninstalled that version of Ruby).

Fixes #7636.

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

Check to see whether each load path for the plugin is a directory. If it is not, notify the user in a friendly message which plugin in the problem and how to resolve the issue.

Make sure the following tasks are checked

@welcome
Copy link

welcome bot commented May 8, 2024

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

@simi
Copy link
Contributor

simi commented May 8, 2024

Hello @sarahsehr. Code looks good, but some specs are failing, which seems not really related to your change, since I can see the same failure on main branch. I'm looking into. 👀

EDIT: false alarm, main branch looks ok, it was outdated tmp during my initial test.

@deivid-rodriguez
Copy link
Contributor

Since this a bundle install run after all, should we automatically reinstall any missing plugins if necessary, instead of erroring out?

@simi
Copy link
Contributor

simi commented Jul 31, 2024

@sarahsehr friendly ping, is there still interest to finish this? Happy to help or take over to fix the one blocking comment in plugin.rb.

@sarahsehr
Copy link
Contributor Author

@sarahsehr friendly ping, is there still interest to finish this? Happy to help or take over to fix the one blocking comment in plugin.rb.

Thanks for the friendly ping! Coincidentally, I was planning to work on this today. :)

@deivid-rodriguez
Copy link
Contributor

We're almost done with a big rework of how plugins currently work at #6957, I wonder if that could actually fix/improve this problem.

@sarahsehr sarahsehr force-pushed the improveErrorForLoadingBrokenPlugin branch from 6694e0a to f285956 Compare August 2, 2024 18:31
@sarahsehr
Copy link
Contributor Author

We're almost done with a big rework of how plugins currently work at #6957, I wonder if that could actually fix/improve this problem.

Thanks for pointing this out! I pulled the branch from that PR and was able to reproduce the issue, so unfortunately it doesn't.

@sarahsehr sarahsehr force-pushed the improveErrorForLoadingBrokenPlugin branch from f285956 to 75e6bfc Compare August 2, 2024 21:40
@sarahsehr
Copy link
Contributor Author

PluginLoadFailedWarning

@simi I have changed the output to warning and continue the install. The output looks like above (ignore the black line, that's debug output from another gem).

@sarahsehr sarahsehr force-pushed the improveErrorForLoadingBrokenPlugin branch 2 times, most recently from 9d8aed8 to 9ad0bce Compare September 13, 2024 19:40
If a plugin has previously been installed, but the path is no longer
valid, `rake setup` will fail with an unexpected error due to the file
not existing.

Instead, we want to present the user with what the issue is and how to
resolve the problem.
@deivid-rodriguez deivid-rodriguez force-pushed the improveErrorForLoadingBrokenPlugin branch from 9ad0bce to 0c6ad3e Compare October 31, 2024 15:29
@deivid-rodriguez
Copy link
Contributor

This seems like a strict improvement, let's merge it!

@deivid-rodriguez deivid-rodriguez merged commit d57d302 into ruby:master Oct 31, 2024
69 checks passed
@deivid-rodriguez
Copy link
Contributor

Thanks so much @sarahsehr!

@sarahsehr sarahsehr deleted the improveErrorForLoadingBrokenPlugin branch October 31, 2024 19:01
deivid-rodriguez added a commit that referenced this pull request Nov 4, 2024
…lugin

Add useful error message for plugin load

(cherry picked from commit d57d302)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid plugin configuration should have a more useful error message when running rake setup

3 participants