Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-99367: Do not mangle sys.path[0] in pdb if safe_path is set#111762
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from2 commits
3fe225d1ec67249a8919ec5fabe6d73c2dac810afcd40d3dbFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -142,8 +142,10 @@ def check(self): | ||||||
| print('Error:', self.orig, 'is a directory') | ||||||
| sys.exit(1) | ||||||
| # Replace pdb's dir with script's dir in front of module search path | ||||||
| # if safe_path is not set, otherwise sys.path[0] is not pdb's dir | ||||||
| ||||||
| # if safe_path is not set, otherwise sys.path[0] is not pdb's dir | |
| # if safe_path is not set |
the part I removed seems a bit grammatically incorrect (and not really necessary) unless I'm missing something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I was trying to comment the reason why we should not replacesys.path[0] - it was not obvious to me. We can either delete the comment or I can try to rephrase the whole thing to make more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ok, rephrase it then. I think it's too terse and not clear what that last part is talking about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is it better now?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
issafe_path not guaranteed to exist? the test doessys.flags.safe_path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I derived this from the original author, but from the source code it seems likesys.flags.safe_path should always exist after 3.11. I'll change it here, maybe together with the comment after we decide what to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don’t remember exactly, but I probably added the check because I was not sure whether it’s okay to depend on ≥ 3.11. If you know that it is, go ahead and remove it.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2493,15 +2493,21 @@ def tearDown(self): | ||
| @unittest.skipIf(sys.flags.safe_path, | ||
| 'PYTHONSAFEPATH changes default sys.path') | ||
| def _run_pdb(self, pdb_args, commands, | ||
| expected_returncode=0, | ||
| extra_env=None): | ||
| self.addCleanup(os_helper.rmtree, '__pycache__') | ||
| cmd = [sys.executable, '-m', 'pdb'] + pdb_args | ||
| if extra_env is not None: | ||
| env = os.environ | extra_env | ||
| else: | ||
| env = os.environ | ||
| with subprocess.Popen( | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stdin=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| env = {**env, 'PYTHONIOENCODING': 'utf-8'} | ||
| ) as proc: | ||
| stdout, stderr = proc.communicate(str.encode(commands)) | ||
| stdout = stdout and bytes.decode(stdout) | ||
| @@ -2513,13 +2519,15 @@ def _run_pdb(self, pdb_args, commands, expected_returncode=0): | ||
| ) | ||
| return stdout, stderr | ||
| def run_pdb_script(self, script, commands, | ||
| expected_returncode=0, | ||
| extra_env=None): | ||
| """Run 'script' lines with pdb and the pdb 'commands'.""" | ||
| filename = 'main.py' | ||
| with open(filename, 'w') as f: | ||
| f.write(textwrap.dedent(script)) | ||
| self.addCleanup(os_helper.unlink, filename) | ||
| return self._run_pdb([filename], commands, expected_returncode, extra_env) | ||
| def run_pdb_module(self, script, commands): | ||
| """Runs the script code as part of a module""" | ||
| @@ -3104,6 +3112,23 @@ def test_issue42384_symlink(self): | ||
| self.assertEqual(stdout.split('\n')[2].rstrip('\r'), expected) | ||
| def test_safe_path(self): | ||
| """ With safe_path set, pdb should not mangle sys.path[0]""" | ||
| script = textwrap.dedent(""" | ||
| import sys | ||
| import random | ||
| print('sys.path[0] is', sys.path[0]) | ||
| """) | ||
| commands = 'c\n' | ||
| with os_helper.temp_cwd() as cwd: | ||
| stdout, _ = self.run_pdb_script(script, commands, extra_env={'PYTHONSAFEPATH': '1'}) | ||
| unexpected = f'sys.path[0] is {os.path.realpath(cwd)}' | ||
| self.assertNotIn(unexpected, stdout) | ||
iritkatriel marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| def test_issue42383(self): | ||
| with os_helper.temp_cwd() as cwd: | ||
| with open('foo.py', 'w') as f: | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Do not mangle ``sys.path[0]`` in :mod:`pdb` if safe_path is set |