Merge branch 'master' into redoconf

* master:
  builder.py: atomically replace the log for a given target.
  redo-ifchange regression: if REDO_LOG is not set, assume it's 1.
  Explicitly reject target/source filenames with newlines in them.
  If redo searched all the way up to /default.do, it would run ./default.do instead.
  Overridden files were accidentally getting reclassified as static.
  Certain redo post-build failures would still mark a target as built.
  minimal/do: remove dependency on 'seq' command.

bin/all.do

@@ -1,3 +1,3 @@
 exec >&2
-redo-ifchange ../redo/version/all ../redo/py list redo-sh
+redo-ifchange ../redo/version/all ../redo/py ../redo/sh list
 xargs redo-ifchange <list

redo/builder.py

@@ -170,7 +170,8 @@ class _BuildJob(object):
             # to produce hello.c, but we don't want that to happen if
             # hello.c was created by the end user.
             debug2("-- static (%r)\n" % t)
-            sf.set_static()
+            if not sf.is_override:
+                sf.set_static()
             sf.save()
             return self._finalize(0)
         sf.zap_deps1()
@@ -182,6 +183,8 @@ class _BuildJob(object):
                 return self._finalize(0)
             else:
                 err('no rule to redo %r\n' % t)
+                sf.set_failed()
+                sf.save()
                 return self._finalize(1)
         # There is no good place for us to pre-create a temp file for
         # stdout.  The target dir might not exist yet, or it might currently
@@ -226,10 +229,20 @@ class _BuildJob(object):
         firstline = open(os.path.join(dodir, dofile)).readline().strip()
         if firstline.startswith('#!/'):
             argv[0:2] = firstline[2:].split(' ')
-        # make sure to create the logfile *before* writing the meta() about it.
+        # make sure to create the logfile *before* writing the meta() about
-        # that way redo-log won't trace into an obsolete logfile.
+        # it.  that way redo-log won't trace into an obsolete logfile.
+        #
+        # We open a temp file and atomically rename it into place here.
+        # This guarantees that redo-log will never experience a file that
+        # gets truncated halfway through reading (eg.  if we build the same
+        # target more than once in a run).  Similarly, we don't want to
+        # actually unlink() the file in case redo-log is about to start
+        # reading a previous instance created during this session.  It
+        # should always see either the old or new instance.
         if env.v.LOG:
-            open(state.logname(self.sf.id), 'w')
+            lfd, lfname = tempfile.mkstemp(prefix='redo.', suffix='.log.tmp')
+            os.fdopen(lfd, 'w')
+            os.rename(lfname, state.logname(self.sf.id))
         dof = state.File(name=os.path.join(dodir, dofile))
         dof.set_static()
         dof.save()
@@ -419,7 +432,8 @@ class _BuildJob(object):
                 sf.csum = None
                 sf.update_stamp()
                 sf.set_changed()
-        else:
+        # rv might have changed up above
+        if rv:
             helpers.unlink(self.tmpname)
             sf = self.sf
             sf.set_failed()
@@ -474,6 +488,11 @@ def run(targets, shouldbuildfunc):
     else:
         selflock = myfile = me = None
 
+    for t in targets:
+        if '\n' in t:
+            err('%r: filenames containing newlines are not allowed.\n' % t)
+            return 204
+
     def cheat():
         if not selflock:
             return 0

redo/env.py

@@ -35,7 +35,7 @@ class Env(object):
         self.VERBOSE = _get_bool('REDO_VERBOSE', '')
         self.XTRACE = _get_bool('REDO_XTRACE', '')
         self.KEEP_GOING = _get_bool('REDO_KEEP_GOING', '')
-        self.LOG = _get_int('REDO_LOG', '')
+        self.LOG = _get_int('REDO_LOG', '1')
         self.LOG_INODE = _get('REDO_LOG_INODE', '')
         self.COLOR = _get_int('REDO_COLOR', '')
         self.PRETTY = _get_int('REDO_PRETTY', '')

redo/paths.py

@@ -32,7 +32,7 @@ def possible_do_files(t):
     # since t is an absolute path, dirbits[0] is always '', so we don't
     # need to count all the way down to i=0.
     for i in range(len(dirbits), 0, -1):
