guillermo / rake-hooks

Rake hooks let you add callbacks to rake tasks.
MIT License
70 stars 21 forks source link

Honor the prerequisites of hooked tasks. #7

Closed scottwb closed 12 years ago

scottwb commented 12 years ago

The current method of hooking tasks breaks two things about prerequisites of the task it hooks.

  1. The set of prerequisites returned from Rake::Task#prerequisites is cleared by before and after. While the invocation of the hooked task will still run those prerequisites, any other code that explicitly checks #prerequisites will break.
  2. It runs the before hook before the prerequisites. I would argue that the before hook should be run immediately before the task being hooked, NOT before the prerequisites. Otherwise, there may be some unpredictable ordering. For example, if the prerequisite had already been run by another dependent task, then the prerequisite happens before the before block. But if it had not been previously run, then the prerequisite happens after the before block.

Consider a more real-world case in the Rails world:

task :foo => :environment do
  # ...something that requires the Rails environment has been loaded
end

before :foo do
  # ...something that you want just before :foo, that also requires environment
end

In this case, rake foo will fail because it needs the :environment task to have been run first. However, this bug could go unnoticed if the :foo task was commonly run by another task that happens to also require the environment.

The current ordering of tasks is not predictable and can cause unexpected problems like this.

Solution

I have patched hook.rb to copy the prerequisites from the old_task to the new_task. This solves both the above problems. When the new_task is invoked, it's prerequisites are checked and invoked if necessary, then the before hook is called. Also, this keeps Rake::Task#prerequisites intact.

I have added some test cases to cover this (which fail without these patches), and have tested it with Rake 0.8.7 and Rake 0.9.2.2.

Cheers, -Scott