nat-n / poethepoet

A task runner that works well with poetry.
https://poethepoet.natn.io/
MIT License
1.39k stars 56 forks source link

Including tasks from another file does not work with the poetry plugin when the poetry command is "" #199

Closed nickvalin1 closed 4 months ago

nickvalin1 commented 6 months ago

Minimal Example:

pyproject.toml:

[tool.poetry]
name = "poe-test"
version = "0.1.0"
description = ""
authors = ["Nick Valin <re@dacted.com>"]

[tool.poetry.dependencies]
python = "^3.12"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

[tool.poe]
poetry_command = ""
include = "tasks.toml"

tasks.toml

[tool.poe.tasks]
test = "python --version"

Run commands

poetry self add 'poethepoet[poetry_plugin]'
poetry install
poetry test

Expected result:

Poe => python --version
Python 3.12.1

Actual result: The command "test" does not exist.

I've been looking into the code, and I believe the root cause is that the tasks from an "include" statement are not registered until after cleo is monkey patched, meaning cleo cannot find the task. The reason it works if the poetry command is not an empty string is because the poetry command is registered as a task before cleo is monkey patched. Cleo can then find the poetry command, and the plugin takes back over and finds the task from the included file.

I see two ways this can be fixed, and I'm happy to create a PR for either, though I'd like some guidance from the maintainers on the favored solution:

  1. The easier solution is to register a dummy command (like "poe") when the poetry command is an empty string.

    if command_prefix == "":
    self._register_command(
        application,
        "",
        {"help": "Run poe tasks defined for this project"},
        "poe",
    )
    for task_name, task in poe_tasks.items():
        if task_name in COMMANDS:
            raise PoePluginException(
                f"Poe task {task_name!r} conflicts with a poetry command. "
                "Please rename the task or the configure a command prefix."
            )
        self._register_command(application, task_name, task)

    Then, when monkey patching cleo, add that dummy command to the token list:

    if prefix == "":
    tokens.insert(0, "poe")

    This allows cleo to do its thing, and the plugin can take back over and load the tasks from the included file.

  2. The (probably) harder solution is to refactor the plugin activation to load tasks from an included file before monkey patching cleo. I haven't tested this solution yet, so I'm not totally sure about the LOE.

Would love for some feedback from the maintainers about their preferred solution.

nat-n commented 6 months ago

Hi @nickvalin1, thanks for reporting this issue and working on a solution.

I checked out your branch locally to see how it runs and I've encountered a curious issue. It seems to direct all of poetry's own tasks to poe when used inside the empty_prefix test project!

I can't say I understand what's happening (or exactly how your fix it meant to work for that matter).

I'm currently in the process of refactoring how config is loaded (particularly includes), so I'll definitely return to this issue as part of that if it still isn't fixed.

nickvalin1 commented 4 months ago

@nat-n any update on the config refactor? If not, can you share a test case that would fail so I can fix my PR?

nat-n commented 4 months ago

Hi @nickvalin1, Yes! the refactor is merged on the development branch. Investigating this issue is on my TODO list leading up to the next release.

nat-n commented 4 months ago

Hi @nickvalin1,

I finally came back to look at your PR. I think approach 2 is cleaner, so I added that onto your PR. Apparently I accidentally closed your PR, so I created a new one here. The fix will be in the next release soon.

Thanks again for spending time on this and sorry for the slow turn around.

nat-n commented 4 months ago

This bug is now fixes with 0.26.0 🚀