Generate launchers for gui-scripts in editable installs#10968
Open
HarperZ9 wants to merge 4 commits into
Open
Conversation
`EditableBuilder._add_scripts()` only iterated the `console_scripts` entry-point group, so scripts declared in `[project.gui-scripts]` were listed in the editable dist-info's `entry_points.txt` (poetry-core's `convert_entry_points()` emits the `gui_scripts` group) but no runnable launcher was ever written to the venv's bin/Scripts directory. Wheel installs were unaffected because pip generates the launchers itself. Iterate the `gui_scripts` group as well. Editable mode writes an identical plain-Python script (plus a Windows `.cmd` wrapper) for every script with no console-vs-gui distinction, so the existing loop applies to gui scripts unchanged. Fixes python-poetry#10481 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
test_builder_installs_proper_files_for_standard_packages, theproject == "simple_project"guard is repeated in multiple places; consider extracting the GUI-script-specific assertions into a separate test or helper to keep the main test simpler and reduce branching. - When building
scriptsin_add_scripts, you might want to guard against missing keys without creating intermediate lists by using something likeentry_points.get("console_scripts") or []andentry_points.get("gui_scripts") or [], which avoids potentialNoneconcatenation if the underlying data structure changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `test_builder_installs_proper_files_for_standard_packages`, the `project == "simple_project"` guard is repeated in multiple places; consider extracting the GUI-script-specific assertions into a separate test or helper to keep the main test simpler and reduce branching.
- When building `scripts` in `_add_scripts`, you might want to guard against missing keys without creating intermediate lists by using something like `entry_points.get("console_scripts") or []` and `entry_points.get("gui_scripts") or []`, which avoids potential `None` concatenation if the underlying data structure changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
Addressed the Sourcery review feedback in
Local verification from this branch on Windows/Python 3.12:
Note: the repo |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scripts declared in
[project.gui-scripts]are not made available bypoetry install(editable mode). This fixes #10481 (labelledkind/bug+status/confirmed).EditableBuilder._add_scripts()only reads theconsole_scriptsentry-point group:so GUI scripts are silently dropped from the venv's
bin/Scriptsdirectory.The editable-vs-wheel asymmetry
Wheel installs are unaffected: the wheel only records entry points in
entry_points.txt, and pip generates the launchers for bothconsole_scriptsandgui_scriptsat install time. Editable installs, by contrast, generate the launcher scripts themselves insideEditableBuilder— and that code path never iteratedgui_scripts. The result is an inconsistency: a project's GUI scripts work afterpip install ./ building a wheel, but are missing afterpoetry install.Note that the editable dist-info's
entry_points.txtalready lists the[gui_scripts]group correctly, because poetry-core'sconvert_entry_points()returns both groups (verified against poetry-core 2.4.1):So the metadata is right; only the launcher generation is incomplete. (A prior attempt, #9549, was closed procedurally pending poetry-core support for the
gui_scriptsgroup — that support now exists.)The fix
One line — also iterate the
gui_scriptsgroup:Editable mode writes an identical plain-Python script (plus a Windows
.cmdwrapper) for every script, with no console-vs-gui distinction, so the rest of the loop (split(" = "), theValueErrorhint handling, the.cmdwrapper) applies to GUI scripts unchanged.Reproduction
A minimal project:
Before this change, after an editable install:
entry_points.txtcontains both[console_scripts]and[gui_scripts]blocks ✅Scripts/mycli(+mycli.cmdon Windows) exists ✅Scripts/myguiis missing ❌After this change,
Scripts/mygui(+mygui.cmd) is generated alongsidemycli, with the same launcher content.Test
Extends
test_builder_installs_proper_files_for_standard_packagesintests/masonry/builders/test_editable_builder.py:simple_projectfixture gains a[project.gui-scripts]entry (gui-foo = "foo:gui").tmp_venv._bin_dir.joinpath("gui-foo").exists(), the launcher file contents, and theRECORDentry.entry_points.txtassertion is updated to include the[gui_scripts]block.The GUI-specific assertions are gated on
project == "simple_project"rather than also applied tosimple_project_legacy, because the legacy[tool.poetry]schema in poetry-core (poetry-schema.json) does not define agui-scriptstable —[tool.poetry.gui-scripts]is rejected as an unexpected property. GUI scripts are a PEP 621–only feature, so the legacy fixture is intentionally left unchanged.Verified: the new assertion fails on
main(no launcher generated) and passes with the fix.ruff check,ruff format --check, andmypyall pass on the changed files.Coordination
I'm aware of other open PRs that touch
EditableBuilder._add_scripts()/ the same launcher path: #9802 (shebang spaces), #10946 (Windows UTF-8 script wrappers), and #10736 (file scripts). As of opening this PR none of them have merged, so this applies cleanly tomain. If any of them merge first, I'm happy to rebase and re-apply thegui_scriptsinclusion to the new launcher path — in particular #9802 hardcodes a"console"kind, and the intent here is simply that GUI scripts also get a generated launcher.AI assistance disclosure
This change was prepared with the help of Claude (Anthropic). A human reviewed the bug, the one-line fix, and the test; the reproduction and the failing-on-main / passing-with-fix behavior were verified by actually running the editable builder and the test suite, not assumed. I'm responsible for the contents of this PR and happy to make any adjustments maintainers request.