From 96faf9d5f0c705aa1d83e938a8ecd9059733c451 Mon Sep 17 00:00:00 2001
From: quix <quixoticsycophant@gmail.com>
Date: Mon, 7 Jun 2010 11:17:16 -0400
Subject: [PATCH] Fix for kite-shaped dep graphs; better workaround for
 FileTask#needed?

Based on a patch by Heath Kehoe (thanks)

(cherry picked from commit 609147e2fb4a4534a0f3c10813bd68035412b03d)

Conflicts:

	test/test_file_task.rb
---
 lib/rake/parallel.rb   |   9 +--
 test/test_file_task.rb | 167 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 167 insertions(+), 9 deletions(-)

diff --git a/lib/rake/parallel.rb b/lib/rake/parallel.rb
index 7f3da6be7..bceba17db 100644
--- a/lib/rake/parallel.rb
+++ b/lib/rake/parallel.rb
@@ -110,15 +110,8 @@ def invoke_parallel(*task_args)
       def invoke_with_call_chain_collector(task_args, new_chain, previous_chain)
         prereqs = invoke_prerequisites_collector(task_args, new_chain)
         parallel = application.parallel
-        if needed? or parallel.tasks[self]
+        if needed? or prereqs.any? { |p| parallel.tasks[p] }
           parallel.tasks[self] = [task_args, prereqs]
-          unless previous_chain == InvocationChain::EMPTY
-            #
-            # Touch the parent to propagate 'needed?' upwards.  This
-            # works because the recursion is depth-first.
-            #
-            parallel.tasks[previous_chain.value] = true
-          end
         end
       end
       
diff --git a/test/test_file_task.rb b/test/test_file_task.rb
index cd21789fa..84f331b11 100644
--- a/test/test_file_task.rb
+++ b/test/test_file_task.rb
@@ -17,7 +17,12 @@ class TestFileTask < Test::Unit::TestCase
   def setup
     Task.clear
     @runs = SerializedArray.new
-    FileUtils.rm_f FILES
+    FileUtils.rm_rf "testdata", :verbose => false
+    FileUtils.mkdir_p "testdata", :verbose => false
+  end
+
+  def teardown
+    FileUtils.rm_rf "testdata", :verbose => false
   end
 
   def test_create_dispersed_timed_files
@@ -119,6 +124,166 @@ def test_two_old_files_in_between
     assert_equal([ANCIENT_FILE, OLDFILE, MIDDLE_AGED_FILE], @runs)
   end
 
+  def test_old_file_in_between_with_missing_leaf
+    create_dispersed_timed_files(MIDDLE_AGED_FILE, OLDFILE)
+    sleep 1
+
+    file MIDDLE_AGED_FILE => OLDFILE do |t|
+      @runs << t.name
+      touch MIDDLE_AGED_FILE, :verbose => false
+    end
+    file OLDFILE => NEWFILE do |t|
+      @runs << t.name
+      touch OLDFILE, :verbose => false
+    end
+    file NEWFILE do |t|
+      @runs << t.name
+      touch NEWFILE, :verbose => false
+    end
+
+    Task[MIDDLE_AGED_FILE].invoke
+    assert_equal([NEWFILE, OLDFILE, MIDDLE_AGED_FILE], @runs)
+  end
+
+  def test_two_old_files_in_between_with_missing_leaf
+    create_dispersed_timed_files(MIDDLE_AGED_FILE, OLDFILE, ANCIENT_FILE)
+    sleep 1
+
+    file MIDDLE_AGED_FILE => OLDFILE do |t|
+      @runs << t.name
+      touch MIDDLE_AGED_FILE, :verbose => false
+    end
+    file OLDFILE => ANCIENT_FILE do |t|
+      @runs << t.name
+      touch OLDFILE, :verbose => false
+    end
+    file ANCIENT_FILE => NEWFILE do |t|
+      @runs << t.name
+      touch ANCIENT_FILE, :verbose => false
+    end
+    file NEWFILE do |t|
+      @runs << t.name
+      touch NEWFILE, :verbose => false
+    end
+
+    Task[MIDDLE_AGED_FILE].invoke
+    assert_equal([NEWFILE, ANCIENT_FILE, OLDFILE, MIDDLE_AGED_FILE], @runs)
+  end
+
+  def test_diamond_graph_with_missing_leaf
+    a, b, c, d = %w[a b c d].map { |n| "testdata/#{n}" }
+    create_timed_files(a, b, c)
+    sleep 1
+    
+    file a => [b, c] do
+      @runs << a
+      touch a
+    end
+    file b => d do
+      @runs << b
+      touch b
+    end
+    file c => d do
+      @runs << c
+      touch c
+    end
+    file d do
+      @runs << d
+      touch d
+    end
+
+    Task[a].invoke
+    assert_equal [a, b, c, d], @runs.sort
+  end
+
+  def test_diamond_graph_with_new_leaf
+    a, b, c, d = %w[a b c d].map { |n| "testdata/#{n}" }
+    create_timed_files(a, b, c)
+    sleep 1
+    touch d
+
+    file a => [b, c] do
+      @runs << a
+      touch a
+    end
+    file b => d do
+      @runs << b
+      touch b
+    end
+    file c => d do
+      @runs << c
+      touch c
+    end
+    file d do
+      @runs << d
+      touch d
+    end
+
+    Task[a].invoke
+    assert_equal [a, b, c], @runs.sort
+  end
+
+  def test_kite_graph_with_missing_leaf
+    a, b, c, d, e = %w[a b c d e].map { |n| "testdata/#{n}" }
+    create_timed_files(a, b, c, d)
+    sleep 1
+
+    file a => [b, c] do
+      @runs << a
+      touch a
+    end
+    file b => d do
+      @runs << b
+      touch b
+    end
+    file c => d do
+      @runs << c
+      touch c
+    end
+    file d => e do
+      @runs << d
+      touch d
+    end
+    file e do
+      @runs << e
+      touch e
+    end
+
+    Task[a].invoke
+    assert_equal [a, b, c, d, e], @runs.sort
+  end
+
+  def test_kite_graph_with_new_leaf
+    a, b, c, d, e = %w[a b c d e].map { |n| "testdata/#{n}" }
+    create_timed_files(a, b, c, d)
+    sleep 1
+    touch e
+
+    file a => [b, c] do
+      @runs << a
+      touch a
+    end
+    file b => d do
+      @runs << b
+      touch b
+    end
+    file c => d do
+      @runs << c
+      touch c
+    end
+    file d => e do
+      @runs << d
+      touch d
+    end
+    file e do
+      @runs << e
+      touch e
+    end
+
+    Task[a].invoke
+    assert_equal [a, b, c, d], @runs.sort
+  end
+
   # I have currently disabled this test.  I'm not convinced that
   # deleting the file target on failure is always the proper thing to
   # do.  I'm willing to hear input on this topic.