Auto-globbing in cmd tasks makes it impossible to pass anything that looks like a glob to the called command

[TBBle]

I'm having an issue with the following in my pyproject.toml

[tool.poe.tasks]
run-tests = "python -m unittest discover -v ./tests/"

This works fine:

PS D:\PS\services\projects\scaler> poetry run poe run-tests
Poe => python -m unittest discover -v ./tests/
.... <Tests run>

However, this fails:

PS D:\PS\services\projects\scaler> poetry run poe run-tests -p "test_unit*.py"
Poe => python -m unittest discover -v ./tests/ -p
usage: python.exe -m unittest discover [-h] [-v] [-q] [--locals] [-f] [-c] [-b] [-k TESTNAMEPATTERNS] [-s START] [-p PATTERN] [-t TOP]
python.exe -m unittest discover: error: argument -p/--pattern: expected one argument

Specifically, "py_unit*.py" has disappeared. This argument is a glob-pattern to be passed as-is to Pytest, so should not be expanded. The bash-like behaviour of "use glob as-is if not matched" would be less-surprising, producing

Poe => python -m unittest discover -v ./tests/ -p "py_unit*.py"

The behaviour of course is closer-to-expected if I happen to be in 'tests'

PS D:\PS\services\projects\scaler\tests> poetry run poe run-tests -p "test_unit*.py"
Poe => python -m unittest discover -v ./tests/ -p test_unit_all_the_tests.py
... <Runs tests from that file>

i.e. when I first set this up, it appeared to be working, but only because I happen to have just one test file matching the glob here. It also will then fail to match any subdirectory-held test files that aren't also named test_unit_all_the_tests.py.

In bash, 'py_unit*.py' would mean "Don't expand this", but there's no way to communicate that to poethepoet as the quotes are already gone before it gets the argument.

[tool.poe.tasks]
run-unit-tests = "python -m unittest discover -v ./tests/ -p 'test_unit*.py'"

has the same problem, except it's always globbed from the top-level, not from the directory I happen to be in, leading to:

PS D:\PS\services\projects\scaler\tests> poetry run poe run-unit-tests -p "test_unit*.py"
Poe => python -m unittest discover -v ./tests/ -p -p test_unit_all_the_tests.py

where the same glob has expanded two different ways.

---

I'm not sure the 'auto-glob' behaviour makes sense, as there's literally no way for me to express "This is a glob, that isn't" through a cmd task, as it unconditionally tries to expand globs.

It's also a somewhat surprising behaviour, since if I wanted that, I'd use the shell runner, or e.g., a pwsh runner per #15, and let it do the globbing.

I don't see a particular justification (or documentation) for the "auto-glob" behaviour, so personally I would suggest removing it from cmd.py, along with the vestigates in shell.py. That would mean the rm -rf example in the Command Tasks list moves to shell, where it belongs, and is rewritten to use a bash glob, instead of a Python glob, i.e. there's no ** in bash.

If it's preferred to be kept, then having a way for the task to specify "No, do not auto-glob my commands" would be handy. I'd just use that all the time, so that poetry run poe X Y Z is literally poetry run <definition of X> Y Z as I would expect.

As another argument towards dropping auto-globbing, I've just realised this would make grep or sls nigh-unusable here, as a regexp on the command line will always be globbed-and-deleted, unless you're very unlucky and it globs into an existing filename, which would not have matched the regexp.

[nat-n]

Thanks for the feedback. The existing behaviour does seem problematic. The auto globbing was a very early feature from when I was still trying to avoid having something like shell tasks and I guess it wasn't that well thought through.

Looking at it now it seems a better way to achieve what I wanted would be just to set shell=True on the Popen call, rather than reimplementing part of the shell behaviour 🤨. One of the reasons I recall not doing this originally was that I'm not so sure whether it would behave consistently on windows (and I don't have ready access to a windows dev setup), but I guess that's what functional tests are for :P.

I should look into this some more. Or maybe make the globbing behaviour opt in as you suggest, though I think there's value in not having to use "shell" tasks for most things due to the complexity that introduces à la #15 .

[TBBle]

I kind of expected, similar e.g., to Dockerfile CMD, that cmd vs shell would differ only in the shell parameter to Popen. These extra behaviours (cmd-type applies Python's glob, shell hunts really hard for bash of some kind, no matter the local platform) have been a surprise to me.

I'm not keen on cmd-type using Popen( ..., shell=True ) simply because that implies another cmd instance (per $COMSPEC) in the chain, and that's approximately never what I want. And unlike POSIX, there's no way to override that (according to the docs the executable parameter is only for POSIX). I really just want cmd-type to be Popen(args_from_task + args_from_sysargv, shell=False), so it does exactly what I wrote.

The current shell approach is better than Popen( ..., shell=True), as it's "bash or fail", but per #15 it probably needs to fail more easily than it does... And it probably needs to be clearer that it's not "system shell" but "bash". Which is why I suggested in #15 that shell isn't the right name for it: it implied to me that it was just Popen( ..., shell=True), and the examples for shell certainly matched, since many of the cmd examples appear to assume an underlying POSIX system too.

Supporting a shell binary override for shell types would probably help, as then you'd be able to specify exactly what to script against, and shell type could be used for, e.g. "Powershell scripts" as well. Or cmd scripts if you're desperate and only need to support Windows, I guess. It would also mean "default shell is bash" is surfaced in the docs, as part of documenting the override for it.

That would probably also be a good opportunity to get rid of the $SHELL check, since that's not really promised to be bash and kind-of steps away from the idea of a "fixed, known environment that works the same everywhere" that we get from Poetry.

It might be better to check if we are running under bash, and then try /usr/bin/bash, and then /usr/bin/sh, and then /bin/sh, and then give up. It's only very recently on recent Windows 10 where you could run Windows-native 'python.exe' under bash from recent gitforwindows or MSYS2, and get the right results; WSL would be right-out, and it's the most-likely bash.exe in the Windows path by default.

Particularly for me, the main use-case I have for poethepoet is to replace the various .bat and .sh scripts that have grown up to wrap the various python invocations my team uses, with platform-agnostic versions, ideally stored in pyproject.toml with the rest of the project definition.

[nat-n]

After taking some time to analyse the problem I think you're right that it doesn't make sense to mess with args passed from the user's shell, and in general quoting an arg should disable glob expansion for that arg. I'll fix it accordingly, which I hope will address your pain point.
 
Thanks also for taking the time to share your perspective. The shell task needs some refinement, though I think there's value in the "simple" cmd task having a little shell-like magic for convenience.