clarify use of subprocess.call() #7812

dimbleby · 2023-04-19T12:12:45Z

The mixing of env.run(..., call=True) (having no interest in the output, returning an int) and other variants (returning the output as a string) has always been kind of icky.

On closer inspection it turns out there's exactly one place that sets call=True, and it doesn't check the return code anyway, and that's a bug: #3717 and duplicate #6182.

So I've arranged that:

in this branch env.run() just returns an empty string, no-one cared about the return value anyway
but also use subprocess.check_call() instead of subprocess.call(), so that an exception is thrown in case of error - consistent with the other two branches

There are already testcases for subprocess.call() throwing an exception here: it's just that it never actually does so.

Fixes #3717 and duplicate #6182

- have it return a string, simplifying the type annotations - have it check for error

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.4.2` -> `1.5.0` | --- ### Release Notes <details> <summary>python-poetry/poetry</summary> ### [`v1.5.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#150---2023-05-19) [Compare Source](python-poetry/poetry@1.4.2...1.5.0) ##### Added - **Introduce the new source priorities `explicit` and `supplemental`** ([#7658](python-poetry/poetry#7658), [#6879](python-poetry/poetry#6879)). - **Introduce the option to configure the priority of the implicit PyPI source** ([#7801](python-poetry/poetry#7801)). - Add handling for corrupt cache files ([#7453](python-poetry/poetry#7453)). - Improve caching of URL and git dependencies ([#7693](python-poetry/poetry#7693), [#7473](python-poetry/poetry#7473)). - Add option to skip installing directory dependencies ([#6845](python-poetry/poetry#6845), [#7923](python-poetry/poetry#7923)). - Add `--executable` option to `poetry env info` ([#7547](python-poetry/poetry#7547)). - Add `--top-level` option to `poetry show` ([#7415](python-poetry/poetry#7415)). - Add `--lock` option to `poetry remove` ([#7917](python-poetry/poetry#7917)). - Add experimental `POETRY_REQUESTS_TIMEOUT` option ([#7081](python-poetry/poetry#7081)). - Improve performance of wheel inspection by avoiding unnecessary file copy operations ([#7916](python-poetry/poetry#7916)). ##### Changed - **Remove the old deprecated installer and the corresponding setting `experimental.new-installer`** ([#7356](python-poetry/poetry#7356)). - **Introduce `priority` key for sources and deprecate flags `default` and `secondary`** ([#7658](python-poetry/poetry#7658)). - Deprecate `poetry run <entry point>` if the entry point was not previously installed via `poetry install` ([#7606](python-poetry/poetry#7606)). - Only write the lock file if the installation succeeds ([#7498](python-poetry/poetry#7498)). - Do not write the unused package category into the lock file ([#7637](python-poetry/poetry#7637)). ##### Fixed - Fix an issue where Poetry's internal pyproject.toml continually grows larger with empty lines ([#7705](python-poetry/poetry#7705)). - Fix an issue where Poetry crashes due to corrupt cache files ([#7453](python-poetry/poetry#7453)). - Fix an issue where the `Retry-After` in HTTP responses was not respected and retries were handled inconsistently ([#7072](python-poetry/poetry#7072)). - Fix an issue where Poetry silently ignored invalid groups ([#7529](python-poetry/poetry#7529)). - Fix an issue where Poetry does not find a compatible Python version if not given explicitly ([#7771](python-poetry/poetry#7771)). - Fix an issue where the `direct_url.json` of an editable install from a git dependency was invalid ([#7473](python-poetry/poetry#7473)). - Fix an issue where error messages from build backends were not decoded correctly ([#7781](python-poetry/poetry#7781)). - Fix an infinite loop when adding certain dependencies ([#7405](python-poetry/poetry#7405)). - Fix an issue where pre-commit hooks skip pyproject.toml files in subdirectories ([#7239](python-poetry/poetry#7239)). - Fix an issue where pre-commit hooks do not use the expected Python version ([#6989](python-poetry/poetry#6989)). - Fix an issue where an unclear error message is printed if the project name is the same as one of its dependencies ([#7757](python-poetry/poetry#7757)). - Fix an issue where `poetry install` returns a zero exit status even though the build script failed ([#7812](python-poetry/poetry#7812)). - Fix an issue where an existing `.venv` was not used if `in-project` was not set ([#7792](python-poetry/poetry#7792)). - Fix an issue where multiple extras passed to `poetry add` were not parsed correctly ([#7836](python-poetry/poetry#7836)). - Fix an issue where `poetry shell` did not send a newline to `fish` ([#7884](python-poetry/poetry#7884)). - Fix an issue where `poetry update --lock` printed operations that were not executed ([#7915](python-poetry/poetry#7915)). - Fix an issue where `poetry add --lock` did perform a full update of all dependencies ([#7920](python-poetry/poetry#7920)). - Fix an issue where `poetry shell` did not work with `nushell` ([#7919](python-poetry/poetry#7919)). - Fix an issue where subprocess calls failed on Python 3.7 ([#7932](python-poetry/poetry#7932)). - Fix an issue where keyring was called even though the password was stored in an environment variable ([#7928](python-poetry/poetry#7928)). ##### Docs - Add information about what to use instead of `--dev` ([#7647](python-poetry/poetry#7647)). - Promote semantic versioning less aggressively ([#7517](python-poetry/poetry#7517)). - Explain Poetry's own versioning scheme in the FAQ ([#7517](python-poetry/poetry#7517)). - Update documentation for configuration with environment variables ([#6711](python-poetry/poetry#6711)). - Add details how to disable the virtualenv prompt ([#7874](python-poetry/poetry#7874)). - Improve documentation on whether to commit `poetry.lock` ([#7506](python-poetry/poetry#7506)). - Improve documentation of `virtualenv.create` ([#7608](python-poetry/poetry#7608)). ##### poetry-core ([`1.6.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.6.0)) - Improve error message for invalid markers ([#569](python-poetry/poetry-core#569)). - Increase robustness when deleting temporary directories on Windows ([#460](python-poetry/poetry-core#460)). - Replace `tomlkit` with `tomli`, which changes the interface of some *internal* classes ([#483](python-poetry/poetry-core#483)). - Deprecate `Package.category` ([#561](python-poetry/poetry-core#561)). - Fix a performance regression in marker handling ([#568](python-poetry/poetry-core#568)). - Fix an issue where wildcard version constraints were not handled correctly ([#402](python-poetry/poetry-core#402)). - Fix an issue where `poetry build` created duplicate Python classifiers if they were specified manually ([#578](python-poetry/poetry-core#578)). - Fix an issue where local versions where not handled correctly ([#579](python-poetry/poetry-core#579)). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/717 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

