Unconditionally store image path in POSIX notation in configuration

datalad_container/containers_add.py

@@ -6,8 +6,11 @@
 import logging
 import os
 import os.path as op
+from pathlib import (
+    Path,
+    PurePosixPath,
+)
 import re
-import sys
 from shutil import copyfile
 
 from datalad.cmd import WitlessRunner
@@ -22,6 +25,7 @@
 from datalad.support.exceptions import InsufficientArgumentsError
 from datalad.interface.results import get_status_dict
 
+from .utils import get_container_configuration
 from .definitions import definitions
 
 lgr = logging.getLogger("datalad.containers.containers_add")
@@ -205,8 +209,8 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
                 "Container names can only contain alphanumeric characters "
                 "and '-', got: '{}'".format(name))
 
-        cfgbasevar = "datalad.containers.{}".format(name)
-        if cfgbasevar + ".image" in ds.config:
+        container_cfg = get_container_configuration(ds, name)
+        if 'image' in container_cfg:
             if not update:
                 yield get_status_dict(
                     action="containers_add", ds=ds, logger=lgr,
@@ -219,16 +223,16 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
             if not (url or image or call_fmt):
                 # No updated values were provided. See if an update url is
                 # configured (currently relevant only for Singularity Hub).
-                url = ds.config.get(cfgbasevar + ".updateurl")
+                url = container_cfg.get("updateurl")
                 if not url:
                     yield get_status_dict(
                         action="containers_add", ds=ds, logger=lgr,
                         status="impossible",
                         message="No values to update specified")
                     return
 
-            call_fmt = call_fmt or ds.config.get(cfgbasevar + ".cmdexec")
-            image = image or ds.config.get(cfgbasevar + ".image")
+            call_fmt = call_fmt or container_cfg.get("cmdexec")
+            image = image or container_cfg.get("image")
 
         if not image:
             loc_cfg_var = "datalad.containers.location"
@@ -329,6 +333,7 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
             return
 
         # store configs
+        cfgbasevar = "datalad.containers.{}".format(name)
         if imgurl != url:
             # store originally given URL, as it resolves to something
             # different and maybe can be used to update the container
@@ -337,7 +342,8 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
         # force store the image, and prevent multiple entries
         ds.config.set(
             "{}.image".format(cfgbasevar),
-            op.relpath(image, start=ds.path),
+            # always store a POSIX path, relative to dataset root
+            str(PurePosixPath(Path(image).relative_to(ds.pathobj))),
             force=True)
         if call_fmt:
             ds.config.set(

datalad_container/containers_list.py

@@ -19,6 +19,8 @@
 from datalad.coreapi import subdatasets
 from datalad.ui import ui
 
+from datalad_container.utils import get_container_configuration
+
 lgr = logging.getLogger("datalad.containers.containers_list")
 
 
@@ -77,22 +79,7 @@ def __call__(dataset=None, recursive=False, contains=None):
                         yield c
 
         # all info is in the dataset config!
-        var_prefix = 'datalad.containers.'
-        containers = {}
-        for var, value in ds.config.items():
-            if not var.startswith(var_prefix):
-                # not an interesting variable
-                continue
-            var_comps = var[len(var_prefix):].split('.')
-            cname = var_comps[0]
-            ccfgname = '.'.join(var_comps[1:])
-            if not ccfgname:
-                continue
-
-            cinfo = containers.get(cname, {})
-            cinfo[ccfgname] = value
-
-            containers[cname] = cinfo
+        containers = get_container_configuration(ds)
 
         for k, v in containers.items():
             if 'image' not in v:

datalad_container/containers_remove.py

@@ -15,6 +15,8 @@
 from datalad.support.constraints import EnsureStr
 from datalad.interface.results import get_status_dict
 
+from datalad_container.utils import get_container_configuration
+
 lgr = logging.getLogger("datalad.containers.containers_remove")
 
 
@@ -24,15 +26,24 @@ class ContainersRemove(Interface):
     # first docstring line is used a short description in the cmdline help
     # the rest is put in the verbose help and manpage
     """Remove a known container from a dataset
+
+    This command is only removing a container from the committed
+    Dataset configuration (configuration scope ``branch``). It will not
+    modify any other configuration scopes.
+
+    This command is *not* dropping the container image associated with the
+    removed record, because it may still be needed for other dataset versions.
+    In order to drop the container image, use the 'drop' command prior
+    to removing the container configuration.
     """
 
     # parameters of the command, must be exhaustive
     _params_ = dict(
         dataset=Parameter(
             args=("-d", "--dataset"),
-            doc="""specify the dataset to query. If no dataset is given, an
-            attempt is made to identify the dataset based on the current
-            working directory""",
+            doc="""specify the dataset from removing a container. If no dataset
+            is given, an attempt is made to identify the dataset based on the
+            current working directory""",
             constraints=EnsureDataset() | EnsureNone()),
         name=Parameter(
             args=("name",),
@@ -42,7 +53,9 @@ class ContainersRemove(Interface):
         ),
         remove_image=Parameter(
             args=("-i", "--remove-image",),
-            doc="""if set, remove container image as well""",
+            doc="""if set, remove container image as well. Even with this flag,
+            the container image content will not be dropped. Use the 'drop'
+            command explicitly before removing the container configuration.""",
             action="store_true",
         ),
     )
@@ -59,12 +72,11 @@ def __call__(name, dataset=None, remove_image=False):
             action='containers_remove',
             logger=lgr)
 
-        section = 'datalad.containers.{}'.format(name)
-        imagecfg = '{}.image'.format(section)
+        container_cfg = get_container_configuration(ds, name)
 
         to_save = []
-        if remove_image and imagecfg in ds.config:
-            imagepath = ds.config.get(imagecfg)
+        if remove_image and 'image' in container_cfg:
+            imagepath = container_cfg['image']
             if op.lexists(op.join(ds.path, imagepath)):
                 for r in ds.remove(
                         path=imagepath,
@@ -78,10 +90,10 @@ def __call__(name, dataset=None, remove_image=False):
                     yield r
                 to_save.append(imagepath)
 
-        if section in ds.config.sections():
+        if container_cfg:
             ds.config.remove_section(
-                section,
-                where='dataset',
+                f'datalad.containers.{name}',
+                scope='branch',
                 reload=True)
             res['status'] = 'ok'
             to_save.append(op.join('.datalad', 'config'))

datalad_container/containers_run.py

@@ -81,6 +81,14 @@ def __call__(cmd, container_name=None, dataset=None,
         ds = require_dataset(dataset, check_installed=True,
                              purpose='run a containerized command execution')
 
+        # this following block locates the target container. this involves a
+        # configuration look-up. This is not using
+        # get_container_configuration(), because it needs to account for a
+        # wide range of scenarios, including the installation of the dataset(s)
+        # that will eventually provide (the configuration) for the container.
+        # However, internally this is calling `containers_list()`, which is
+        # using get_container_configuration(), so any normalization of
+        # configuration on-read, get still be implemented in this helper.
         container = None
         for res in find_container_(ds, container_name):
             if res.get("action") == "containers":