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.
This commit is contained in:
Avery Pennarun 2010-11-19 06:26:49 -08:00
commit c10875d7d7

View file

@ -103,7 +103,6 @@ def _build(t):
if not dofile: if not dofile:
raise BuildError('no rule to make %r' % t) raise BuildError('no rule to make %r' % t)
state.stamp(dofile) state.stamp(dofile)
unlink(t)
tmpname = '%s.redo.tmp' % t tmpname = '%s.redo.tmp' % t
unlink(tmpname) unlink(tmpname)
f = open(tmpname, 'w+') f = open(tmpname, 'w+')
@ -190,10 +189,10 @@ def main():
if state.stamped(t) == None: if state.stamped(t) == None:
err('%s: failed in another thread\n' % relp) err('%s: failed in another thread\n' % relp)
retcode[0] = 2 retcode[0] = 2
l.unlock()
else:
l.unlock() # build() reacquires it l.unlock() # build() reacquires it
jwack.start_job(t, lambda: build(t), lambda t,rv: done(t,rv)) jwack.start_job(t, lambda: build(t), lambda t,rv: done(t,rv))
else:
l.unlock()
return retcode[0] return retcode[0]