From 34669fba65bd902a5430608d5cd4b20b7b691ddc Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Sat, 6 Oct 2018 00:14:02 -0400 Subject: [PATCH] Use os.lstat() instead of os.stat(). I think this aligns better with how redo works. Otherwise, if a.do creates a as a symlink, then changes to the symlink's *target* will change a's stat/stamp information without re-running a.do, which looks to redo like you modified a by hand, which causes it to stop running a.do altogether. With this change, modifications to a's target are okay, but they don't trigger any redo dependency changes. If you want that, then a.do should redo-ifchange on its symlink target explicitly. --- builder.py | 2 +- state.py | 2 +- t/201-fail/all.do | 1 + t/360-symlinks/.gitignore | 4 ++++ t/360-symlinks/a.do | 4 ++++ t/360-symlinks/all.do | 32 ++++++++++++++++++++++++++++++++ t/360-symlinks/b.do | 3 +++ t/360-symlinks/clean.do | 1 + 8 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 t/360-symlinks/.gitignore create mode 100644 t/360-symlinks/a.do create mode 100644 t/360-symlinks/all.do create mode 100644 t/360-symlinks/b.do create mode 100644 t/360-symlinks/clean.do diff --git a/builder.py b/builder.py index 5c6aa34..fe978fa 100644 --- a/builder.py +++ b/builder.py @@ -53,7 +53,7 @@ def _nice(t): def _try_stat(filename): try: - return os.stat(filename) + return os.lstat(filename) except OSError, e: if e.errno == errno.ENOENT: return None diff --git a/state.py b/state.py index 1943ca9..08ce5dd 100644 --- a/state.py +++ b/state.py @@ -275,7 +275,7 @@ class File(object): def read_stamp(self): try: - st = os.stat(os.path.join(vars.BASE, self.name)) + st = os.lstat(os.path.join(vars.BASE, self.name)) except OSError: return STAMP_MISSING if stat.S_ISDIR(st.st_mode): diff --git a/t/201-fail/all.do b/t/201-fail/all.do index 86dcd60..e82355a 100644 --- a/t/201-fail/all.do +++ b/t/201-fail/all.do @@ -10,4 +10,5 @@ rm -f fail touch fail ../flush-cache +# since we created this file by hand, fail.do won't run, so it won't fail. redo-ifchange fail >&/dev/null || exit 55 # expected to pass diff --git a/t/360-symlinks/.gitignore b/t/360-symlinks/.gitignore new file mode 100644 index 0000000..2b0acc8 --- /dev/null +++ b/t/360-symlinks/.gitignore @@ -0,0 +1,4 @@ +*.did +*.extra +a +b diff --git a/t/360-symlinks/a.do b/t/360-symlinks/a.do new file mode 100644 index 0000000..30b8320 --- /dev/null +++ b/t/360-symlinks/a.do @@ -0,0 +1,4 @@ +echo x >>a.did +rm -f $2.extra +echo foo >$2.extra +ln -s $2.extra $3 diff --git a/t/360-symlinks/all.do b/t/360-symlinks/all.do new file mode 100644 index 0000000..a5259e8 --- /dev/null +++ b/t/360-symlinks/all.do @@ -0,0 +1,32 @@ +rm -f a a.extra b b.did +d0="" +redo a +redo-ifchange b +d1=$(cat b.did) +[ "$d0" != "$d1" ] || exit 11 + +# b only rebuilds if a changes +../flush-cache +redo-ifchange b +d2=$(cat b.did) +[ "$d1" = "$d2" ] || exit 12 + +# forcibly changing a should rebuild b. +# a is already symlink to a.extra, but redo shouldn't care about the +# target of symlinks, so it shouldn't freak out that a.extra has changed. +# Anyway, b should still rebuild because a was rebuilt. +../flush-cache +redo a +redo-ifchange b +d3=$(cat b.did) +[ "$d2" != "$d3" ] || exit 13 + +# Explicitly check that changing a's symlink target (a.extra) does *not* +# trigger a rebuild of b, because b depends on the stamp of the symlink, +# not what the symlink points to. In redo, you declare dependencies on +# specific filenames, not the things they happen to refer to. +../flush-cache +touch a.extra +redo-ifchange b +d4=$(cat b.did) +[ "$d3" = "$d4" ] || exit 14 diff --git a/t/360-symlinks/b.do b/t/360-symlinks/b.do new file mode 100644 index 0000000..52159b0 --- /dev/null +++ b/t/360-symlinks/b.do @@ -0,0 +1,3 @@ +echo x >>b.did +redo-ifchange a +cat a >$3 diff --git a/t/360-symlinks/clean.do b/t/360-symlinks/clean.do new file mode 100644 index 0000000..fceeb8e --- /dev/null +++ b/t/360-symlinks/clean.do @@ -0,0 +1 @@ +rm -f *~ .*~ a b *.extra *.did