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 deprecation warning with --binstubs flag #129

Merged
merged 2 commits into from
May 28, 2022

Conversation

danarnold
Copy link
Contributor

The --binstubs flag to bundle install is deprecated and raises a deprecation warning in more recent versions of bundler. From rubygems/bundler/UPGRADING.md:

The bundle install command will no longer accept a --binstubs flag.

The --binstubs option has been removed from bundle install and replaced with the bundle binstubs command. The --binstubs flag would create binstubs for all executables present inside the gems in the project. This was hardly useful since most users will only use a subset of all the binstubs available to them. Also, it would force the introduction of a bunch of most likely unused files into source control. Because of this, binstubs now must be created and checked into version control individually.

I've updated the bundle install task to run the equivalent command with bundle binstubs instead of using the --binstubs flag with bundle install to fix the deprecation warning. I'm not sure if this is the preferred solution since it makes the install task run an additional command, but it seems reasonable to me because it accomplishes the same goal.

@mattbrictson
Copy link
Member

I worry this will break backwards compatibility with projects that are still using Bundler 1.x. Is there a way to address the deprecation warning while still maintaining capistrano-rails's compatibility with Bundler 1.x projects?

@danarnold
Copy link
Contributor Author

That's a good point. We could run bundler --version at the beginning of the task and then execute different commands based on the version that's running, but that adds one more thing to run every time the task executes. Alternatively, we could add a config option to set version compatibility instead of running bundler --version, but I worry that people could miss this. Maybe both, where it will fall back to bundler --version but use the config option if it's set? That's the most flexible, but it's also the most heavy handed solution.

@mattbrictson
Copy link
Member

Could we leave the bundle_binstubs behavior as-is, and introduce a new setting for the Bundler 2+ behavior? I am not Sur what would be a good name... Maybe bundle_generate_binstubs? Then we could update the README to direct people to use the new name.

@mattbrictson
Copy link
Member

Actually, bundle_binstubs_path might be a better name, since that more explicitly describes what the setting is used for.

@danarnold
Copy link
Contributor Author

How about a new option :bundle_binstubs_command which could be set to either :install (which would be for bundler <2.1) or :binstubs (which would be for bundler >= 2.1)? Then we could default it to :install for now, requiring users to opt in to the new behavior.

I think :bundle_binstubs_path is more descriptive than :bundle_binstubs for sure. Of course, renaming it does introduce a backwards incompatible change. If we're adding a new option, we don't technically need to change it at all.

@mattbrictson
Copy link
Member

How about a new option :bundle_binstubs_command ...

That sounds good! 👍

…tion :bundle_binstubs_command, which can be either the default :install or :binstubs for bundler >= 2.1. add documentation for this option to the README.
@danarnold
Copy link
Contributor Author

I've made those changes in the above commit.

Copy link
Member

@mattbrictson mattbrictson 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, thanks!

@mattbrictson mattbrictson added the ✨ Feature Adds a new feature label May 28, 2022
@mattbrictson mattbrictson merged commit a68b50a into capistrano:master May 28, 2022
@mattbrictson
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature Adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants