$3 and stdout no longer refer to the same file.

This is slightly inelegant, as the old style
	echo foo
	echo blah
	chmod a+x $3

doesn't work anymore; the stuff you wrote to stdout didn't end up in $3.
You can rewrite it as:
	exec >$3
	echo foo
	echo blah
	chmod a+x $3

Anyway, it's better this way, because now we can tell the difference between
a zero-length $3 and a nonexistent one.  A .do script can thus produce
either one and we'll either delete the target or move the empty $3 to
replace it, whichever is right.

As a bonus, this simplifies our detection of whether you did something weird
with overlapping changes to stdout and $3.
This commit is contained in:
Avery Pennarun 2010-12-11 00:29:04 -08:00
commit 59201dd7a0
9 changed files with 54 additions and 29 deletions

View file

@ -56,7 +56,8 @@ class BuildJob:
def __init__(self, t, sf, lock, shouldbuildfunc, donefunc): def __init__(self, t, sf, lock, shouldbuildfunc, donefunc):
self.t = t # original target name, not relative to vars.BASE self.t = t # original target name, not relative to vars.BASE
self.sf = sf self.sf = sf
self.tmpname = '%s.redo.tmp' % t self.tmpname1 = '%s.redo1.tmp' % t
self.tmpname2 = '%s.redo2.tmp' % t
self.lock = lock self.lock = lock
self.shouldbuildfunc = shouldbuildfunc self.shouldbuildfunc = shouldbuildfunc
self.donefunc = donefunc self.donefunc = donefunc
@ -66,7 +67,6 @@ class BuildJob:
assert(self.lock.owned) assert(self.lock.owned)
t = self.t t = self.t
sf = self.sf sf = self.sf
tmpname = self.tmpname
try: try:
if not self.shouldbuildfunc(t): if not self.shouldbuildfunc(t):
# target doesn't need to be built; skip the whole task # target doesn't need to be built; skip the whole task
@ -107,8 +107,9 @@ class BuildJob:
else: else:
err('no rule to make %r\n' % t) err('no rule to make %r\n' % t)
return self._after2(1) return self._after2(1)
unlink(tmpname) unlink(self.tmpname1)
ffd = os.open(tmpname, os.O_CREAT|os.O_RDWR|os.O_EXCL, 0666) unlink(self.tmpname2)
ffd = os.open(self.tmpname1, os.O_CREAT|os.O_RDWR|os.O_EXCL, 0666)
close_on_exec(ffd, True) close_on_exec(ffd, True)
self.f = os.fdopen(ffd, 'w+') self.f = os.fdopen(ffd, 'w+')
# this will run in the dofile's directory, so use only basenames here # this will run in the dofile's directory, so use only basenames here
@ -116,7 +117,7 @@ class BuildJob:
os.path.basename(dofile), os.path.basename(dofile),
os.path.basename(basename), # target name (extension removed) os.path.basename(basename), # target name (extension removed)
ext, # extension (if any), including leading dot ext, # extension (if any), including leading dot
os.path.basename(tmpname) # randomized output file name os.path.basename(self.tmpname2) # randomized output file name
] ]
if vars.VERBOSE: argv[1] += 'v' if vars.VERBOSE: argv[1] += 'v'
if vars.XTRACE: argv[1] += 'x' if vars.XTRACE: argv[1] += 'x'
@ -162,39 +163,37 @@ class BuildJob:
def _after1(self, t, rv): def _after1(self, t, rv):
f = self.f f = self.f
tmpname = self.tmpname
before_t = self.before_t before_t = self.before_t
after_t = _try_stat(t) after_t = _try_stat(t)
before_tmp = os.fstat(f.fileno()) st1 = os.fstat(f.fileno())
after_tmp = _try_stat(tmpname) st2 = _try_stat(self.tmpname2)
after_where = os.lseek(f.fileno(), 0, os.SEEK_CUR)
if after_t != before_t and not stat.S_ISDIR(after_t.st_mode): if after_t != before_t and not stat.S_ISDIR(after_t.st_mode):
err('%r modified %r directly!\n' % (self.argv[2], t)) err('%s modified %s directly!\n' % (self.argv[2], t))
err('...you should update $3 (a temp file) instead of $1.\n') err('...you should update $3 (a temp file) or stdout, not $1.\n')
rv = 206 rv = 206
elif after_tmp and before_tmp != after_tmp and before_tmp.st_size > 0: elif st2 and st1.st_size > 0:
err('%r wrote to stdout *and* replaced $3.\n' % self.argv[2]) err('%s wrote to stdout *and* created $3.\n' % self.argv[2])
err('...you should write status messages to stderr, not stdout.\n') err('...you should write status messages to stderr, not stdout.\n')
rv = 207 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 rv==0:
if os.path.exists(tmpname) and os.stat(tmpname).st_size: if st2:
# there's a race condition here, but if the tmpfile disappears os.rename(self.tmpname2, t)
# at *this* point you deserve to get an error, because you're os.unlink(self.tmpname1)
# doing something totally scary. elif st1.st_size > 0:
os.rename(tmpname, t) os.rename(self.tmpname1, t)
else: if st2:
unlink(tmpname) os.unlink(self.tmpname2)
else: # no output generated at all; that's ok
unlink(self.tmpname1)
unlink(t)
sf = self.sf sf = self.sf
sf.is_generated = True sf.is_generated = True
sf.update_stamp() sf.update_stamp()
sf.set_changed() sf.set_changed()
sf.save() sf.save()
else: else:
unlink(tmpname) unlink(self.tmpname1)
unlink(self.tmpname2)
sf = self.sf sf = self.sf
sf.set_failed() sf.set_failed()
sf.save() sf.save()

