capistrano / bundler

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

Generating binstubs on deployment seems destined to break when old releases are removed #111

Open lazyatom opened 5 years ago

lazyatom commented 5 years ago

We're seeing an issue that seems superficially similar to #22 -- in a nutshell, binstubs are generated with paths to the Gemfile for a specific Capistrano release, and once that release is reaped by Capistrano, the binstubs used by the current version of the application break.

Background

The relevant part of our Capistrano configuration:

set :bundle_binstubs, -> { shared_path.join('binstubs') }
set :bundle_gemfile, -> { release_path.join('Gemfile') }
set :bundle_flags, '--deployment'
append :linked_dirs, '.bundle'

On our servers, we have /app/shared/binstubs in the PATH, and this is the bundler config, symlinked into every release:

$ cat /app/shared/.bundle/config
---
BUNDLE_PATH: "/app/shared/bundle"
BUNDLE_DISABLE_SHARED_GEMS: "true"
BUNDLE_FROZEN: "true"
BUNDLE_BIN: "/app/binstubs"
BUNDLE_JOBS: "4"
BUNDLE_WITHOUT: "development:test"

We're using Bundler 1.17.3:

$ bundle -v
Bundler version 1.17.3

And these Capistrano versions:

$ bundle list | grep capistrano
  * capistrano (3.11.0)
  * capistrano-bundler (1.3.0)
  * capistrano-faster-assets (1.1.0)
  * capistrano-maintenance (1.2.0)
  * capistrano-rails (1.4.0)

(I realise capistrano-bundler is not at the latest release, but the changes between 1.3.0 and 1.5.0 don't seem like they would impact this at all)

The issue

When deploying, bundle install works as you'd expect, installing the gems and binstubs into the shared directory:

bundle install --gemfile /app/releases/20190425155236/Gemfile --path /app/shared/bundle --binstubs /app/shared/binstubs --jobs 4 --without development test --deployment

However, within the generated binstubs, they all include a reference to the specific release directory:

$ grep GEMFILE binstubs/rake
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../../releases/20190425155236/Gemfile",

Since binstubs aren't automatically regenerated on every deploy, but we only retain a certain number of releases, so at some point, the release referenced in the binstubs is removed, and they all break.

Our current workaround is to explicitly set BUNDLE_GEMFILE to /app/current/Gemfile, but this causes other unrelated issues (e.g. new gems aren't visible to the application until after the symlink has been changed).

I think we're largely following the default approach for capistrano-bundler as described in the README, and I think this is the default behaviour of bundler.

45 suggests that the right way to avoid this is by checking binstubs into the application repository. That would certainly work, since by definition the relative path of each binstubs and the Gemfile won't change.

So I suppose my question is: shouldn't everyone who is generating binstubs on deployment be seeing this issue? Maybe nobody else is either reaping old releases or generating binstubs? If this is the default behaviour, I wonder if the implementation should actually explicitly not support generating binstubs, and the documentation should propose the approach from #45?

Hopefully that's all clear; let me know if there's any other details that I can provide which might help

mattbrictson commented 5 years ago

It seems to me that having Capistrano generate binstubs is fundamentally problematic. There is the issue you mention of the binstubs pointing to reaped releases. But even if that were fixed and the binstubs always pointed to the "current" Gemfile, that won't work either because during a deploy you are generating the binstubs before you update the current symlink. So there would be a period of time during deploys where the binstubs are pointing to the wrong Gemfile.

I always generate binstubs locally and commit them to git, as discussed in #45.

So I suppose my question is: shouldn't everyone who is generating binstubs on deployment be seeing this issue? Maybe nobody else is either reaping old releases or generating binstubs? If this is the default behaviour, I wonder if the implementation should actually explicitly not support generating binstubs, and the documentation should propose the approach from #45?

I would be in favor of removing mention of the bundle_binstubs config from the README and recommending the approach from #45. We should still keep the feature around, to avoid breaking existing deploys. If you would like to open a PR for the README changes I am happy to merge it.