-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
regression in pdb output between 2.7 and 3.5 #70241
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
Comments
Under python 2.7 using the "run" command within pdb and passing it arguments causes those arguments to be printed out. Under 3.5, this is no longer true. $ python2.7 -m pdb pdb_run.py
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys
(Pdb) c
('Command-line args:', ['pdb_run.py'])
The program finished and will be restarted
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys
(Pdb) run a b c "this is a long argument"
Restarting pdb_run.py with arguments:
a b c this is a long argument
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys
$ python3.5 -m pdb pdb_run.py
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys
(Pdb) c
Command-line args: ['pdb_run.py']
The program finished and will be restarted
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys
(Pdb) run a b c "this is a long argument"
Restarting pdb_run.py with arguments:
pdb_run.py
> /Users/dhellmann/Dropbox/PyMOTW/Python3/pymotw-3/source/pdb/pdb_run.py(7)<module>()
-> import sys It looks like the issue is in the pdb main loop. Under 2.7 the restart logic has: except Restart:
print "Restarting", mainpyfile, "with arguments:"
print "\t" + " ".join(sys.argv[1:]) but under 3.5 that was changed to: except Restart:
print("Restarting", mainpyfile, "with arguments:")
print("\t" + " ".join(args)) The do_run() method has already reset sys.argv before throwing Restart, so to print the correct arguments sys.argv[1:] should be used. |
I should also mention that I haven't tested early versions of 3.x to see where exactly the regression was introduced. |
The regression was introduced here: e023091 in main() it prints " ".join(sys.argv[1:]) instead of " ".join(args). In do_run (line 1029) sys.argv gets updated with the arg for that particular "run" command: sys.argv = shlex.split(arg) The quickest fix would be to change main back to print sys.argv rather than arg. But I'm a little uneasy with a program modifying sys.argv (I wouldn't do it). So I'm wondering if that should be changed. What do you think? |
I'm also uneasy about the sys.argv mangling. It seem to me that it would be safer, and more explicit, to pass the desired arguments in the Restart exception instance. |
I like the idea of adding it to the Restart exception. Should we do this refactor in the same PR as fixing the regression or keep them separate? |
Since it's a different implementation approach, I'd prefer a separate PR. Whether we merge the existing one depends on whether we'd like to backport a fix for this to 3.9 and 3.8. |
Do you need my help here? |
It’s fixed now. The tests failed when we merged this old PR. I guess they passed a few months ago but something changed in the meantime. If we want to backport then we need both PRs. |
Commit 652bfde is a bugfix and this issue is marked as Python 3.8-3.10, so yeah, a backport is worth it. You can create a PR for Python 3.9 combining the two commits. I like to use "git cherry-pick -x <sha1>" manually for that. And then, use the label to backport this PR to Python 3.8. |
Thanks, Victor. I created a workflow issue for the CI question: |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: