Drop the python2 patch sys_path_security-issue_16202

[mkoeppe]

This patch, applied by #13579 ("Python sys.path security risk") 7 years ago was never accepted in upstream Python. We do not bother to do the same in python3; and it causes build problems (#29367).

Related tickets:

• #25358 Test safe directory without python subprocess
• #26457 Do not depend on a patched Python

[mkoeppe]

New commits:

​1a578e4

Remove python2 patch sys_path_security-issue_16202

[fbissey]

Good riddance.

[jhpalmieri]

Doctest failures with a Python 2 build, so let's get rid of the offending doctests.

---

New commits:

​fd4e408

trac 29394: get rid of corresponding doctest

[fbissey]

Hum, I thought it was going to be handled in a different ticket, but yes that test should go, of course. And that's the only place I know where the issue would crop up.

[jhpalmieri]

Do we need to delete the function test_safe_directory in doctest/control.py?

[fbissey]

Replying to jhpalmieri:

Do we need to delete the function test_safe_directory in doctest/control.py?

I wouldn't think it is needed. After all it works well with python without change. Let me double check.

[jhpalmieri]

To partially answer my own question: that function gives another doctest failure with a Python 2 build. Its docstring also says "This is only relevant with Python 2, because Sage's Python 2 is patched to give a warning when the current directory is unsafe, but Python 3 is not." So I think we should remove the function entirely.

[fbissey]

Oh right, I am forgetting we are still doing py2 builds here. Yes it should go.

[jhpalmieri]

I'll provide a branch which removes it. If you're happy with that, once I run tests successfully with Python 2, I can set to positive review on your behalf.

[mkoeppe]

#25358 has a better fix for test_safe_directory

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​c1f6dcd

trac 29395: remove test_safe_directory

[fbissey]

Replying to mkoeppe:

#25358 has a better fix for test_safe_directory

So, that's why I thought that stuff was dealt with in another ticket. I am not completely losing my marbles.

[jhpalmieri]

Replying to fbissey:

Replying to mkoeppe:

#25358 has a better fix for test_safe_directory

If you want to try to revive that, you can. It can be done on #25358, with this ticket as a prerequisite, since one of the main obstacles there was the patched Python. Once we remove this patch to Python, the current incarnation of test_safe_directory is no longer relevant, as far as I can tell.

So, that's why I thought that stuff was dealt with in another ticket. I am not completely losing my marbles.

That ticket had completely stalled, though.

[fbissey]

Thanks John, we have to make a decision on what we do with #25358 (and how fast) before we proceed with the current changes. Plain removal is attractive to me. Less code.

[mkoeppe]

I think test_safe_directory is a useful security mechanism. Its current implementation depends on the patch that we are now dropping (and hence on Python2)

[mkoeppe]

Replying to jhpalmieri:

Replying to fbissey:

Replying to mkoeppe:

#25358 has a better fix for test_safe_directory

If you want to try to revive that, you can.

Done

It can be done on #25358, with this ticket as a prerequisite

No, present ticket depends on #25358

[mkoeppe]

New commits:

​904b1a1	Test safe directory without subprocess
​7f5d8dc	Remove python2 patch sys_path_security-issue_16202
​cbdce1c	trac 29394: get rid of corresponding doctest

[gh-timokau]

For what it's worth I gave up on #25358 because jdmeyer had a pretty strong opinion on that. I'm not sure if that has changed. If it was up to me, I think I'd vote complete removal too. #25358 was intended as a compromise.

[mkoeppe]

If we want, we can of course keep *both* tests in #25358. Keeping py2 as "secure" as it was; and adding security to py3.

[jhpalmieri]

We can't keep the secure py2 test if we're getting rid of the patch, or am I misunderstanding you?

[mkoeppe]

Replying to jhpalmieri:

We can't keep the secure py2 test if we're getting rid of the patch, or am I misunderstanding you?

Right, it would be purely decorative or for the hypothetical situation that someone decides to run sagelib with a patched python2.

[mkoeppe]

Testing this branch at ​https://github.com/mkoeppe/sage/actions/runs/61077298 (on top of other tickets)

[jhpalmieri]

This passes tests for me on OS X with builds based on Python 2 and Python 3.

[dimpase]

lgtm

[mkoeppe]

Thanks