8000 bpo-42384: pdb: correctly populate sys.path[0] by hexagonrecursion · Pull Request #23338 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-42384: pdb: correctly populate sys.path[0] #23338

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 22, 2021
Merged

bpo-42384: pdb: correctly populate sys.path[0] #23338

merged 3 commits into from
Jan 22, 2021

Conversation

hexagonrecursion
Copy link
Contributor
@hexagonrecursion hexagonrecursion commented Nov 17, 2020 •
edited by miss-islington
Loading

https://bugs.python.org/issue42384

Automerge-Triggered-By: GH:gvanrossum

@hexagonrecursion
bpo-42383 and bpo-42384: do not break if the current directory change…
9da9afd 8000 
…s and correctly populate sys.path[0]
@the-knights-who-say-ni

This comment has been minimized.

Sign in to view
@hexagonrecursion

This comment has been minimized.

Sign in to view
@shihai1991
Copy link
Member
shihai1991 commented Nov 17, 2020 •
edited
Loading

The new tests I added fail on Windows. I'll fix them later.

Thanks for your contribute, Looks like you need sign the CLA :)

shihai1991
shihai1991 reviewed Nov 17, 2020
View reviewed changes
Misc/NEWS.d/next/Library/2020-11-17-14-30-12.bpo-42383.ubl0Y_.rst Outdated
@@ -0,0 +1,2 @@
Fix pdb: previously pdb would fail to restart the debugging target if it was
Copy link
Member

Choose a reason for hiding this comment

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

a PR own a misc file is enough. in here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a PR own a misc file is enough. in here

I do not understand. Are you saying I should merge the two misc files into one?

Copy link
Member

Choose a reason for hiding this comment

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

yes, you are right.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, one PR always means a bugfix or a sepeareted developing feature, so a single misc file is good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the situation: I initially treated pdb as a black box. It had two separate bugs. One bug affected how sys.path[0] was populated while the other prevented pdb from properly restarting the target. So I opened two bugs. Then I looked at the code and it just so happens that changing one line fixes both bugs. I'm not sure what to do now.

Copy link
Member

Choose a reason for hiding this comment

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

My personal experience(NOT the official way):

  • Creating the first PR to solve the fist issue you want fixed.
  • After the first PR merged, we can add some description info in second issue, something like: After PR-XXX merged, this issue should be resolved by the way.. MAYBE you can create an another PR to add the testcase of the second issue.

@hexagonrecursion

This comment has been minimized.

Sign in to view
@hexagonrecursion hexagonrecursion changed the title bpo-42383 and bpo-42384: pdb: reexecute the right file even if the current directory changes and correctly populate sys.path[0] bpo-42384: pdb: correctly populate sys.path[0] Nov 19, 2020
@hexagonrecursion
Copy link
Contributor Author

Following this instructions

#23338 (comment)

My personal experience(NOT the official way):

* Creating the first PR to solve the fist issue you want fixed.

* After the first PR merged, we can add some description info in second issue, something like: `After PR-XXX merged, this issue should be resolved by the way.`. MAYBE you can create an another PR to add the testcase of the second issue.

I have updated the pull request.

@hexagonrecursion hexagonrecursion mentioned this pull request Nov 19, 2020
Merged
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 20, 2020
@gvanrossum
Copy link
Member

This looks like the original behavior is just a bug (I think maybe far in the past the path value was always relative) and I am okay with backporting to 3.8 and 3.9.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Dec 23, 2020
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 22, 2021
@gvanrossum gvanrossum added needs backport to 3.8 needs backport to 3.9 only security fixes and removed stale Stale PR or inactive for long period of time. labels Jan 22, 2021
@miss-islington miss-islington merged commit 8603dfb into python:master Jan 22, 2021
@miss-islington
Copy link
Contributor

Thanks @hexagonrecursion for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@bedevere-bot
Copy link

GH-24290 is a backport of this pull request to the 3.9 branch.

@bedevere-bot
Copy link

GH-24291 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 22, 2021
@hexagonrecursion @miss-islington
bpo-42384: pdb: correctly populate sys.path[0] (pythonGH-23338)
1392e78 
Automerge-Triggered-By: GH:gvanrossum
(cherry picked from commit 8603dfb)

Co-authored-by: Andrey Bienkowski <hexagonrecursion@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 22, 2021
@hexagonrecursion @miss-islington
bpo-42384: pdb: correctly populate sys.path[0] (pythonGH-23338)
72ed9fe 
Automerge-Triggered-By: GH:gvanrossum
(cherry picked from commit 8603dfb)

Co-authored-by: Andrey Bienkowski <hexagonrecursion@gmail.com>
@shihai1991
Copy link
Member

Thanks Andrey for your contirbute.
Thanks Guido for your review.

@hexagonrecursion hexagonrecursion deleted the fix-issue42383-and-issue42384 branch January 24, 2021 16:38
This was referenced Jan 25, 2021
Merged
Merged
gvanrossum pushed a commit that referenced this pull request Jan 25, 2021
@hexagonrecursion
[3.9] bpo-42384: pdb: correctly populate sys.path[0] (GH-23338) (#24321)
f2df795
gvanrossum pushed a commit that referenced this pull request Jan 25, 2021
@hexagonrecursion
[3.8] bpo-42384: pdb: correctly populate sys.path[0] (GH-23338) (#24320)
c10180e
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@hexagonrecursion @adorilson
bpo-42384: pdb: correctly populate sys.path[0] (pythonGH-23338)
d5b73b2 
Automerge-Triggered-By: GH:gvanrossum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shihai1991 shihai1991 shihai1991 left review comments

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hexagonrecursion @the-knights-who-say-ni @shihai1991 @gvanrossum @miss-islington @bedevere-bot
0