Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Script" tasks not working on windows? #51

Closed
tcamise-gpsw opened this issue Feb 7, 2022 · 9 comments
Closed

"Script" tasks not working on windows? #51

tcamise-gpsw opened this issue Feb 7, 2022 · 9 comments

Comments

@tcamise-gpsw
Copy link

tcamise-gpsw commented Feb 7, 2022

Hello. I am not able to get a "script" task to actually run on Windows (git bash). For sanity testing, I am trying to just run the "poe" task from this repo. It is configured in your pyproject.toml as:

  [tool.poe.tasks.poe]
  help   = "Execute poe from this repo (useful for testing)"
  script = "poethepoet:main

I am running it via:

poetry run poe poe

On windows, I get nothing:

[13:38:53] Tim@WINDOWS ~/temp/poethepoet/poethepoet (main)
$ poetry run poe poe
Poe => poe

On ubuntu, it works:

[13:38:26] tcamise@UBUNTU ~/temp/poethepoet (main)
$ poetry run poe poe
Poe => poe
Poe the Poet - A task runner that works well with poetry.
version 0.12.2

Result: No task specified.

USAGE
  poe [-h] [-v | -q] [--root PATH] [--ansi | --no-ansi] task [task arguments]

GLOBAL OPTIONS
  -h, --help     Show this help page and exit
  --version      Print the version and exit
  -v, --verbose  Increase command output (repeatable)
  -q, --quiet    Decrease command output (repeatable)
  -d, --dry-run  Print the task contents but don't actaully run it
  --root PATH    Specify where to find the pyproject.toml
  --ansi         Force enable ANSI output
  --no-ansi      Force disable ANSI output

CONFIGURED TASKS
  format         Run black on the code base
  clean          Remove generated files
  test           Run unit and feature tests
  test-quick     Run unit and feature tests, excluding slow ones
  types          Run the type checker
  lint           Run the linter
  style          Validate code style
  check-docs     Validate rst syntax in the docs
  check          Run all checks on the code base
  poe            Execute poe from this repo (useful for testing)

Am I doing something dumb here?

@nat-n
Copy link
Owner

nat-n commented Feb 7, 2022

Hi @tcamise-gpsw, thanks for reporting this.

I don't seem to be able to reproduce this issue, using git bash on Windows 11 running in VirtualBox.

I'm not sure what could be happening here, as long as "python" is on your path I'd expect it to work (see relevant code)

If you're able to debug it, or share anything potentially relevant about your environment then I'd be interested to see if there's something poethepoet can do to be more robust.

I'll leave this issue open for a while in case more details come to light.

@tcamise-gpsw
Copy link
Author

tcamise-gpsw commented Feb 8, 2022

I was just doing some shotgun print debugging and may have found something.

If I print out the cmd that is getting sent to Popen here it looks like this:

('C:\\Users\\Tim\\.poetry\\bin\\poetry.BAT', 'run', 'python', '-c', "import sys; from os import environ; from importlib import import_module; sys.argv = ['poe']; sys.path.append('src');\nimport_module('poethepoet').main()")

If I just send this from git bash via:

 C:\\Users\\Tim\\.poetry\\bin\\poetry.BAT run python -c "import sys; from os import environ; from importlib import import_module; sys.argv  = ['poe']; sys.path.append('src');\nimport_module('poethepoet').main()"

I get:

SyntaxError: unexpected character after line continuation character

If I remove the \n it works. The original test also works if I remove the \n from the script _handle_run

I have no idea what the consequences of removing that \n are. Any ideas?

@tcamise-gpsw
Copy link
Author

If it is acceptable, i made a PR here with just that change.

@nat-n
Copy link
Owner

nat-n commented Feb 8, 2022

Hi @tcamise-gpsw, thanks for looking into this!

I gotta say I'm still not sure what's going on here.

Unfortunately the \n as a line break is necessary in the case that self.format_args_class(self.named_args) actually returns something (which is only if the task has args configured), because it precedes a class.

