From 7dd63efb376cc01c0cecb186192b1430adae33a1 Mon Sep 17 00:00:00 2001
From: "Robert L. Bocchino Jr" <bocchino@icloud.com>
Date: Sun, 27 Nov 2016 23:35:28 -0800
Subject: [PATCH] Add cyclic dependence detection.

If a depends on b which depends on a, redo would just freeze.  Now it
aborts with a somewhat helpful error message.

[Updated by apenwarr for coding style and to add a test.]
---
 builder.py                 |  9 ++++++++-
 state.py                   |  6 ++++++
 t/355-deps-cyclic/a.do     |  1 +
 t/355-deps-cyclic/all.do   |  2 ++
 t/355-deps-cyclic/b.do     |  1 +
 t/355-deps-cyclic/clean.do |  1 +
 vars.py                    | 11 +++++++++++
 vars_init.py               |  2 ++
 8 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 t/355-deps-cyclic/a.do
 create mode 100644 t/355-deps-cyclic/all.do
 create mode 100644 t/355-deps-cyclic/b.do
 create mode 100644 t/355-deps-cyclic/clean.do

diff --git a/builder.py b/builder.py
index c8197b7..83211fc 100644
--- a/builder.py
+++ b/builder.py
@@ -206,6 +206,7 @@ class BuildJob:
         os.environ['REDO_PWD'] = state.relpath(newp, vars.STARTDIR)
         os.environ['REDO_TARGET'] = self.basename + self.ext
         os.environ['REDO_DEPTH'] = vars.DEPTH + '  '
+        vars.add_lock(str(self.lock.fid))
         if dn:
             os.chdir(dn)
         os.dup2(self.f.fileno(), 1)
@@ -378,7 +379,13 @@ def main(targets, shouldbuildfunc):
                 # be released; but we should never run get_token() while
                 # holding a lock, or we could cause deadlocks.
                 jwack.release_mine()
-                lock.waitlock()
+                try:
+                    lock.waitlock()
+                except state.CyclicDependencyError:
+                    err('cyclic dependency while building %s\n' % _nice(t))
+                    jwack.get_token(t)
+                    retcode[0] = 208
+                    return retcode[0]
                 lock.unlock()
                 jwack.get_token(t)
                 lock.trylock()
diff --git a/state.py b/state.py
index cf0c643..85536de 100644
--- a/state.py
+++ b/state.py
@@ -11,6 +11,9 @@ STAMP_DIR='dir'     # the stamp of a directory; mtime is unhelpful
 STAMP_MISSING='0'   # the stamp of a nonexistent file
 
 
+class CyclicDependencyError(Exception): pass
+
+
 def _connect(dbfile):
     _db = sqlite3.connect(dbfile, timeout=TIMEOUT)
     _db.execute("pragma synchronous = off")
@@ -347,6 +350,9 @@ class Lock:
 
     def waitlock(self):
         assert(not self.owned)
+        if str(self.fid) in vars.get_locks():
+            # Lock already held by parent: cyclic dependence
+            raise CyclicDependencyError
         fcntl.lockf(self.lockfile, fcntl.LOCK_EX, 0, 0)
         self.owned = True
             
diff --git a/t/355-deps-cyclic/a.do b/t/355-deps-cyclic/a.do
new file mode 100644
index 0000000..1cdbf46
--- /dev/null
+++ b/t/355-deps-cyclic/a.do
@@ -0,0 +1 @@
+redo-ifchange b
diff --git a/t/355-deps-cyclic/all.do b/t/355-deps-cyclic/all.do
new file mode 100644
index 0000000..a835661
--- /dev/null
+++ b/t/355-deps-cyclic/all.do
@@ -0,0 +1,2 @@
+. ../skip-if-minimal-do.sh
+! redo a >/dev/null 2>&1 || exit 204
diff --git a/t/355-deps-cyclic/b.do b/t/355-deps-cyclic/b.do
new file mode 100644
index 0000000..b1b976e
--- /dev/null
+++ b/t/355-deps-cyclic/b.do
@@ -0,0 +1 @@
+redo-ifchange a
diff --git a/t/355-deps-cyclic/clean.do b/t/355-deps-cyclic/clean.do
new file mode 100644
index 0000000..c5bc950
--- /dev/null
+++ b/t/355-deps-cyclic/clean.do
@@ -0,0 +1 @@
+rm -f *~ a b
diff --git a/vars.py b/vars.py
index 77156ee..25870fa 100644
--- a/vars.py
+++ b/vars.py
@@ -28,3 +28,14 @@ os.environ['REDO_UNLOCKED'] = ''  # not inheritable by subprocesses
 
 NO_OOB = os.environ.get('REDO_NO_OOB', '') and 1 or 0
 os.environ['REDO_NO_OOB'] = ''    # not inheritable by subprocesses
+
+
+def get_locks():
+  """Get the list of held locks."""
+  return os.environ.get('REDO_LOCKS', '').split(':')
+
+def add_lock(name):
+  """Add a lock to the list of held locks."""
+  locks = set(get_locks())
+  locks.add(name)
+  os.environ['REDO_LOCKS'] = ':'.join(list(locks))
diff --git a/vars_init.py b/vars_init.py
index 47821d1..859dfe8 100644
--- a/vars_init.py
+++ b/vars_init.py
@@ -34,3 +34,5 @@ def init(targets):
 
         import state
         state.init()
+
+    os.environ['REDO_LOCKS'] = os.environ.get('REDO_LOCKS', '')