Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'after' is not doing every time what is expected #1064

Closed
dspaeth-faber opened this issue May 27, 2014 · 13 comments
Closed

'after' is not doing every time what is expected #1064

dspaeth-faber opened this issue May 27, 2014 · 13 comments

Comments

@dspaeth-faber
Copy link

after 'task1', 'my_task' is not every time executed at the end of the task.
For instance in a rake-file:

  task 'demo1' do
    puts "demo1"
  end

  task 'after' do
    puts "after demo1"
  end

  after 'demo1', 'after'

  task 'demo1' do
    puts 'more to do for demo1'
  end

# expected output
> demo1
> more to do for demo1
> after demo1

# real output
> demo1
> after demo1
> more to do for demo1

I don't know a solution at the moment. I think to get this issue fixed execute in rake application has to be extented, so that after tasks are executed at the end.

@seenmyfate
Copy link
Member

The current implementation of after is basically the same as Rake's task enhancements, and the order of the enhancement matters. In this case, I think you will see the behaviour you expect by ensuring the after call is defined below the enhancement to the demo1 task.

Whether or not this behaviour is generally correct is another question, but I think it does yield the expected results in the most common cases.

@dspaeth-faber
Copy link
Author

@seenmyfate You'r right with your workaround. The only question for me was, if

Whether or not this behaviour is generally correct is another question

With the given name I wouldn't expect the behavior.

@will-in-wi
Copy link
Contributor

I've never seen a task be redefined with more behavior. Does this still behave the same way with newest Rake, and is this a common technique? I'd expect the output to be:

more to do for demo1
after demo1

@leehambley
Copy link
Member

leehambley commented Oct 10, 2017 via email

@dspaeth-faber
Copy link
Author

@will-in-wi it's still part of rake. It's a shortcut for Rake::Task['xxx:xxx'].enhance . It behaves a mostly like Double-Colon Rules within make, but differ a little.

With #ehnhance rake enables you to add further prerequisites and more actions (make calls it recipe) to a task. This has 2 benefits in my point of view. First you can reuse every task defined within a gem and enhance this task at your will. Second you can specify actions more closely to the prerequisites. For instance instead of:

file 'a.x' => ['b.x', 'c.x'] do
     do_somthing_with 'b.x'
     do_somthing_with 'c.x'
end

you can write:

file 'a.x' => ['b.x'] do
     do_somthing_with 'b.x'
end

 file 'a.x' => [ 'c.x'] do
     do_somthing_with 'c.x'
end

@will-in-wi
Copy link
Contributor

Well, I've learned something new about rake and capistrano. I agree the behavior as currently defined is unexpected. I'd be in favor of merging a PR which corrects this. I might have time to take a look, but if someone else wants to take a look, go for it!

@will-in-wi
Copy link
Contributor

This test seems to be what is needed. It just needs to be made to pass.

will-in-wi@2c34ced

@dspaeth-faber
Copy link
Author

In my point of view this spec is correct. To get it pass can be difficult.

after and before use enhance to hook into the task execution order. before is not critical because it add's only a prerequisite. after can be dricky.

The first solution which comes into my mind is monkey patching Rake::Task#enhance. Rake::Task#define_task uses Rake::Task#enhance internally. So you can manipulate @actions which contains all actions.

But I don't know if changing the behavior of after is a breaking change to some people.

To me it doesn't matter any more. I'm out of ruby business for a year now.

@mattbrictson
Copy link
Member

Just to play devil's advocate: is it worth making a breaking change to "correct" this behavior? To be clear, it probably won't break projects that use vanilla Capistrano and don't re-open tasks (a pretty esoteric feature of Rake, IMO). However I suspect a large portion of the Capistrano community assembles their deployment scripts by taking snippets from gists, Stack Overflow answers, and plugins, many which are not actively maintained. It is possible these could break in ways that are surprising and difficult to debug.

If we wanted to be really cautious, we could do a deprecation/warning cycle, sort of like we did with invoke vs invoke!: #1686

Also consider this ticket was dormant since 2014. There doesn't seem to be a large number of people advocating for it.

@will-in-wi
Copy link
Contributor

I'm totally okay with closing this as wontfix. I agree that it does not seem like many people are having issues with it, and it technically might break something.

It does seem like it is unexpected behavior, however.

So I'm not sure I have much of an opinion. Closing is less work than fixing, so I tend toward that unless there is a reason to fix.

@leehambley
Copy link
Member

I think we came to the root of the issue thanks to @dspaeth-faber 's help. There's a trail of discussion, given that we all agree, I'm closing [wontfix] thanks all.

@dspaeth-faber
Copy link
Author

@mattbrictson I wouldn't call it an esoteric feature. When you use rake as a build tool, then you will like it. Further more when you use after and before you rely on this feature.

@leehambley & @will-in-wi

One more point I want to mention. The current implementation of after and before has one more drwaback. Imagine you use a third party gem. Every thing works great, except one Task.

The gem look's like this:

 namespace :my_gem do

   task :pre1 do
      puts "pre1"
   end

    task :after1 do
      puts "after1"
    end

    task :to_change do
      puts "the org task"
    end

    before :to_change, :pre1
    after :to_change, :after1

  end

You try to override the task you need with a different behavior via:

Rake::Task["my_gem:to_change"].clear

# override to_change
namespace :my_gem do
  task :to_change do
     puts "changed task"
  end
end

task :pre2 do
    puts "pre2"
end

task :after2 do
    puts "after2"
end

# add more dependencies
before "my_gem:to_change", :pre1
after "my_gem:to_change", :after1

Expected output:

   pre1
   pre2
   changed task
   after1
   after2

Real output:

  pre1
  changed task
  after1

In this case you have to check every before and after within my_gem and dependent gems, to get it right.

In my opinion I would deprecate after and before at all in favor of rakes own features.

@mattbrictson
Copy link
Member

Thanks for the additional example; that is a good illustration of what makes Capistrano vs Rake concepts confusing. If Capistrano were in a 0.x state, this would definitely be something I would consider changing. However given the maturity of the project and large number of existing users and plugins, we cannot change or deprecate the before and after DSL without a huge amount of pain.

Also, IMHO, the before/after DSL, while certainly flawed, is much more intuitive than Rake's system of declaring prerequisites and enhancements. Making Capistrano more similar to Rake is a laudable goal and would certainly reduce surprising edge cases, but I am not convinced it would overall make Capistrano easier to use or more approachable as a general-purpose deployment tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants