capistrano / bundler

Bundler support for Capistrano 3.x
MIT License
219 stars 83 forks source link

Fix deprecation warning with --binstubs flag #129

Closed danarnold closed 2 years ago

danarnold commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

danarnold commented 2 years ago

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 commented 2 years ago

How about a new option :bundle_binstubs_command ...

That sounds good! 👍

danarnold commented 2 years ago

I've made those changes in the above commit.

mattbrictson commented 2 years ago

Released in https://github.com/capistrano/bundler/releases/tag/v2.1.0