Merge pull request #2155 from FCP-INDI/fix/S_parameter

🐛 Fix `-S` parameter

.github/Dockerfiles/C-PAC.develop-jammy.Dockerfile

@@ -45,7 +45,8 @@ RUN rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* /root/.cache/* \
     && chmod 777 $(ls / | grep -v sys | grep -v proc)
 ENV PYTHONUSERBASE=/home/c-pac_user/.local
 ENV PATH=$PATH:/home/c-pac_user/.local/bin \
-    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages
+    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages \
+    _SHELL=/bin/bash
 
 # set user
 WORKDIR /home/c-pac_user

.github/Dockerfiles/C-PAC.develop-lite-jammy.Dockerfile

@@ -46,7 +46,8 @@ RUN rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* /root/.cache/* \
     && chmod 777 $(ls / | grep -v sys | grep -v proc)
 ENV PYTHONUSERBASE=/home/c-pac_user/.local
 ENV PATH=$PATH:/home/c-pac_user/.local/bin \
-    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages
+    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages \
+    _SHELL=/bin/bash
 
 # set user
 WORKDIR /home/c-pac_user

CHANGELOG.md

@@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - A bug in which AWS S3 encryption was looked for in Nipype config instead of pipeline config (only affected uploading logs).
 - Restored `bids-validator` functionality.
+- Fixed empty `shell` variable in cluster run scripts.
 - A bug in which bandpass filters always assumed 1D regressor files have exactly 5 header rows.
 
 ### Removed

CPAC/pipeline/cpac_runner.py

@@ -14,6 +14,8 @@
 
 # You should have received a copy of the GNU Lesser General Public
 # License along with C-PAC. If not, see <https://www.gnu.org/licenses/>.
+"""Run C-PAC."""
+
 from multiprocessing import Process
 import os
 from time import strftime
@@ -23,6 +25,7 @@
 import yaml
 
 from CPAC.longitudinal_pipeline.longitudinal_workflow import anat_longitudinal_wf
+from CPAC.pipeline.utils import get_shell
 from CPAC.utils.configuration import check_pname, Configuration, set_subject
 from CPAC.utils.configuration.yaml_template import upgrade_pipeline_to_1_8
 from CPAC.utils.ga import track_run
@@ -100,10 +103,7 @@ def run_condor_jobs(c, config_file, subject_list_file, p_name):
 
 # Create and run script for CPAC to run on cluster
 def run_cpac_on_cluster(config_file, subject_list_file, cluster_files_dir):
-    """
-    Function to build a SLURM batch job submission script and
-    submit it to the scheduler via 'sbatch'.
-    """
+    """Build a batch job submission script and submit to the scheduler."""
     # Import packages
     import getpass
     import re
@@ -137,7 +137,6 @@ def run_cpac_on_cluster(config_file, subject_list_file, cluster_files_dir):
     time_limit = "%d:00:00" % hrs_limit
 
     # Batch file variables
-    shell = subprocess.getoutput("echo $SHELL")
     user_account = getpass.getuser()
     num_subs = len(sublist)
 
@@ -174,7 +173,7 @@ def run_cpac_on_cluster(config_file, subject_list_file, cluster_files_dir):
     # Set up config dictionary
     config_dict = {
         "timestamp": timestamp,
-        "shell": shell,
+        "shell": get_shell(),
         "job_name": "CPAC_" + pipeline_config.pipeline_setup["pipeline_name"],
         "num_tasks": num_subs,
         "queue": pipeline_config.pipeline_setup["system_config"]["on_grid"]["SGE"][

CPAC/pipeline/test/test_cpac_runner.py

@@ -1,13 +1,23 @@
 import os
+from pathlib import Path
 
 import pkg_resources as p
 import pytest
 
 from CPAC.pipeline.cpac_pipeline import load_cpac_pipe_config
 from CPAC.pipeline.cpac_runner import run_T1w_longitudinal
+from CPAC.pipeline.utils import get_shell
 from CPAC.utils.bids_utils import create_cpac_data_config
 
 
+def test_shell() -> None:
+    """Test that ``get_shell`` returns a path to an executable BASH."""
+    shell: str = get_shell()
+    assert shell.lower().endswith("bash"), "Default shell isn't BASH?"
+    assert Path(shell).exists(), "No default shell found."
+    assert os.access(shell, os.X_OK), "Default shell not executable."
+
+
 @pytest.mark.skip(reason="not a pytest test")
 def test_run_T1w_longitudinal(bids_dir, cfg, test_dir, part_id):
     sub_data_list = create_cpac_data_config(

CPAC/pipeline/utils.py

@@ -17,13 +17,30 @@
 """C-PAC pipeline engine utilities."""
 
 from itertools import chain
+import os
+import subprocess
+from typing import Optional
 
 from CPAC.func_preproc.func_motion import motion_estimate_filter
 from CPAC.utils.bids_utils import insert_entity
 
 MOVEMENT_FILTER_KEYS = motion_estimate_filter.outputs
 
 
+def get_shell() -> str:
+    """Return the path to default shell."""
+    shell: Optional[str] = subprocess.getoutput(
+        f"which $(ps -p {os.getppid()} -o comm=)"
+    )
+    if not shell:
+        try:
+            shell = os.environ["_SHELL"]
+        except KeyError:
+            msg = "Shell command not found."
+            raise EnvironmentError(msg)
+    return shell
+
+
 def name_fork(resource_idx, cfg, json_info, out_dct):
     """Create and insert entities for forkpoints.
 

Dockerfile

@@ -45,7 +45,8 @@ RUN rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* /root/.cache/* \
     && chmod 777 $(ls / | grep -v sys | grep -v proc)
 ENV PYTHONUSERBASE=/home/c-pac_user/.local
 ENV PATH=$PATH:/home/c-pac_user/.local/bin \
-    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages
+    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages \
+    _SHELL=/bin/bash
 
 # set user
 WORKDIR /home/c-pac_user

variant-lite.Dockerfile

@@ -46,7 +46,8 @@ RUN rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* /root/.cache/* \
     && chmod 777 $(ls / | grep -v sys | grep -v proc)
 ENV PYTHONUSERBASE=/home/c-pac_user/.local
 ENV PATH=$PATH:/home/c-pac_user/.local/bin \
-    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages
+    PYTHONPATH=$PYTHONPATH:$PYTHONUSERBASE/lib/python3.10/site-packages \
+    _SHELL=/bin/bash
 
 # set user
 WORKDIR /home/c-pac_user