github-actions · 2024-03-03T22:46:17Z

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

dimbleby force-pushed the subprocess-call branch 4 times, most recently from 9945c7b to 1ecb2dc Compare April 20, 2023 19:45

radoering previously approved these changes Apr 22, 2023

View reviewed changes

radoering force-pushed the subprocess-call branch from 1ecb2dc to 7da994a Compare April 22, 2023 11:25

radoering enabled auto-merge (squash) April 22, 2023 11:32

clean up env.run(..., call=True)

acce602

- have it return a string, simplifying the type annotations - have it check for error

radoering merged commit 6eccf92 into python-poetry:master Apr 22, 2023

auto-merge was automatically disabled April 22, 2023 11:34
Head branch was pushed to by a user without write access

dimbleby dismissed radoering’s stale review via acce602 April 22, 2023 11:34

dimbleby force-pushed the subprocess-call branch from 7da994a to acce602 Compare April 22, 2023 11:34

dimbleby deleted the subprocess-call branch April 22, 2023 11:35

dimbleby mentioned this pull request Apr 24, 2023

Fix EditableBuilder ignoring error in build script #6183

Open

radoering mentioned this pull request May 18, 2023

release: bump version to 1.5.0 #7947

Merged

github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clarify use of subprocess.call() #7812

clarify use of subprocess.call() #7812

dimbleby commented Apr 19, 2023 •

edited

Loading

github-actions bot commented Mar 3, 2024

clarify use of subprocess.call() #7812

clarify use of subprocess.call() #7812

Conversation

dimbleby commented Apr 19, 2023 • edited Loading

github-actions bot commented Mar 3, 2024

dimbleby commented Apr 19, 2023 •

edited

Loading