Correct usage of build_args for singularity runner.

Existing implementation in some corner cases overrides singularity runner parameters when it should not be doing so. One corner case is the following:

Using docker configuration to build the singularity image file. In this case, the `build_args` parameter is automatically set to `--fakeroot`
whenever it is detected the singularity runtime supports this. This is differet from case when there is singularity section in MLCube configuration
file in which case the `build_args` is correctly read from system settings file (default location is `${HOME}/mlcube.yaml`) or from MLCubec configuration
file.

This fix changes this behavior. The singularity runner will log the warning message whenever it does not override the build_args as it would do previously.

runners/mlcube_singularity/mlcube_singularity/singularity_client.py

@@ -8,6 +8,7 @@
 from pathlib import Path
 from shlex import shlex
 
+import omegaconf
 import requests
 import semver
 
@@ -236,21 +237,50 @@ class Client:
     """
 
     @classmethod
-    def from_env(cls) -> "Client":
+    def from_env(
+            cls, preferred_exec: t.Optional[str] = None, platform_name: t.Optional[str] = None
+    ) -> "Client":
+        """Create Singularity client trying different commands to run it, return first that succeeds.
+
+        Args:
+            preferred_exec: First executable to try (e.g., the one from system settings file or MLCube config).
+            platform_name: Try executable in system settings file for this platform. Has lower priority than
+                `preferred_exec` if both specified.
+        Return:
+            Instance of singularity client that this system can run.
+        """
+        # TODO sergey: what if there are multiple different ways to run singularity (like different wrapper scripts
+        #      with pre/post actions, and user wants to use this particular runner) - think about adding `platform` -
+        #      the one that user specified on a command line.
+        # TODO sergey: what if by trying to run it below we trigger these unwanted pre/post condition actions?
         # Check if system settings file contains information about singularity.
         executables = ["singularity", "sudo singularity", "apptainer", "sudo apptainer"]
 
-        system_settings = SystemSettings()
-        if "singularity" in system_settings.platforms:
-            executables = [
-                system_settings.platforms["singularity"]["singularity"]
-            ] + executables
-            logger.info(
-                "Client.from_env found singularity platform config in MLCube system settings file "
-                "(file=%s, platform=%s). Will try it first for detecting singularity CLI client.",
-                system_settings.path.as_posix(),
-                system_settings.platforms["singularity"],
-            )
+        def _add(_exec: str) -> None:
+            if _exec in executables:
+                executables.remove(_exec)
+            executables.insert(0, _exec)
+
+        # Check if preferred executable is provided, and is not one of those above.
+        if preferred_exec is not None:
+            logger.debug("Client.from_env adding preferred singularity executable '%s'.", preferred_exec)
+            _add(preferred_exec)
+
+        if platform_name:
+            system_settings = SystemSettings()
+            # TODO sergey: we can have other `singularity` and `apptainer` platforms named differently. Need probably to
+            #      look at `value -> runner` that contains actual runner type.
+            if platform_name in system_settings.platforms:
+                preconfigured_platform: omegaconf.DictConfig = system_settings.platforms[platform_name]
+                if "singularity" in preconfigured_platform:
+                    _add(preconfigured_platform["singularity"])
+                    logger.info(
+                        "Client.from_env found singularity platform config in MLCube system settings file "
+                        "(file=%s, platform=%s). Will try it first or second for detecting singularity CLI client.",
+                        system_settings.path.as_posix(),
+                        preconfigured_platform["singularity"],
+                    )
+
         logger.debug(
             "Client.from_env will try candidate executables in the following order "
             "(first available will be selected): %s",

runners/mlcube_singularity/mlcube_singularity/singularity_run.py

@@ -71,15 +71,15 @@ def merge(mlcube: DictConfig) -> None:
 
         # This maybe useful to automatically detect singularity executable
         try:
-            client: t.Optional[Client] = Client.from_env()
+            client: t.Optional[Client] = Client.from_env(mlcube.runner.singularity)
 
-            singularity_exec = s_cfg.singularity if "singularity" in s_cfg else mlcube.runner.singularity
-            client_singularity = " ".join(client.singularity)
-            if singularity_exec != client_singularity:
+            exec_from_cfg = s_cfg.singularity if "singularity" in s_cfg else mlcube.runner.singularity
+            exec_from_env = " ".join(client.singularity)
+            if exec_from_cfg != exec_from_env:
                 # Not updating for now since this is consistent with previous implementation.
                 logger.warning(
-                    "Config.merge singularity executable from config (%s) is not the one MLCube can run (%s).",
-                    singularity_exec, client_singularity
+                    "Config.merge singularity executable from configuration '(%s)' is not the one MLCube detected "
+                    "from environment '(%s)'.", exec_from_cfg, exec_from_env
                 )
         except ExecutionError:
             client = None
@@ -140,22 +140,30 @@ def merge(mlcube: DictConfig) -> None:
         # The idea is that we can use the remote docker image as a source for the build process, automatically
         # generating an image name in a local environment. Key here is that the source has a scheme - `docker://`
         # The --fakeroot switch is useful and is supported in singularity version >= 3.5
+        # TODO sergey: why do we even think that this case is special - why not rely on system settings that we can
+        #      automatically configure (e.g., detect correct (executable, fakeroot or no fakeroot) pairs?
         extra_args = {}
         if client is not None:
             if "singularity" not in s_cfg:
+                # TODO sergey: is this really a good idea to (maybe) replace exec from system settings if they differ?
                 extra_args["singularity"] = " ".join(client.singularity)
             if "build_args" not in s_cfg:
                 if client.supports_fakeroot():
-                    logger.info(
-                        "Config.merge [build_args] will use --fakeroot CLI switch (CLI client seems to be "
-                        "supporting it)."
-                    )
-                    extra_args["build_args"] = "--fakeroot"
+                    if 'fakeroot' not in mlcube.runner.build_args:
+                        logger.warning(
+                            "Config.merge build_args for runner configuration in system settings does not contain "
+                            "fakeroot, while singularity runtime seems to be supporting this. Previous implementation " 
+                            "would override this and would use '--fakeroot' in some corner cases (like this), but this "
+                            "was not intended behavior (a bug). If you believe your singularity runtime needs "
+                            "`--fakeroot`, please update your system settings file (for the platform you are "
+                            "using now)."
+                        )
                 else:
-                    logger.warning(
-                        "Config.merge [build_args] will not use --fakeroot CLI switch (CLI client too old or "
-                        "version unknown)"
-                    )
+                    if 'fakeroot' in mlcube.runner.build_args:
+                        logger.warning(
+                            "Config.merge you singularity client seems not to be supporting fakeroot, but the runner "
+                            "in system settings file is configured to use this switch."
+                        )
 
         build_file = "docker://" + d_cfg["image"]
         if "tar_file" in d_cfg: