change in behavior to before/after hooks from 2.x to 3.x

[arronmabrey]

Hi @leehambley

I came across a surprise this morning when updating a few projects from 2.x to 3.4.0.

I didn't see it documented anywhere and wanted to check if it was intended or maybe a regression.

In my 2.x projects I had multiple callbacks defined for a single before hook definition.

before 'deploy:baz', 'deploy:foo', 'deploy:bar'

Running cap production deploy:baz

Under 2.15.5 the tasks that get run:

• deploy:foo
• deploy:bar
• deploy:baz

* executing `deploy:baz'
triggering before callbacks for `deploy:baz'
* executing `deploy:foo'
* executing `deploy:bar'

Under 3.4.0 the tasks that get run:

• deploy:foo
• deploy:baz

** Invoke deploy:baz (first_time)
** Invoke deploy:foo (first_time)
** Execute deploy:foo
** Execute deploy:baz

You can see that under 3.4.0 the 'deploy:bar' task gets silently skipped.

I'd expect that before/after hooks in 3.4.0 would do one of the following:

1. Behave the same as 2.x and run multiple callbacks that are defined with a single hook definition.
2. Raise an argument error that too many callbacks were passed to the before/after method, and now it only excepts a single callback per hook definition.

I'd prefer option (1), but understand that going from 2.x to 3.x is a good chance to make breaking api changes if needed.

Thanks,
-- Arron

[leehambley]

Our hooks implementation is now based on https://github.com/guillermo/rake-hooks, I admit that this behaviour is confusing (and probably a bug).

We had such poor test coverage of the old code, and I'd rarely ever seen people passing multiple tasks to those hok-setup functions.

[ssalat]

Ok, same here, a bit irritated why it then already made it in the latest release? I'm quite sure that this bug happens in 95% of all usages.

[leehambley]

Capistrano 3 does not document that before() or after() take many arguments. We'd take a patch, but we probably won't have time to work on this ourselves any time soon. It should be an easy fix (which is why I linked the original implementation)

[mattbrictson]

Capistrano 3.4 takes any extra before/after arguments and passes them to Rake::Task.define_task as task arguments. Anyone that depends on this 3.4 behavior will break if we interpret the extra arguments as additional task names instead.

I don't see a way to "fix" this without it being a breaking change.

Good news is this is definitely something we can detect and show a deprecation warning for, if we do eventually decide to do this in Capistrano 4.0.