CmdAction with list does not set shell=False

[scr-oath]

cmd-action state

If action is a list of strings and instances of any Path class from pathlib, by default it will be executed without the shell (Popen argument shell=False).

However, if you trace through it, the call to Popen is still shell=True.

dodo.py

#!/usr/bin/env python3
import logging

import doit
from doit.action import CmdAction

DOIT_CONFIG = {
    'verbosity': 2,
    'default_tasks': ['check_results']
}


def task_check_results():
    def check_results(echo_as_list_in_CmdAction, echo_as_list_in_CmdAction_shell_False):
        failed = False
        logging.warning('echo_as_list_in_CmdAction = "%s"', echo_as_list_in_CmdAction)
        if echo_as_list_in_CmdAction != 'foo bar':
            failed = True

        logging.warning('echo_as_list_in_CmdAction_shell_False = "%s"', echo_as_list_in_CmdAction_shell_False)
        if echo_as_list_in_CmdAction_shell_False != 'foo bar':
            failed = True

        return not failed

    return {
        'actions': [check_results],
        'getargs': {
            'echo_as_list_in_CmdAction': ('echo_as_list_in_CmdAction', 'stdout'),
            'echo_as_list_in_CmdAction_shell_False': ('echo_as_list_in_CmdAction_shell_False', 'stdout'),
        },
        'uptodate': [False],
    }


def task_echo_as_list_in_CmdAction():
    return {
        'actions': [CmdAction(['echo', '-n', 'foo bar'], save_out='stdout')],
    }


def task_echo_as_list_in_CmdAction_shell_False():
    return {
        'actions': [CmdAction(['echo', '-n', 'foo bar'], save_out='stdout', shell=False)],
    }


def task_echo_as_list():
    return {
        'actions': [['echo', '-n', 'foo bar']],
    }


if __name__ == '__main__':
    doit.run(globals())

Environment

1. OS: macOS Monterey 12.0.0
2. python version: 3.9.5
3. doit version: 0.34.0

[scr-oath]

doit
.  echo_as_list_in_CmdAction_shell_False
foo bar.  echo_as_list_in_CmdAction

.  check_results
WARNING:root:echo_as_list_in_CmdAction = "
"
WARNING:root:echo_as_list_in_CmdAction_shell_False = "foo bar"
TaskFailed - taskid:check_results
Python Task failed: '<function task_check_results.<locals>.check_results at 0x7fa636f175e0>' returned False

[scr-oath]

Here's another way of showing it:

dodo.py

#!/usr/bin/env python3
import logging
from typing import Optional

import doit
from doit.action import CmdAction
from doit.task import Task

DOIT_CONFIG = {
    'verbosity': 2,
}


class CheckShell:
    def __init__(self, want_shell: bool = False):
        self.want_shell = want_shell
        self.task: Optional[Task] = None

    def configure_task(self, task: Task):
        self.task = task
        task.actions[0].save_out = 'stdout'

    def __call__(self) -> bool:
        if self.want_shell != self.task.actions[0].shell:
            logging.error(f'For task {self.task.name}: wanted {self.want_shell}, but first action was {self.task.actions[0].shell}')
        return True


def task_echo_as_list_in_CmdAction():
    return {
        'actions': [CmdAction(['echo', '-n', 'foo bar'], save_out='stdout')],
        'uptodate': [CheckShell(want_shell=False)],
    }


def task_echo_as_list_in_CmdAction_shell_False():
    return {
        'actions': [CmdAction(['echo', '-n', 'foo bar'], save_out='stdout', shell=False)],
        'uptodate': [CheckShell(want_shell=False)],
    }


def task_echo_as_list():
    return {
        'actions': [['echo', '-n', 'foo bar']],
        'uptodate': [CheckShell(want_shell=False)],
    }


if __name__ == '__main__':
    doit.run(globals())

Run

doit
ERROR:root:For task echo_as_list_in_CmdAction: wanted False, but first action was True
-- echo_as_list_in_CmdAction
-- echo_as_list_in_CmdAction_shell_False
-- echo_as_list

[schettino72]

If action is a list of strings and instances of any Path class from pathlib, by default it will be executed without the shell (Popen argument shell=False).

This applies ONLY when you do not explicitly use the CmdAction class.
With this interpretation your examples show correct behaviour, right?

Maybe the docs could be more clear about that.

[scr-oath]

Yep - I guess I figured it out, and yes, documentation would be very helpful to explicitly state that with a quick note that a constructed CmdAction does not infer its parameters (i.e if you want shell set to True of false, you have to pass it yourself).

And yes, this does seem to work with that new understanding; thanks for considering!

[scr-oath]

Even places like https://pydoit.org/dependencies.html?highlight=configure_task#saving-computed-values may need to link to that gotcha - the switch from ['cmd', 'arg1'] to CmdAction(['cmd', 'arg1'], save_out='stdout') may surprise and confuse folks when the behavior switches out from under them.