Ensure correct operation with read-only target dirs and .do file dirs.

Although I expect this is rather rare, some people may want to build in
a read-write subdir of a read-only tree.  Other than some confusing
error reporting, this works fine in redo after the recent changes to
temp file handling, but let's add a test to make sure it stays that
way.  The test found a bug in minimal/do, so let's fix that.

Reported-by: Jeff Stearns <jeff.stearns@gmail.com>
This commit is contained in:
Avery Pennarun 2018-12-13 12:58:56 +00:00
commit 39e017869d
5 changed files with 44 additions and 16 deletions

View file

@ -36,7 +36,7 @@ _dirsplit()
} }
# Like /usr/bin/dirname, but avoids a fork and uses _dirsplit semantics. # Like /usr/bin/dirname, but avoids a fork and uses _dirsplit semantics.
dirname() qdirname()
( (
_dirsplit "$1" _dirsplit "$1"
dir=${_dirsplit_dir%/} dir=${_dirsplit_dir%/}
@ -283,7 +283,7 @@ _run_dofile()
# done. # done.
_do() _do()
{ {
local dir="$1" target="$1$2" tmp="$1$2.redo.tmp" local dir="$1" target="$1$2" tmp="$1$2.redo.tmp" tdir=
local dopath= dodir= dofile= ext= local dopath= dodir= dofile= ext=
if [ "$_cmd" = "redo" ] || if [ "$_cmd" = "redo" ] ||
( [ ! -e "$target" -o -d "$target" ] && ( [ ! -e "$target" -o -d "$target" ] &&
@ -309,7 +309,8 @@ _do()
target=$(_relpath "$target" "$PWD") || return 98 target=$(_relpath "$target" "$PWD") || return 98
tmp=$(_relpath "$tmp" "$PWD") || return 97 tmp=$(_relpath "$tmp" "$PWD") || return 97
base=${target%$ext} base=${target%$ext}
[ ! -e "$DO_BUILT" ] || [ ! -d "$(dirname "$target")" ] || tdir=$(qdirname "$target")
[ ! -e "$DO_BUILT" ] || [ ! -w "$tdir/." ] ||
: >>"$target.did.tmp" : >>"$target.did.tmp"
# $qtmp is a temporary file used to capture stdout. # $qtmp is a temporary file used to capture stdout.
# Since it might be accidentally deleted as a .do file # Since it might be accidentally deleted as a .do file

View file

@ -374,23 +374,22 @@ class _BuildJob(object):
# FIXME: race condition here between updating stamp/is_generated # FIXME: race condition here between updating stamp/is_generated
# and actually renaming the files into place. There needs to # and actually renaming the files into place. There needs to
# be some kind of two-stage commit, I guess. # be some kind of two-stage commit, I guess.
if st1.st_size > 0: if st1.st_size > 0 and not st2:
# script wrote to stdout. Copy its contents to the tmpfile. # script wrote to stdout. Copy its contents to the tmpfile.
unlink(self.tmpname) unlink(self.tmpname)
try: try:
newf = open(self.tmpname, 'w') newf = open(self.tmpname, 'w')
except OSError, e: except IOError, e:
dnt = os.path.dirname(t) dnt = os.path.dirname(os.path.abspath(t))
if not os.path.exists(dnt): if not os.path.exists(dnt):
# This could happen, so report a simple error message # This could happen, so report a simple error message
# that gives a hint for how to fix your .do script. # that gives a hint for how to fix your .do script.
err('%s: target dir %r does not exist!\n' % (t, dnt)) err('%s: target dir %r does not exist!\n' % (t, dnt))
else: else:
# I don't know why this would happen, so raise the # This could happen for, eg. a permissions error on
# full exception if it ever does. # the target directory.
err('%s: save stdout to %s: %s\n' err('%s: copy stdout: %s\n' % (t, e))
% (t, self.tmpname, e)) rv = 209
raise
else: else:
self.outfile.seek(0) self.outfile.seek(0)
while 1: while 1:
@ -407,12 +406,10 @@ class _BuildJob(object):
# Atomically replace the target file # Atomically replace the target file
os.rename(self.tmpname, t) os.rename(self.tmpname, t)
except OSError, e: except OSError, e:
# Nowadays self.tmpname is in the same directory as # This could happen for, eg. a permissions error on
# t, so there's no very good reason for this to # the target directory.
# fail. Thus, raise the full exception if it ever
# does.
err('%s: rename %s: %s\n' % (t, self.tmpname, e)) err('%s: rename %s: %s\n' % (t, self.tmpname, e))
raise rv = 209
else: # no output generated at all; that's ok else: # no output generated at all; that's ok
unlink(t) unlink(t)
sf = self.sf sf = self.sf

1
t/204-readonly/.gitignore vendored Normal file
View file

@ -0,0 +1 @@
/rodir

25
t/204-readonly/all.do Normal file
View file

@ -0,0 +1,25 @@
[ -e rodir ] && chmod u+w rodir
[ -e rodir/rwdir ] && chmod u+w rodir/rwdir
rm -rf rodir
mkdir rodir rodir/rwdir
cd rodir
cat >default.ro1.do <<-EOF
chmod u+w "\$(dirname "\$1")"
echo 'redir' >\$3
EOF
cat >default.ro2.do <<-EOF
chmod u+w "\$(dirname "\$1")"
echo 'stdout'
EOF
# Check that:
# - redo works when the .do file is in a read-only directory.
# - redo works when the target is in a read-only directory that becomes
# writable only *after* launching the .do script. (For example, the .do
# might mount a new read-write filesystem in an otherwise read-only
# tree.)
chmod a-w . rwdir
redo rwdir/a.ro1
chmod a-w . rwdir
redo rwdir/a.ro2

4
t/204-readonly/clean.do Normal file
View file

@ -0,0 +1,4 @@
[ -e rodir ] && chmod u+w rodir
[ -e rodir/rwdir ] && chmod u+w rodir/rwdir
rm -rf rodir
rm -f *~ .*~