From 0126f6be1ec02236a861a0462df4d8aa26012c23 Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Fri, 10 Dec 2010 20:53:31 -0800 Subject: [PATCH] Don't wipe the timestamp when a target fails to redo. It's really a separate condition. And since we're not removing the target *file* in case of error - we update it atomically, and keeping it is better than losing it - there's no reason to wipe the timestamp in that case either. However, we do need to know that the build failed, so that anybody else (especially in a parallel build) who looks at that target knows that it died. So add a separate flag just for that. --- builder.py | 20 ++++++++++++++------ redo-ifchange.py | 2 ++ state.py | 31 +++++++++++++++++++++---------- t/flush-cache.sh | 3 ++- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/builder.py b/builder.py index 652c12d..06ccbb3 100644 --- a/builder.py +++ b/builder.py @@ -42,6 +42,12 @@ def _try_stat(filename): raise +class ImmediateReturn(Exception): + def __init__(self, rv): + Exception.__init__(self, "immediate return with exit code %d" % rv) + self.rv = rv + + class BuildJob: def __init__(self, t, sf, lock, shouldbuildfunc, donefunc): self.t = t # original target name, not relative to vars.BASE @@ -57,9 +63,12 @@ class BuildJob: t = self.t sf = self.sf tmpname = self.tmpname - if not self.shouldbuildfunc(t): - # target doesn't need to be built; skip the whole task - return self._after2(0) + try: + if not self.shouldbuildfunc(t): + # target doesn't need to be built; skip the whole task + return self._after2(0) + except ImmediateReturn, e: + return self._after2(e.rv) if (os.path.exists(t) and not os.path.exists(t + '/.') and not sf.is_generated): # an existing source file that was not generated by us. @@ -174,8 +183,7 @@ class BuildJob: else: unlink(tmpname) sf = self.sf - sf.stamp = None - sf.set_changed() + sf.set_failed() sf.save() f.close() if rv != 0: @@ -259,7 +267,7 @@ def main(targets, shouldbuildfunc): assert(lock.owned) if vars.DEBUG_LOCKS: log('%s (...unlocked!)\n' % _nice(t)) - if state.File(name=t).stamp == None: + if state.File(name=t).is_failed(): err('%s: failed in another thread\n' % _nice(t)) retcode[0] = 2 lock.unlock() diff --git a/redo-ifchange.py b/redo-ifchange.py index cd706b0..fb5af22 100755 --- a/redo-ifchange.py +++ b/redo-ifchange.py @@ -42,6 +42,8 @@ def dirty_deps(f, depth, max_changed): def should_build(t): f = state.File(name=t) + if f.is_failed(): + raise builder.ImmediateReturn(32) return dirty_deps(f, depth = '', max_changed = vars.RUNID) diff --git a/state.py b/state.py index dc3a7c5..4ecc254 100644 --- a/state.py +++ b/state.py @@ -59,6 +59,7 @@ def db(): " is_generated int, " " checked_runid int, " " changed_runid int, " + " failed_runid int, " " stamp, " " csum)") _db.execute("create table Deps " @@ -134,18 +135,19 @@ def relpath(t, base): class File(object): # use this mostly to avoid accidentally assigning to typos __slots__ = ['id', 'name', 'is_generated', - 'checked_runid', 'changed_runid', + 'checked_runid', 'changed_runid', 'failed_runid', 'stamp', 'csum'] def _init_from_cols(self, cols): (self.id, self.name, self.is_generated, - self.checked_runid, self.changed_runid, + self.checked_runid, self.changed_runid, self.failed_runid, self.stamp, self.csum) = cols def __init__(self, id=None, name=None, cols=None): if cols: return self._init_from_cols(cols) - q = ('select rowid, name, is_generated, checked_runid, changed_runid, ' + q = ('select rowid, name, is_generated, ' + ' checked_runid, changed_runid, failed_runid, ' ' stamp, csum ' ' from Files ') if id != None: @@ -175,21 +177,26 @@ class File(object): def save(self): _write('update Files set ' - ' is_generated=?, checked_runid=?, changed_runid=?, ' + ' is_generated=?, ' + ' checked_runid=?, changed_runid=?, failed_runid=?, ' ' stamp=?, csum=? ' ' where rowid=?', [self.is_generated, - self.checked_runid, self.changed_runid, + self.checked_runid, self.changed_runid, self.failed_runid, self.stamp, self.csum, self.id]) def set_checked(self): self.checked_runid = vars.RUNID - + def set_changed(self): debug2('BUILT: %r (%r)\n' % (self.name, self.stamp)) self.changed_runid = vars.RUNID + def set_failed(self): + debug2('FAILED: %r\n' % self.name) + self.failed_runid = vars.RUNID + def set_static(self): self.update_stamp() @@ -200,15 +207,19 @@ class File(object): self.stamp = newstamp self.set_changed() - def is_changed(self): - return self.changed_runid and self.changed_runid >= vars.RUNID - def is_checked(self): return self.checked_runid and self.checked_runid >= vars.RUNID + def is_changed(self): + return self.changed_runid and self.changed_runid >= vars.RUNID + + def is_failed(self): + return self.failed_runid and self.failed_runid >= vars.RUNID + def deps(self): q = ('select Deps.mode, Deps.source, ' - ' name, is_generated, checked_runid, changed_runid, ' + ' name, is_generated, ' + ' checked_runid, changed_runid, failed_runid, ' ' stamp, csum ' ' from Files ' ' join Deps on Files.rowid = Deps.source ' diff --git a/t/flush-cache.sh b/t/flush-cache.sh index 4e04d03..f44286d 100755 --- a/t/flush-cache.sh +++ b/t/flush-cache.sh @@ -4,5 +4,6 @@ echo ".timeout 5000" echo "pragma synchronous = off;" echo "update Files set checked_runid=null, " \ - " changed_runid=changed_runid-1;" + " changed_runid=changed_runid-1, " \ + " failed_runid=null;" ) | sqlite3 "$REDO_BASE/.redo/db.sqlite3"