Skip to content

Conversation

@mkmkme
Copy link
Collaborator

@mkmkme mkmkme commented Oct 18, 2025

pyinvoke has several serious issues. First and foremost, it is very unsafe to pass any string arguments to run(), because it does not verify the string arguments at all. Given this function in tasks.py:

@task
def hello(c, name="user"):
    run(f"echo {name}")

you can get quite interesting results:

❯ invoke hello
user
❯ invoke hello --name="; date"

Sat Oct 18 11:41:56 CEST 2025

This is an issue number 2 in pyinvoke and I can't see it closed in any foreseeable future:
pyinvoke/invoke#2

Additionally, running linters from invoke.run() disables any colors from the linters which makes it less comfortable to use and caused us to force colors in some targets:
6111059

Even with that change, I have to pass additional -c any time I would like to run linters with colored output.

Considering these main issues, I have considered different alternatives to pyinvoke with one of them being tox:
#106

However, tox seems to be somewhat of an overkill for such a simple task, and also packaging was done via invoke as well. Therefore I've decided to pick make as an alternative.

This commit replaces tasks.py with a corresponding Makefile, removes invoke from dev dependencies (additionally removing click that was mentioned there explicitly for some reason) and replaces all the calls for invoke with calls for make.

This PR closes #62

Pull Request check-list

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

Description of change

pyinvoke has several serious issues. First and foremost, it is very
unsafe to pass any string arguments to `run()`, because it does not
verify the string arguments at all. Given this function in `tasks.py`:

```python
@task
def hello(c, name="user"):
    run(f"echo {name}")
```

you can get quite interesting results:

```sh
❯ invoke hello
user
❯ invoke hello --name="; date"

Sat Oct 18 11:41:56 CEST 2025

```

This is an issue number 2 in pyinvoke and I can't see it closed in any
foreseeable future:
pyinvoke/invoke#2

Additionally, running linters from `invoke.run()` disables any colors
from the linters which makes it less comfortable to use and caused us to
force colors in some targets:
6111059

Even with that change, I have to pass additional `-c` any time I would
like to run linters with colored output.

Considering these main issues, I have considered different alternatives
to pyinvoke with one of them being `tox`:
#106

However, `tox` seems to be somewhat of an overkill for such a simple
task, and also packaging was done via `invoke` as well. Therefore I've
decided to pick `make` as an alternative.

This commit replaces `tasks.py` with a corresponding `Makefile`, removes
`invoke` from `dev` dependencies (additionally removing `click` that was
mentioned there explicitly for some reason) and replaces all the calls
for `invoke` with calls for `make`.

Signed-off-by: Mikhail Koviazin <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.27%. Comparing base (b0d9b12) to head (ad10a3b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #233   +/-   ##
=======================================
  Coverage   76.27%   76.27%           
=======================================
  Files         129      129           
  Lines       33904    33904           
=======================================
  Hits        25860    25860           
  Misses       8044     8044           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mkmkme mkmkme mentioned this pull request Oct 18, 2025
5 tasks
@mkmkme mkmkme enabled auto-merge October 18, 2025 10:22
Copy link
Collaborator

@bogdanp05 bogdanp05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks way cleaner than before, thanks!

@mkmkme mkmkme merged commit 50b9e73 into main Oct 21, 2025
85 checks passed
@mkmkme mkmkme deleted the mkmkme/makefile branch October 21, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow for TESTARGS in invoke test

4 participants