webmap-import: Remove option --mvtdir-tmp.

Having a shared temporary directory, flock(2)'ed to avoid races, is a
great idea in theory but unfortunately doesn't work so well with
systemd.exec(5)'s ReadWritePaths settings since

    ReadWritePaths=/var/www/webmap/tiles
    ReadWritePaths=/var/www/webmap/tiles.tmp

creates multiple mount points pointing at the same file system and
rename(2)/renameat2(2) can't cope with that.  Quoting the manual:

    EXDEV  oldpath and newpath are not on the same mounted filesystem.
           (Linux permits a filesystem to be mounted at multiple points,
           but rename() does not work across different mount points, even
           if the same filesystem is mounted on both.)

So the options are to either use a single ReadWritePaths=/var/www/webmap,
or --mvtdir-tmp=/var/www/webmap/tiles/.tmp.  Both kind of defeat the
point (we'd in fact want to use --mvtdir-tmp=/var/tmp/webmap/tiles), so
we use mkdtemp(3) instead.

2 files changed, 17 insertions, 43 deletions

diff --git a/export_mvt.py b/export_mvt.py
index 73d2ac7..a929b78 100644
--- a/export_mvt.py
+++ b/export_mvt.py
@@ -23,7 +23,6 @@
 from os import O_RDONLY, O_WRONLY, O_CREAT, O_EXCL, O_CLOEXEC, O_DIRECTORY, F_OK
 import os
 from errno import EAGAIN
-from fcntl import flock, LOCK_EX
 import json
 import logging
 from pathlib import Path
@@ -278,9 +277,7 @@ def compress_brotli(path : str,
 # pylint: disable-next=too-many-branches, too-many-statements
 def exportMVT(ds : gdal.Dataset,
               layers : dict[str,dict[str,Any]],
-              tmpdir : Path|None,
               dst : Path,
-              mvtname : str = 'mvt',
               drvname : str = 'MVT',
               default_options : dict[str,Any]|None = None,
               tile_extension : str = '.pbf',
@@ -312,32 +309,18 @@ def exportMVT(ds : gdal.Dataset,
             mvtconf[export_layername] = x
             export_layers[export_layername] = (layername, export_layerdef)
 
-    if tmpdir is None:
-        # use a temporary directory in the parent to make sure we can
-        # atomically rename/exchange directories
-        tmpdir2 = tempfile.mkdtemp(prefix='.tmp.mvt-', dir=str(dst.parent))
-        logging.debug('Using "%s" as temporary directory for MVT', tmpdir2)
-    else:
-        # assume it already exists (it's shared by all invocation of the program)
-        tmpdir2 = str(tmpdir)
+    # use a sibling temporary directory to make sure we can atomically rename/exchange
+    # directories
+    tmpdir = tempfile.mkdtemp(prefix='.tmp.mvt-', dir=dst.parent)
+    logging.debug('Using "%s" as temporary directory for MVT', tmpdir)
 
-    dir_fd = os.open(tmpdir2, O_RDONLY|O_CLOEXEC|O_DIRECTORY)
+    mvtname = 'mvt'
+    dbname = 'db'
+    dir_fd = os.open(tmpdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY)
     try:
-        if tmpdir is not None:
-            logging.debug('flock("%s", LOCK_EX)', tmpdir2)
-            flock(dir_fd, LOCK_EX)
-            with os.scandir(dir_fd) as it:
-                if next(it, None) is not None:
-                    logging.warning('Temporary directory "%s" exists and is not empty',
-                                    str(tmpdir))
-
-        # createMVT() crashes if mvtname exists so that's also what we want here
-        tmpname = '.' + mvtname + '-tmp'
-        os.mkdir(tmpname, mode=0o700, dir_fd=dir_fd)
-
         start = time_monotonic()
+        os.mkdir(dbname, mode=0o700, dir_fd=dir_fd)
         basedir = Path(f'/proc/self/fd/{dir_fd}')
-        # gdal.create() raises an exception when path exists, and that's what we want
         dso = createMVT(drv, path=str(basedir.joinpath(mvtname)),
                         default_options=default_options,
                         options = {
@@ -349,7 +332,7 @@ def exportMVT(ds : gdal.Dataset,
                             'TYPE': 'overlay',
                             'BUFFER': 32,
                             'TILE_EXTENSION': tile_extension.removeprefix('.'),
-                            'TEMPORARY_DB': str(basedir.joinpath(tmpname).joinpath('tmp-mvt.db')),
+                            'TEMPORARY_DB': str(basedir.joinpath(dbname).joinpath('tmp.db')),
                             'CONF': json.dumps(mvtconf, ensure_ascii=False, separators=(',',':')),
                         })
 
@@ -380,7 +363,7 @@ def exportMVT(ds : gdal.Dataset,
             lyr_dst = None
             lyr_src = None
 
-        dso = None
+        dso = None # close MVT dataset
         start2 = time_monotonic()
         logging.info('Exported %d features to %d MVT layers in %s',
                      feature_count, layer_count, format_time(start2 - start))
@@ -438,18 +421,17 @@ def exportMVT(ds : gdal.Dataset,
 
     finally:
         lyr_dst = None
-        dso = None
+        dso = None # close MVT dataset
         srs = None
-        for p in (tmpname, mvtname):
+        for p in (dbname, mvtname):
             if os.access(p, F_OK, dir_fd=dir_fd, follow_symlinks=False):
-                logging.debug('rmtree("%s/%s")', tmpdir2, p)
+                logging.debug('rmtree("%s/%s")', tmpdir, p)
                 shutil.rmtree(p, dir_fd=dir_fd)
 
+        logging.debug('rmdir("%s")', tmpdir)
+        os.rmdir(tmpdir)
+
         try:
             os.close(dir_fd)
         except (OSError, ValueError):
             logging.exception('Could not close directory')
-
-        if tmpdir is None:
-            logging.debug('rmdir("%s")', tmpdir2)
-            os.rmdir(tmpdir2)
diff --git a/webmap-import b/webmap-import
index 1b9cde8..1a019a4 100755
--- a/webmap-import
+++ b/webmap-import
@@ -584,9 +584,6 @@ def main() -> NoReturn:
         help='optional directory for lock files to source paths')
     parser.add_argument('--mvtdir', default=None,
         help='optional directory for Mapbox Vector Tiles (MVT)')
-    parser.add_argument('--mvtdir-tmp', default=None,
-        help='temporary directory for Mapbox Vector Tiles (MVT); it must exists and be '
-             'on the same file system as the --mvtdir value')
     parser.add_argument('--compress-tiles', default=False, action='store_true',
         help='whether to compress Mapbox Vector Tiles (MVT) files')
     parser.add_argument('--force', default=False, action='store_true',
@@ -645,11 +642,9 @@ def main() -> NoReturn:
 
     if args.mvtdir is not None:
         args.mvtdir = Path(args.mvtdir)
-        if args.mvtdir == Path():
+        if args.mvtdir == Path(): # make sure it's not curdir as we don't want to exchange it
             raise RuntimeError('Invalid value for --mvtdir')
         args.mvtdir.parent.mkdir(parents=True, exist_ok=True)
-        if args.mvtdir_tmp is not None:
-            args.mvtdir_tmp = Path(args.mvtdir_tmp)
 
     if args.cachedir is not None:
         args.cachedir = Path(args.cachedir)
@@ -722,11 +717,8 @@ def main() -> NoReturn:
                          ', '.join(args.groupname) if args.groupname is not None else '*')
         else:
             exportMVT(dso, layers=export_layers,
-                      tmpdir=args.mvtdir_tmp,
                       dst=args.mvtdir,
                       default_options=config.get('vector-tiles', None),
-                      mvtname=(args.groupname[0] if args.groupname is not None and
-                                    len(args.groupname) == 1 else 'mvt'),
                       compress=args.compress_tiles)
 
         if dsoTransaction: