diff --git a/builder.py b/builder.py index ef6319a..e21478a 100644 --- a/builder.py +++ b/builder.py @@ -1,4 +1,4 @@ -import sys, os, random +import sys, os, random, errno import vars, jwack, state from helpers import log, log_, debug2, err, unlink, close_on_exec @@ -31,6 +31,16 @@ def _nice(t): return os.path.normpath(os.path.join(vars.PWD, t)) +def _try_stat(filename): + try: + return os.stat(filename) + except OSError, e: + if e.errno == errno.ENOENT: + return None + else: + raise + + class BuildJob: def __init__(self, t, lock, shouldbuildfunc, donefunc): self.t = t @@ -38,6 +48,7 @@ class BuildJob: self.lock = lock self.shouldbuildfunc = shouldbuildfunc self.donefunc = donefunc + self.before_t = _try_stat(self.t) def start(self): assert(self.lock.owned) @@ -96,13 +107,30 @@ class BuildJob: def _after(self, t, rv): try: - self._after1(t, rv) + rv = self._after1(t, rv) finally: self._after2(rv) def _after1(self, t, rv): f = self.f tmpname = self.tmpname + before_t = self.before_t + after_t = _try_stat(t) + before_tmp = os.fstat(f.fileno()) + after_tmp = _try_stat(tmpname) + after_where = os.lseek(f.fileno(), 0, os.SEEK_CUR) + if after_t != before_t: + err('%r modified %r directly!\n' % (self.argv[2], t)) + err('...you should update $3 (a temp file) instead of $1.\n') + rv = 206 + elif after_tmp and before_tmp != after_tmp and before_tmp.st_size > 0: + err('%r wrote to stdout *and* replaced $3.\n' % self.argv[2]) + err('...you should write status messages to stderr, not stdout.\n') + rv = 207 + elif after_where > 0 and after_tmp and after_tmp.st_size != after_where: + err('%r wrote differing data to stdout and $3.\n' % self.argv[2]) + err('...you should write status messages to stderr, not stdout.\n') + rv = 208 if rv==0: if os.path.exists(tmpname) and os.stat(tmpname).st_size: # there's a race condition here, but if the tmpfile disappears @@ -121,6 +149,7 @@ class BuildJob: else: if vars.VERBOSE or vars.XTRACE: log('%s (done)\n\n' % _nice(t)) + return rv def _after2(self, rv): try: diff --git a/t/deps/.gitignore b/t/deps/.gitignore index 9463dda..efb409b 100644 --- a/t/deps/.gitignore +++ b/t/deps/.gitignore @@ -1,2 +1,4 @@ t1a t2.count +overwrite +overwrite[123] diff --git a/t/deps/clean.do b/t/deps/clean.do index 7898c96..b18296c 100644 --- a/t/deps/clean.do +++ b/t/deps/clean.do @@ -1 +1,2 @@ -rm -f *~ .*~ *.count t1a +rm -f *~ .*~ *.count t1a overwrite overwrite[123] + diff --git a/t/deps/overwrite.do b/t/deps/overwrite.do new file mode 100644 index 0000000..8ae3639 --- /dev/null +++ b/t/deps/overwrite.do @@ -0,0 +1,4 @@ +redo overwrite1 2>&1 && exit 55 +redo overwrite2 2>&1 && exit 56 +redo overwrite3 2>&1 && exit 57 +exit 0 diff --git a/t/deps/overwrite1.do b/t/deps/overwrite1.do new file mode 100644 index 0000000..392f38f --- /dev/null +++ b/t/deps/overwrite1.do @@ -0,0 +1,2 @@ +# this shouldn't be allowed; we're supposed to write to $3, not $1 +echo >$1 diff --git a/t/deps/overwrite2.do b/t/deps/overwrite2.do new file mode 100644 index 0000000..dca62e5 --- /dev/null +++ b/t/deps/overwrite2.do @@ -0,0 +1,5 @@ +# this shouldn't be allowed; stdout is connected to $3 already, so if we +# replace it *and* write to stdout, we're probably confused. +echo hello world +rm -f $3 +echo goodbye world >$3 diff --git a/t/deps/overwrite3.do b/t/deps/overwrite3.do new file mode 100644 index 0000000..e5643fd --- /dev/null +++ b/t/deps/overwrite3.do @@ -0,0 +1,8 @@ +# we don't delete $3 here, we just truncate and overwrite it. But redo +# can detect this by checking the current file position of our stdout when +# we exit, and making sure it equals either 0 or the file size. +# +# If it doesn't, then we accidentally wrote to *both* stdout and a separate +# file, and we should get warned about it. +echo hello world +echo goodbye world >$3 diff --git a/t/deps/test.do b/t/deps/test.do index e0df597..6a30599 100644 --- a/t/deps/test.do +++ b/t/deps/test.do @@ -1,3 +1,3 @@ -redo test1 test2 ifchange-fail +redo test1 test2 ifchange-fail overwrite