Skip to content

gh-131591: Add remote debugging attachment protocol documentation #132638

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ivonastojanovic
Copy link
Contributor

@ivonastojanovic ivonastojanovic commented Apr 17, 2025

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 17, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

Add a developer-facing document describing the protocol used by
remote_exec(pid, script) to execute Python code in a running process.
This is intended to guide debugger and tool authors in reimplementing
the protocol.
@ivonastojanovic ivonastojanovic force-pushed the doc-remote-debugger-attachment branch from 21b4799 to 34c64f1 Compare April 17, 2025 14:43
@pablogsal pablogsal self-requested a review April 17, 2025 14:43
@pablogsal pablogsal self-assigned this Apr 17, 2025
@pablogsal
Copy link
Member

@hugovk @willingc I will review this today but it would awesome if you can give this a go as well :)

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, very clearly written.

Copy link
Contributor

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick look at the first section. (Edit: I was reviewing when Hugo posted his, there is some overlap)

Everything needs to be wrapped to 79 chars -- this affects almost every line.

@pablogsal
Copy link
Member

pablogsal commented Apr 17, 2025

This is a spec so we should write in a more formal way. The formal tone is better suited for this technical documentation because it establishes the necessary precision required when explaining technical operations like memory manipulation and code injection. This formality isn't just stylistic preference—it serves a functional purpose by reducing ambiguity. @ivonastojanovic here are a bunch of advice on how to change the tone:

  1. Maintain consistent formality: Avoid casual phrases like "That's why" or constructions that directly address the reader as "you." Instead, use more objective phrasing such as "This protocol requires two pieces of information."

  2. Use passive voice (some times): While active voice is generally clearer, technical documentation often benefits from passive voice where the focus should be on processes rather than actors (e.g., "The PyRuntime structure is located" rather than "You need to find the PyRuntime structure").

  3. Minimize colloquial language: Replace informal transitions like "Next up" with more formal alternatives such as "Subsequently" or "The following step involves."

@chris-eibl
Copy link
Member

Maybe a short summary about the life time of the remote script shall be added to the documentation, too?
See #132712 (comment).

@ivonastojanovic ivonastojanovic force-pushed the doc-remote-debugger-attachment branch from a8e5827 to bb9d85d Compare April 20, 2025 10:55
:file:`python{XY}.dll`, where ``X`` and ``Y`` are the major and minor
version numbers of the Python version (for example, ``python311.dll``), and
record its base address.
3. Locate the ``PyRuntim`` section. Section names in the PE format are limited
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
3. Locate the ``PyRuntim`` section. Section names in the PE format are limited
3. Locate the ``PyRuntime`` section. Section names in the PE format are limited

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's no typo and due to the 8 byte length limit in the PE format. Maybe that should be reformulated to make it clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the explanation to clarify the PE section name limit. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

5 participants