From 887df98ead43ff340851adda9698926bba04fb82 Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Sat, 13 Oct 2018 01:30:42 -0400 Subject: [PATCH] builder.py: refresh the File object after obtaining the lock. We need to create the File object to get its f.id, then lock that id. During that gap, another instance of redo may have modified the file or its state data, so we have to refresh it. This fixes 'redo -j10 t/stress'. --- builder.py | 9 ++++++--- t/all.do | 3 --- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builder.py b/builder.py index 5bc1619..c14eb9c 100644 --- a/builder.py +++ b/builder.py @@ -121,9 +121,6 @@ class BuildJob: # For example, a rule called default.c.do could be used to try # to produce hello.c, but we don't want that to happen if # hello.c was created by the end user. - # FIXME: always refuse to redo any file that was modified outside - # of redo? That would make it easy for someone to override a - # file temporarily, and could be undone by deleting the file. debug2("-- static (%r)\n" % t) sf.set_static() sf.save() @@ -348,6 +345,12 @@ def main(targets, shouldbuildfunc): log('%s (locked...)\n' % _nice(t)) locked.append((f.id,t)) else: + # We had to create f before we had a lock, because we need f.id + # to make the lock. But someone may have updated the state + # between then and now. + # FIXME: separate obtaining the fid from creating the File. + # FIXME: maybe integrate locking into the File object? + f.refresh() BuildJob(t, f, lock, shouldbuildfunc, done).start() state.commit() assert(state.is_flushed()) diff --git a/t/all.do b/t/all.do index 7aa7c95..4e9f6bb 100644 --- a/t/all.do +++ b/t/all.do @@ -29,6 +29,3 @@ sed 's/\.do$//' | { redo "$d" done } - -# if all that worked run a repeated stress test to look for races -redo stress