webmap-download: Simplify locking logic.

Don't use a lockdir by default, and always use lockfiles of the form
‘sha1_hex(path).lck’ if --lockdir is set.

1 files changed, 11 insertions, 21 deletions

diff --git a/webmap-download b/webmap-download
index 9d0861a..f87cddf 100755
--- a/webmap-download
+++ b/webmap-download
@@ -134,7 +134,7 @@ if __name__ == '__main__':
     parser.add_argument('--cachedir', default=None,
         help='destination directory for downloaded files (default: .)')
     parser.add_argument('--lockdir', default=None,
-        help='directory for lock files (default: value of --cachedir option)')
+        help='optional directory for lock files')
     parser.add_argument('--debug', action='store_true', help=argparse.SUPPRESS)
     parser.add_argument('--exit-code', action=argparse.BooleanOptionalAction,
         help='whether to exit with status 1 in case of download failures')
@@ -166,16 +166,8 @@ if __name__ == '__main__':
 
     # intentionally leave the dirfd open until the program terminates
     opendir_args = O_RDONLY|O_CLOEXEC|O_PATH|O_DIRECTORY
-    destdir_fd = None       if args.cachedir is None else os.open(args.cachedir, opendir_args)
-    lockdir_fd = destdir_fd if args.lockdir  is None else os.open(args.lockdir, opendir_args)
-
-    if ((destdir_fd is None and lockdir_fd is None) or
-            (destdir_fd is not None and lockdir_fd is not None and destdir_fd == lockdir_fd)):
-        lockdir_is_destdir = True
-    else:
-        destdir_st = os.fstat(destdir_fd)
-        lockdir_st = os.fstat(lockdir_fd)
-        lockdir_is_destdir = destdir_st.st_ino == lockdir_st.st_ino
+    destdir_fd = None if args.cachedir is None else os.open(args.cachedir, opendir_args)
+    lockdir_fd = None if args.lockdir  is None else os.open(args.lockdir, opendir_args)
 
     sessionRequests = requests.Session()
 
@@ -217,17 +209,14 @@ if __name__ == '__main__':
 
             # place an exclusive lock on a lockfile as the destination can be used by other layers
             # hence might be updated in parallel
-            if lockdir_is_destdir:
-                lockfile = dest + '.lck'
-            else:
-                # use a flat hierarchy when lockdir != destdir as this avoids leaving empty directories
-                # behind when removing left overs with tmpfiles.d(5)
+            if lockdir_fd is not None:
                 lockfile = sha1(dest.encode('utf-8')).hexdigest() + '.lck'
-            # use O_TRUNC to bump lockfile's mtime
-            lock_fd = os.open(lockfile, O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, mode=0o644, dir_fd=lockdir_fd)
+                # use O_TRUNC to bump lockfile's mtime
+                lock_fd = os.open(lockfile, O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, mode=0o644, dir_fd=lockdir_fd)
             try:
-                logging.debug('flock("%s", LOCK_EX)', lockfile)
-                flock(lock_fd, LOCK_EX)
+                if lockdir_fd is not None:
+                    logging.debug('flock("%s", LOCK_EX)', lockfile)
+                    flock(lock_fd, LOCK_EX)
                 try:
                     st = os.stat(dest, dir_fd=destdir_fd)
                 except (OSError, ValueError):
@@ -248,7 +237,8 @@ if __name__ == '__main__':
                     progress=pbar)
                 downloads.add(url)
             finally:
-                os.close(lock_fd)
+                if lockdir_fd is not None:
+                    os.close(lock_fd)
         except Exception:
             logging.exception('Could not download %s as %s', url, dest)
             if args.exit_code: