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

poetry run does not relay exit code #2369

Closed
2 tasks done
pawamoy opened this issue Apr 30, 2020 · 12 comments
Closed
2 tasks done

poetry run does not relay exit code #2369

pawamoy opened this issue Apr 30, 2020 · 12 comments
Labels
kind/bug Something isn't working as expected

Comments

@pawamoy
Copy link

pawamoy commented Apr 30, 2020

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.

Issue

Poetry does not relay the exit code of a script (as in from Poetry's scripts section) when ran with poetry run.

To reproduce, run the following snippet or check the repo directly: https://github.com/pawamoy/poetry-issue-2369

$ git clone https://github.com/pawamoy/poetry-issue-2369
$ cd poetry-issue-2369
$ poetry install -v
$ poetry shell
$ poetry-issue-2369
$ echo $?
1
$ exit
$ poetry run poetry-issue-2369
$ echo $?
0

Additional information:

$ poetry run which poetry-issue-2369
/home/pawamoy/.cache/pypoetry/virtualenvs/poetry-issue-2369-Q6VvSue2-py3.8/bin/poetry-issue-2369

$ cat /home/pawamoy/.cache/pypoetry/virtualenvs/poetry-issue-2369-Q6VvSue2-py3.8/bin/poetry-issue-2369
#!/home/pawamoy/.cache/pypoetry/virtualenvs/poetry-issue-2369-Q6VvSue2-py3.8/bin/python
#EASY-INSTALL-ENTRY-SCRIPT: 'poetry-issue-2369','console_scripts','poetry-issue-2369'
__requires__ = 'poetry-issue-2369'
import re
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
    sys.exit(
        load_entry_point('poetry-issue-2369', 'console_scripts', 'poetry-issue-2369')()
    )

Note how the installed script wraps the entry-point call into sys.exit(...).

It means that, as a best-practice, the entry-point itself should not use sys.exit(...).

My main function returns an integer, and it's the __main__ module that wraps it in sys.exit(...).

This allows to run both the installed script with poetry-issue-2369 and the Python module with python -m poetry-issue-2369 seamlessly.

# cli.py
def main(args=None):
    return 1
# __main__.py
import sys

from poetry_issue_2369.cli import main

if __name__ == "__main__":
    sys.exit(main(sys.argv[1:]))

Solution

It seems to be because this snippet does not wrap the last line in sys.exit(...):

cmd += [
    "import sys; "
    "from importlib import import_module; "
    "sys.argv = {!r}; {}"
    "import_module('{}').{}()".format(args, src_in_sys_path, module, callable_)
]

https://github.com/python-poetry/poetry/blob/master/poetry/console/commands/run.py#L42-L47

The solution would be to wrap it in sys.exit(...):

-    "import_module('{}').{}()".format(args, src_in_sys_path, module, callable_)
+    "sys.exit(import_module('{}').{}())".format(args, src_in_sys_path, module, callable_)
@pawamoy pawamoy added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Apr 30, 2020
@hartzell
Copy link

I'm seeing the same issue with Poetry version 1.0.10 on a CentOS 7 system using Python 3.7.6 build via Spack.

@hartzell
Copy link

After more digging, I was running into pallets/click#747. Switching from returning values to explicitly calling sys.exit(n) (for various values of n) results in calls to poetry run my_console_script exiting with the values I expect.

@pawamoy
Copy link
Author

pawamoy commented Sep 11, 2020

You mean you are now using sys.exit yourself in your script entrypoint?

I don't want to do this because then:

  • it's redundant with my __main__ module, which runs sys.exit(main(sys.argv[1:]))
  • it's redundant with the contents of installed scripts, which look like:
#!/path/to/some/python
# -*- coding: utf-8 -*-
import re
import sys
from my_package.my_module import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())
  • if I have several entrypoints, I have to remember to use sys.exit in each one of them
  • each one of these entrypoint is not composable anymore since they will exit the interpreter (I cannot call them from other Python code)

I guess what I could do to solve these points is to duplicate each "script" function:

# this one returns an integer
def func_a(args):
    return 2

# this one actually sys.exits
def entrypoint_a(args):
    sys.exit(func_a(args))

And use the entrypoints in tool.poetry.scripts:

[tool.poetry.scripts]
a = "my_package.my_module:entrypoint_a"

@pawamoy
Copy link
Author

pawamoy commented Sep 11, 2020

But since console scripts, when installed, do the sys.exit for you, it would feel more consistent if poetry run would do it for you as well.

@hartzell
Copy link

I agree that having poetry do the sys.exit would be the right thing.

But, for better or worse, I'm using click, and since it doesn't do anything with the return value of a function that is a @click.command, it doesn't matter what poetry does, it's getting dropped before it ever gets into poetry's hands.

Here's one conversation in the Click repo: pallets/click#747

@ypicard
Copy link

ypicard commented Jun 18, 2021

Any news on this?

@pawamoy
Copy link
Author

pawamoy commented Jun 18, 2021

Working fix in #2904, but I didn't get feedback on how to properly test the change. Anyway, I'm using PDM now instead of Poetry 🙂

@ypicard
Copy link

ypicard commented Jun 23, 2021

What is your feedback on PDM? It looks fairly new and not too popular yet, but a lot closer to npm so promising. Would love some feedback!

@claco
Copy link

claco commented Oct 31, 2021

Care here for this problem. For me, this manifested when I converted a pre-commit hooks project from pip/setup to poetry:

Pip/Setup console_scripts do:

if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

Poetry installed scripts do:

    main()

So, hooks entrys in .pre-comit-hooks.yml that used to fail, now don't, because they all return int, and used to sys.exit, and now don't.

In order to fix this, one could add sys.exit to the of then imported main(), but that means all python (not cli) consumers that call main() will get a hard sys exit, not just the returns int.

That then means, one must drop a separate script entry point, like hook(), that calls main() and calls sys.exit with its return. It's all kind of awkword after moving from setup.py and expecting scripts to do the same thing.

@rc-mattschwager
Copy link
Contributor

I'm also running into this UX oddity. My pattern is:

foo/__main__.py:

def main():
    # Do things...
    return exit_code

if __name__ == "__main__":
    sys.exit(main())

pyproject.toml

[tool.poetry.scripts]
foo = "foo.__main__:main"

Running the following does not relay the exit code, as mentioned above:

$ poetry run foo

However, since I'm using foo/__main__.py, the following does relay the exit code:

$ poetry run python -m foo

It would be nice to reconcile this difference and have scripts relay the exit code 👍

@finswimmer
Copy link
Member

This should be fixed in the latest preview release (1.2.0b1) by #4456.

@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Jun 11, 2022
carmenbianca added a commit to fsfe/reuse-tool that referenced this issue Jun 22, 2023
This is needed because Poetry 1.2 is the version that introduced return
codes for scripts generated by Poetry, and we depend on the return code
of `reuse`. See <python-poetry/poetry#2369>.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
carmenbianca added a commit to fsfe/reuse-tool that referenced this issue Jun 25, 2023
This is needed because Poetry 1.2 is the version that introduced return
codes for scripts generated by Poetry, and we depend on the return code
of `reuse`. See <python-poetry/poetry#2369>.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
carmenbianca added a commit to fsfe/reuse-tool that referenced this issue Jun 25, 2023
This is needed because Poetry 1.2 is the version that introduced return
codes for scripts generated by Poetry, and we depend on the return code
of `reuse`. See <python-poetry/poetry#2369>.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants