Added a bundler:clean task

[betesh]

No description provided.

[kirs]

make sure to run cap bundler:install immediately after the rollback.

It brings some kind of unstability to rollbacked releases. I'm not really sure that we need to include this task into the gem.

[betesh]

As I commented on #65, now that there's a hook for running bundler:install on a rollback, that part of the documentation is no longer applicable. I have updated the documentation. Please reconsider. Thank you.

[Kriechi]

Why altering the log level? Can't you use something like caputure to get the output and then print it?
Apart from that: 👍

[betesh]

@Kriechi Done, please take another look.

[Kriechi]

Nice!
What happens if the app was bundled with bundle_path set to something? Should that parameter be used during bundle clean as well?

[betesh]

Agreed. There are some arguments that should be passed here if they are set. I have to take a look at the docs to see which ones are applicable. Hopefully I can get to this later today.

[betesh]

Done, a few days later than I'd hope. Please take a look again.

[betesh]

@kirs Can we get this merged?

[cknoxrun]

Would love to see this merged too!

[adrazek]

Will this PR be merged someday ?

[mattbrictson]

What about just adding --clean to your :bundle_flags?

[betesh]

That's risky--what if the deployment fails and need to be rolled back? Some gems (nokogiri, etc.) may take a long time to install and rollbacks need to be as fast as possible.

[mattbrictson]

@betesh If rollbacks need to be as fast as possible, then it seems like running bundle clean at any point is therefore risky, as pointed out earlier in this thread. Not sure we should encourage users to run clean by including it as part of this gem if it complicates/slows rollbacks.

[mattbrictson]

Since there are some rollback risks involved in supporting this feature, and since it is fairly trivial for someone to implement their own clean task in deploy.rb if they need it, I'm going to close this PR.

[will-in-wi]

I just ran across this. I'm about to write my own Gem to do it, but it might be worth putting in core. If this is in core, it should only be a task with no default hook.

bundle install --clean is an issue since it means that a failed deploy would leave gems in an inconsistent state.

If someone wants this task, the would just need to add it as an after published task.

This would indeed slow rollbacks slightly, but as long as we don't make it the default, I don't think it is an issue. We should just make sure to document the tradeoff.

In my case, we are running out of disk space due to tons of Gems lying around. I'm running a cleanup command periodically right now.

@mattbrictson, have I provided some decent arguments in favor of this change?

[adrazek]

+200

[mattbrictson]

If you are running out of disk space due to old gems piling up, then this is a more pressing issue than I realized. In that case, I'm in favor or adding the clean task as long as it is not run automatically and its tradeoffs are documented, as you suggested. In rare cases it may not be possible to rollback at all, e.g. if a gem was yanked.

[will-in-wi]

@betesh: Could you take a whack at documenting the tradeoffs?

To give you a head start, what do you think of something like:

If you want to clean up gems after a successful deploy, add after 'deploy:published', 'bundler:clean' to config/deploy.rb.

Downsides:

• If a rollback requires rebuilding a Gem such as Nokogiri, the rollback will take a while.
• In rare cases, if a gem that was used in the previously deployed version was yanked, rollback would entirely fail.

[mattbrictson]

@betesh Also, could you clarify the purpose behind capturing the bundle clean output and interpreting/reformatting it? Why not let Bundler's output go straight through to stdout, like we do for bundle install? Making assumptions about the output and parsing it makes the task more brittle and doesn't seem to provide much additional benefit.

[betesh]

@mattbrictson It's been a while but IIRC, I capture the output so that you can see the results even when your log level is higher than debug. This is helpful, as long as it's not too noisy, so I reformat it to keep it concise.

@will-in-wi I'm too busy at the momet to push changes of my own. I recommend forking my fork and submitting a PR to me. Once I merge your PR, it will show up on this PR. I did that once before here and it worked well.

One suggestion about your proposed documentation: I would give a little more context about why nokogiri in particular takes so long, and make it clear that it's one of several examples, but not all gems take that long.

[will-in-wi]

@betesh: I've added a docs PR: https://github.com/betesh/bundler/pull/1

@mattbrictson: Could you comment on what you'd prefer with the output capture? I can add a change to fix that if needed.

[betesh]

@will-in-wi I've merged your PR into mine, but feel free to open another PR if there's followup after @mattbrictson gives feedback

[mattbrictson]

Regarding the output, I think the capture is not needed at all. For example, if I add this task to my project and run it, I get plenty of self-explanatory output when using Airbrussh's default settings:

task "bundler:clean" do
  on release_roles(:all) do
    within current_path do
      execute :bundle, :clean
    end
  end
end

$ cap staging bundler:clean
00:00 bundler:clean
      01 bundle clean
      01 Removing unf_ext (0.0.7.1)
      01 Removing sprockets (3.6.1)
      01 Removing responders (2.2.0)
      01 Removing activerecord (4.2.6)
      01 Removing redis (3.2.2)
      01 Removing sidekiq (4.1.2)
[snip]
      01 Removing mime-types (2.99.2)
      01 Removing dotenv-rails (2.1.0)
      01 Removing actionpack (4.2.5.1)
      01 Removing commander (4.3.8)
    ✔ 01 deployer@xx.xx.xx.xx 16.848s

Adding the capture just complicates the implementation, IMHO. If Bundler's output is confusing or incomplete, that should be a PR for the Bundler project, not for Capistrano.

[betesh]

If Bundler's output is confusing or incomplete

It's not confusing or incomplete. In this particular context, I found it useful but unnecessarily verbose. I agree that it's a matter of opinion.

[will-in-wi]

@mattbrictson I added another PR without capture: #85

[mattbrictson]

Closed in favor of #85, which combines commits from this PR along with some modifications from @will-in-wi.