-        basedir = '/'.join(dirbits[:i])
+        basedir = '/'.join(dirbits[:i]) or '/'
         subdir = '/'.join(dirbits[i:])
         for dofile, basename, ext in _default_do_files(filename):
             yield (basedir, dofile,

t/100-args/all.do

@@ -1,2 +1,2 @@
-redo test.args test2.args passfailtest
+redo test.args test2.args passfailtest noargs/run
 . ../skip-if-minimal-do.sh

t/100-args/clean.do

@@ -1 +1 @@
-rm -f passfail *~ .*~
+rm -f passfail *~ .*~ */*~ */.*~ noargs/all

t/100-args/noargs/all.do

@@ -0,0 +1 @@
+echo RAN >$3

t/100-args/noargs/run.do

@@ -0,0 +1,3 @@
+rm -f all
+redo-ifchange  # should not default to 'all' since not running from top level
+[ ! -e all ] || exit 11

t/122-defaults-parent/.gitignore

@@ -0,0 +1 @@
+/*.log

t/122-defaults-parent/all.do

@@ -0,0 +1,32 @@
+rm -f x/shouldfail
+
+log=$PWD/$1.log
+
+expect_fail() {
+	local rv=$1
+	shift
+	if ("$@") >>$log 2>&1; then
+		cat "$log" >&2
+		echo "unexpected success:" "$@" >&2
+		return $rv
+	else
+		return 0
+	fi
+}
+
+# These should all fail because there is no matching .do file.
+# In previous versions of redo, it would accidentally try to use
+# $PWD/default.do even for ../path/file, which is incorrect.  That
+# could cause it to return success accidentally.
+
+rm -f "$log"
+cd inner
+expect_fail 11 redo ../x/shouldfail
+expect_fail 12 redo-ifchange ../x/shouldfail
+
+rm -f "$log"
+cd ../inner2
+expect_fail 21 redo ../x/shouldfail2
+expect_fail 22 redo-ifchange ../x/shouldfail2
+
+exit 0

t/122-defaults-parent/clean.do

@@ -0,0 +1,2 @@
+rm -f *~ .*~ */*~ */.*~ *.tmp */*.tmp x/shouldfail *.log */*.log
+

t/122-defaults-parent/inner/default.do

@@ -0,0 +1,2 @@
+echo "inner/default.do: PWD=$PWD '$1' '$2' '$3'" >&2
+echo SUSPICIOUS

t/122-defaults-parent/inner2/default.do

@@ -0,0 +1,2 @@
+echo "inner/default.do: PWD=$PWD '$1' '$2' '$3'" >&2
+# output file is left empty

t/350-deps/all.do

@@ -1,2 +1,2 @@
 redo test1 test2 ifchange-fail overwrite gentest doublestatic \
-	basic/test
+	basic/test override/all

t/350-deps/clean.do

@@ -1,3 +1,3 @@
-redo basic/clean
+redo basic/clean override/clean
 rm -f *~ .*~ *.count t1a overwrite overwrite[123] \
 	genfile2 genfile.log static.log

t/350-deps/override/.gitignore

@@ -0,0 +1,4 @@
+a
+b
+*.log
+stamp

t/350-deps/override/a.do

@@ -0,0 +1 @@
+echo hello-a >$3

t/350-deps/override/all.do

@@ -0,0 +1,34 @@
+exec >&2
+rm -f a b *.log stamp
+
+echo 1 >stamp
+redo b
+[ "$(cat b)" = "hello-a-1-b" ] || exit 11
+
+../../flush-cache
+echo 2 >stamp
+redo-ifchange b
+[ "$(cat b)" = "hello-a-1-b" ] || exit 21  # a unchanged; b not redone
+
+. ../../skip-if-minimal-do.sh
+
+# Unfortunately the test below depends on the specific wording of the
+# "override" warning message, of the form:
+#    redo: a - you modified it; skipping
+# That's because this is specifically a test that the warning message
+# gets generated.  I added that test because (of course) when we didn't
+# test it, the warning message accidentally got broken.  Oops.  If you
+# rephrase the message, you'll have to also change the test.
+
+../../flush-cache
+echo 3 >stamp
+echo over-a >a
+redo-ifchange b >$1.log 2>&1
+[ "$(cat b)" = "over-a-3-b" ] || exit 31  # a overwritten, b redone
+grep "a - " "$1.log" >/dev/null || exit 32  # expected a warning msg
+
+../../flush-cache
+echo 4 >stamp
+redo-ifchange b >$1.log 2>&1
+[ "$(cat b)" = "over-a-3-b" ] || exit 41  # a not changed, b not redone
+grep "a - " "$1.log" >/dev/null || exit 42  # still expect a warning msg

t/350-deps/override/b.do

@@ -0,0 +1,2 @@
+redo-ifchange a
+printf '%s-%s-b\n' "$(cat a)" "$(cat stamp)" >$3

t/350-deps/override/clean.do

@@ -0,0 +1 @@
+rm -f *~ .*~ stamp *.log a b