From c10875d7d74975e5c3cc8b256acd105ed1ae302c Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Fri, 19 Nov 2010 06:26:49 -0800 Subject: [PATCH] Don't unlink(t) before building; always rebuild the target if it was locked. The 'redo' command is supposed to *always* rebuild, not just if nobody else rebuilt it. (If you want "rebuild sometimes" behaviour, use redo-ifchange.) Thus, we shouldn't be short circuiting it just because a file was previously locked and then built okay. However, there's still a race condition in parallel builds, because redo-ifchange only checks the build stamp of each file once, then passes it to redo. Thus, we end up trying to build the same stuff over and over. This change actually makes it build *more* times, which seems dumb, but is one step closer to right. Doing this broke 'make test', however, because we were unlinking the target right before building it, rather than replacing it atomically as djb's original design suggested we should do. Thus, because of the combination of the above two bugs, CC would appear and then disappear even as people were trying to actually use it. Now it gets replaced atomically so it should at least work at all times... even though we're still building it more than once, which is incorrect. --- redo.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/redo.py b/redo.py index 517d52e..faba7e9 100755 --- a/redo.py +++ b/redo.py @@ -103,7 +103,6 @@ def _build(t): if not dofile: raise BuildError('no rule to make %r' % t) state.stamp(dofile) - unlink(t) tmpname = '%s.redo.tmp' % t unlink(tmpname) f = open(tmpname, 'w+') @@ -190,10 +189,10 @@ def main(): if state.stamped(t) == None: err('%s: failed in another thread\n' % relp) retcode[0] = 2 + l.unlock() + else: l.unlock() # build() reacquires it jwack.start_job(t, lambda: build(t), lambda t,rv: done(t,rv)) - else: - l.unlock() return retcode[0]