From 6dd70ff65e74bded4e3b46e2762b06796c92fce0 Mon Sep 17 00:00:00 2001
From: Josh Holtrop <jholtrop@gmail.com>
Date: Tue, 13 Jun 2017 19:37:41 -0400
Subject: [PATCH] raise error when circular dependencies are found - close #38

---
 build_tests/simple/error_circular_dependency.rb  |  5 +++++
 build_tests/simple/error_circular_dependency2.rb |  3 +++
 lib/rscons/job_set.rb                            |  6 ++++++
 spec/build_tests_spec.rb                         | 14 ++++++++++++++
 4 files changed, 28 insertions(+)
 create mode 100644 build_tests/simple/error_circular_dependency.rb
 create mode 100644 build_tests/simple/error_circular_dependency2.rb

diff --git a/build_tests/simple/error_circular_dependency.rb b/build_tests/simple/error_circular_dependency.rb
new file mode 100644
index 0000000..7cb9bc6
--- /dev/null
+++ b/build_tests/simple/error_circular_dependency.rb
@@ -0,0 +1,5 @@
+Rscons::Environment.new do |env|
+  env.Command("foo", "bar")
+  env.Command("bar", "baz")
+  env.Command("baz", "foo")
+end
diff --git a/build_tests/simple/error_circular_dependency2.rb b/build_tests/simple/error_circular_dependency2.rb
new file mode 100644
index 0000000..6ac13c8
--- /dev/null
+++ b/build_tests/simple/error_circular_dependency2.rb
@@ -0,0 +1,3 @@
+Rscons::Environment.new do |env|
+  env.Command("foo", "foo")
+end
diff --git a/lib/rscons/job_set.rb b/lib/rscons/job_set.rb
index 19132de..f3885ad 100644
--- a/lib/rscons/job_set.rb
+++ b/lib/rscons/job_set.rb
@@ -47,12 +47,18 @@ module Rscons
     # @return [nil, Hash]
     #   The next job to run.
     def get_next_job_to_run(targets_still_building)
+      targets_skipped = Set.new
       @jobs.keys.each do |target|
         skip = false
         (@jobs[target][0][:sources] + (@build_dependencies[target] || []).to_a).each do |src|
           if @jobs.include?(src)
+            if targets_skipped.include?(src) or (src == target)
+              # We have encountered a circular dependency.
+              raise "Circular build dependency for #{src}"
+            end
             # Skip this target because it depends on another target later in
             # the job set.
+            targets_skipped << target
             skip = true
             break
           end
diff --git a/spec/build_tests_spec.rb b/spec/build_tests_spec.rb
index e0465ac..c62069c 100644
--- a/spec/build_tests_spec.rb
+++ b/spec/build_tests_spec.rb
@@ -732,6 +732,20 @@ EOF
     expect(`./test-static.exe`).to match /Hi from one/
   end
 
+  it "raises an error for a circular dependency" do
+    test_dir("simple")
+    result = run_test(rsconsfile: "error_circular_dependency.rb")
+    expect(result.stderr).to match /Circular build dependency for (foo|bar|baz)/
+    expect(result.status).to_not eq 0
+  end
+
+  it "raises an error for a circular dependency where a build target contains itself in its source list" do
+    test_dir("simple")
+    result = run_test(rsconsfile: "error_circular_dependency2.rb")
+    expect(result.stderr).to match /Circular build dependency for foo/
+    expect(result.status).to_not eq 0
+  end
+
   context "backward compatibility" do
     it "allows a builder to call Environment#run_builder in a non-threaded manner" do
       test_dir("simple")