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

Pdb can't use the commands through -c or .pdbrc files #90095

Closed
Zrincet mannequin opened this issue Nov 30, 2021 · 15 comments
Closed

Pdb can't use the commands through -c or .pdbrc files #90095

Zrincet mannequin opened this issue Nov 30, 2021 · 15 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes release-blocker stdlib Python modules in the Lib dir

Comments

@Zrincet
Copy link
Mannequin

Zrincet mannequin commented Nov 30, 2021

BPO 45937
Nosy @Zrincet

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-11-30.03:36:50.577>
labels = ['3.8', 'library']
title = "Pdb can't use the commands through -c or .pdbrc files"
updated_at = <Date 2021-11-30.03:36:50.577>
user = 'https://github.com/Zrincet'

bugs.python.org fields:

activity = <Date 2021-11-30.03:36:50.577>
actor = 'Zrincet'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-11-30.03:36:50.577>
creator = 'Zrincet'
dependencies = []
files = []
hgrepos = []
issue_num = 45937
keywords = []
message_count = 1.0
messages = ['407350']
nosy_count = 1.0
nosy_names = ['Zrincet']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue45937'
versions = ['Python 3.8']

Linked PRs

@Zrincet
Copy link
Mannequin Author

Zrincet mannequin commented Nov 30, 2021

I have a python file:

def do_something():
    pass

def main():
    a = 1
    b = 2
    c = a+b
    print(f"c:{c}")
    pass

if __name__ == '__main__':
    main()

I run this command:

python -m pdb -c "b 5" -c "commands 1;;do_something();;end" -c "c" main.py

It doesn't work.
And I have a .pdbrc file

b 5
commands 1
do_something()
end
c

It doesn't work either.

@Zrincet Zrincet mannequin added 3.8 only security fixes stdlib Python modules in the Lib dir labels Nov 30, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@Delengowski
Copy link

Would I be able to get some clarification if this is even supposed to be possible? I myself had a desire to do this and have performed the necessary setup to make an attempt at making this possible but given the long standing stack overflow post about this, I'm not sure if this is even a bug or not.

https://stackoverflow.com/questions/2013698/commands-for-breakpoints-in-a-pdbrc-file

@gaogaotiantian
Copy link
Member

This is fixed now, you can define any commands/aliases in .pdbrc as you want.

@Zrincet
Copy link

Zrincet commented Mar 12, 2024

This is fixed now, you can define any commands/aliases in .pdbrc as you want.

Just wanted to drop a quick thank you for fixing this bug that's been lurking around for ages. Huge relief to see it finally squashed!

gaogaotiantian added a commit to gaogaotiantian/cpython that referenced this issue Mar 12, 2024
…ble tests (pythonGH-110496)

(cherry picked from commit 44f9a84)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
gaogaotiantian added a commit to gaogaotiantian/cpython that referenced this issue Mar 12, 2024
…ble tests (pythonGH-110496)

(cherry picked from commit 44f9a84)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
iritkatriel pushed a commit that referenced this issue Mar 12, 2024
#116661)

[3.12] gh-90095: Make .pdbrc work properly and add some reasonable tests (GH-110496)
(cherry picked from commit 44f9a84)
iritkatriel pushed a commit that referenced this issue Mar 12, 2024
#116660)

[3.11] gh-90095: Make .pdbrc work properly and add some reasonable tests (GH-110496)
(cherry picked from commit 44f9a84)
@wbolster
Copy link
Contributor

wbolster commented Mar 14, 2024

👋🏼 hello @gaogaotiantian @iritkatriel,