5
t/.gitignore vendored
View file

@ -11,3 +11,8 @@ test2.args
/makedir /makedir
/makedir.log /makedir.log
/chdir1 /chdir1
/silence
/silence.do
/touch1
/touch1.do
/deltest2

View file

@ -1,3 +1,4 @@
exec >$3
cat <<-EOF cat <<-EOF
gcc -Wall -o /dev/fd/1 -c "\$1" gcc -Wall -o /dev/fd/1 -c "\$1"
EOF EOF

View file

@ -1,3 +1,4 @@
exec >$3
cat <<-EOF cat <<-EOF
OUT="\$1" OUT="\$1"
shift shift

View file

@ -1,4 +1,5 @@
redo example/clean curse/clean deps/clean "space dir/clean" redo example/clean curse/clean deps/clean "space dir/clean"
rm -f c c.c c.c.c c.c.c.b c.c.c.b.b d mode1 makedir.log chdir1 rm -f c c.c c.c.c c.c.c.b c.c.c.b.b d mode1 makedir.log chdir1 \
rm -f hello [by]ellow *.o *~ .*~ CC LD passfail hello [by]ellow *.o *~ .*~ CC LD passfail silence silence.do \
rm -rf makedir touch1 touch1.do
rm -rf makedir

View file

@ -1,5 +1,6 @@
redo-ifchange config.sh redo-ifchange config.sh
. ./config.sh . ./config.sh
exec >$3
cat <<-EOF cat <<-EOF
redo-ifchange \$1.c redo-ifchange \$1.c
gcc -MD -MF \$3.deps.tmp -o \$3 -c \$1.c gcc -MD -MF \$3.deps.tmp -o \$3 -c \$1.c

7
t/silencetest.do Normal file
View file

@ -0,0 +1,7 @@
echo 'echo hello' >silence.do
redo silence
[ -e silence ] || exit 55
echo 'true' >silence.do
redo silence
[ ! -e silence ] || exit 66
rm -f silence.do

View file

@ -1,4 +1,5 @@
redo-ifchange all redo-ifchange all
./hello >&2 ./hello >&2
redo deltest deltest2 test.args test2.args passfailtest chdirtest \ redo deltest deltest2 test.args test2.args passfailtest chdirtest \
curse/test deps/test "space dir/test" modetest makedir2 curse/test deps/test "space dir/test" modetest makedir2 \
silencetest touchtest

9
t/touchtest.do Normal file
View file

@ -0,0 +1,9 @@
echo 'echo hello' >touch1.do
redo touch1
[ -e touch1 ] || exit 55
rm -f touch1
echo 'touch $3' >touch1.do
redo touch1
[ -e touch1 ] || exit 66
[ -z "$(cat touch1)" ] || exit 77
rm -f touch1.do