Hence the tests fail in CI, did they work fir you locally?

I have a few ideas of how to tackle this, but since I don't know exactly what's required to reproduce it (and don't have direct access to a proper Window dev environment), I'll need some help verifying the solution(s). Are you game?

  1. My first idea would be to replace the line with f"{os.linesep}{self.format_args_class(self.named_args)} " in case this is something failing to recognise the escape code as a line break so it thinks it is a "continuation character" followed by the letter n. The space at the end might also help.
  2. If that doesn't help, then maybe the need for the line break can be avoided by avoiding the use of the class keyword here:
    f"class {classname}:\n"
    I guess something like
        return (
            f"type({classname},(object,),{"
            + ",".join(f'"{name}": {value!r}' for name, value in named_args.items())
            + "})"
        )

Does that work?

@tcamise-gpsw
Copy link
Author

Yes, I am game to continue debugging this. But I'll need to get back to you towards the end of the week.

@jasal82
Copy link

jasal82 commented Apr 28, 2022

I just ran across a similar issue in another context and unfortunately discovered this issue too late. It would have saved me several hours of print debugging. The symptom is that script tasks executed via poetry shell; poe task or poetry run poe task have the wrong sys.path set and thus are not finding modules installed in venv. I traced the cause to the Popen call utilizing the wrong python executable and found two possible solutions:

  1. setting shell=True (seems to open up more possibilities for resolving python).
  2. modifying the command in task/script.py to use sys.executable instead of just python, i.e.
        cmd = (
            sys.executable,
            "-c",
            "import sys; "
            "from os import environ; "
            "from importlib import import_module; "
            f"sys.argv = {argv!r}; sys.path.append('src');"
            f"\n{self.format_args_class(self.named_args)}"
            f"import_module('{target_module}').{function_call}",
        )

The first option is not a proper solution, so I would like to ask that the second option be implemented.

@nat-n
Copy link
Owner

nat-n commented Apr 29, 2022

Hi @jasal82, thanks for the feedback. I'm keen to get to the bottom of these issues.

Could you help me better understand exactly what is going wrong in your case?

  1. is this issue exclusively with script tasks?
  2. which python are you getting instead of the one from the poetry venv? Do you have an idea why?
  3. I assume you're on windows?
  4. Are you using poe installed by poetry inside your project?
  5. Which python version are you using? how is it installed? is it the same between system and the poetry venv?
  6. If you have the patience for more debugging could you share with my what the sys.path is vs what is should be?
  7. Could you share what you get if you add a print just before this line to show the full subprocess command?
  8. Is it possible that the target environment is not setup properly? (e.g. forgot to run poetry install)

Unfortunately I don't think always using sys.executable is a reliable solution. It'll work as long as poe is itself installed inside the poetry venv, but not if it's installed in another env or via pipx etc.

Constructing the command with just "python" should result in python being executed in the correct environment if the path is set correctly. Poe uses one of three different strategies for ensuring the path is setup depending on the circumstances:

  1. in the most common case poe actually runs poetry run python ... which should be the most reliable method, but has the highest performance overhead
  2. if poe detects that it is running in a poetry shell (or a recursive command) then it does nothing with the expectation that everything should already be in place
  3. if running a multistage task (e.g. a sequence of tasks) then poe attempts to set the necessary env vars to "activate the venv" for the subprocess. AFAIK this always works perfectly... but maybe it doesn't?

@tcamise-gpsw
Copy link
Author

Hello. So this is actually working for me now. That is, I can run all of the script tasks on Windows.

I did upgrade to the 0.13.1 so I'm assuming that this is what fixed it. In any case, feel free to close this if you want.

@nat-n
Copy link
Owner

nat-n commented Jun 21, 2022

I just pushed a minor refactor to the development branch that should avoid whatever the original issue was related to new lines in the generated python script.

Closing this issue, please comment if it doesn't seem to be solved after the next release.

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

No branches or pull requests

3 participants