it seems the change to fix this bug (#110496 and its backports) broke .pdbrc with comment lines in them, resulting in SyntaxError whenever they're encountered.

for example, a .pdbrc file like this:

# pretty-print often used expressions:
alias pl pp locals()  # ‘print locals’
alias pv pp vars(%*)  # ‘print vars’

… works fine on 3.12.2:

$ PYENV_VERSION=3.12.2 python -c 'breakpoint()'
--Return--
> <string>(1)<module>()->None
(Pdb) 

but the 3.12 branch with the change backported (#116661) fails:

$ PYENV_VERSION=3.12-dev python -c 'breakpoint()'
--Return--
*** SyntaxError: invalid syntax
*** SyntaxError: invalid syntax
(Pdb) 

comment lines in .pdbrc files are widely used, and even the stdlib docs include an example using them.

so this is clearly a regression. since this is a really recent (unreleased) change, i didn't bother to open a new bug report for it (which would require adding a lot of context)

@wbolster
Copy link
Contributor

wbolster commented Mar 14, 2024

as for a fix, this seems to do to the trick:

diff --git Lib/pdb.py Lib/pdb.py
index fd73f14a7e..7a9be7e5cf 100755
--- Lib/pdb.py
+++ Lib/pdb.py
@@ -297,7 +297,7 @@ def setup(self, f, tb):
         self.curframe_locals = self.curframe.f_locals
 
         if self.rcLines:
-            self.cmdqueue = self.rcLines
+            self.cmdqueue = [line for line in self.rcLines if not line.lstrip().startswith("#")]
             self.rcLines = []
 
     # Override Bdb methods

alternatively, actually allowing # ... commands (as a no-op) would also work, and would avoid this situation:

(Pdb) # test
*** SyntaxError: invalid syntax

@iritkatriel iritkatriel reopened this Mar 14, 2024
@iritkatriel iritkatriel added release-blocker and removed 3.8 only security fixes labels Mar 14, 2024
@iritkatriel
Copy link
Member

iritkatriel commented Mar 14, 2024

Thanks. I reopened this and marked as blocker so we won't forget to fix/revert it.

@gaogaotiantian
Copy link
Member

Okay this is my fault. The logic was there but somehow I ignored it.

However, when I took a deeper look at this issue, it's actually more complicated.

The only documentation for .pdbrc is this:

If a file .pdbrc exists in the user’s home directory or in the current directory, it is read with 'utf-8' encoding and executed as if it had been typed at the debugger prompt. This is particularly useful for aliases. If both files exist, the one in the home directory is read first and aliases defined there can be overridden by the local file.

So the current behavior is correct (in the way that it aligns to the documentation). If you type # comment in pdb in 3.12, that reports syntax error. Thus if you put comments in .pdbrc file, it should report syntax error.

However, this current behavior is not desirable as there are already .pdbrc files that contain comments in the world (our documentation even used it). The "bug fix" is actually also a "breaking change". Also it would be useful to have comments in .pdbrc file.

Oh, and this is accidentally fixed in 3.13 because we used codeop.compile_command to compile instead of raw compile, which checks for comments and returns a no-op code object. So it's not an issue for main.

Here are the options for 3.12:

  1. Allow comments as pdb commands (we have multiple ways to do it)
  2. Keep the original behavior, not allow comments as command, but allow it to exist in .pdbrc file
  3. (maybe not ideal) Keep the current "correct" behavior and break user code
  4. (optional) Document .pdbrc correctly in all versions

Comments are actually easier, empty line is a totally different issue - they have meanings in pdb. If you type enter directly without any input (empty line), that means "repeat last command". How are we going to deal with that in .pdbrc file? Do we ignore it which contradicts our documentation (and deprived the user's ability to use it in .pdbrc), or do we respect it which could be unintuitive to many users?

In any case, we should have better documentation for .pdbrc.

@gaogaotiantian gaogaotiantian added 3.11 only security fixes 3.12 bugs and security fixes labels Mar 14, 2024
@iritkatriel
Copy link
Member

Let's not break 3.11 and 3.12. This is not a bugfix that we would backport (it's not a bug that hurts people, while the fix might).
So either fix or revert.

Doc updates are fine for all versions of course.

@gaogaotiantian
Copy link
Member

I'll patch 3.11 and 3.12 so the behavior is consistent as before (ignore comments and empty lines in .pdbrc). The question is - do we want to do the same in 3.13, or we want a different behavior? If we want this behavior on all versions, I'll fix 3.13 and backport, otherwise I'll patch 3.12 and 3.11 separately.

The actual question left (if we fix docs separately) - how we deal with empty lines in .pdbrc in the future. Do we ignore them, or do we consider them as a "repeat". We should document them either way but this is a design decision we need to make. It seems like ignoring might be more intuitive.

@wbolster
Copy link
Contributor

good point about the empty lines, didn't think of that. my suggested quick fix can be easily adapted to that of course.

i firmly believe they should be treated as no-op in a pdbrc file, i.e. treat them the same as comments.

it's rather common to collect various snippets (e.g. copy-pasted from stack overflow) in a .pdbrc, and have empty lines between them.

i think this is how it worked before, so let's not break that, similar to comment lines.

@iritkatriel
Copy link
Member

Maybe change 3.13 too, and update the docs to match behaviour. (Doesn't seem like there's a benefit in changing this.)

@gaogaotiantian
Copy link
Member

Okay I'll do a PR to keep the consistency of the behavior and update the docs (in a way that can be applied to 3.12 and 3.11 as well because no behavior is changed).

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 15, 2024
…-116834)

(cherry picked from commit a50cf6c)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 15, 2024
…-116834)

(cherry picked from commit a50cf6c)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
iritkatriel pushed a commit that referenced this issue Mar 15, 2024
…) (#116854)

gh-90095: Ignore empty lines and comments in `.pdbrc` (GH-116834)
(cherry picked from commit a50cf6c)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
iritkatriel pushed a commit that referenced this issue Mar 15, 2024
…) (#116855)

gh-90095: Ignore empty lines and comments in `.pdbrc` (GH-116834)
(cherry picked from commit a50cf6c)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@gaogaotiantian
Copy link
Member

Okay we restored the behavior for 3.12 and 3.11, and we also updated the docs to clarify the behavior of .pdbrc. I'll close this. Let us know if there are more stuff to do for the matter.

@wbolster
Copy link
Contributor

wow, that was quick. thanks 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes release-blocker stdlib Python modules in the Lib dir
Projects
Development

No branches or pull requests

5 participants