py3: tiny fix in env.py

[chapoton]

New commits:

​c6ca0b1

py3 tiny fix in env.py

[chapoton]

green bot, please review

[jdemeyer]

Please use sys.executable instead.

[git]

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

​812181f

using sys.executable instead

[chapoton]

indeed, better. Thx

[chapoton]

there is another one in src/sage/doctest/control.py, let us fix it too

[git]

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

​83aa62d

another use of sys.executable

[jdemeyer]

Very good! Positive review if this passes tests on Python 2 and 3.

[chapoton]

hmm. The change in doctest control breaks one doctest in python3:

sage -t --long src/sage/doctest/control.py
**********************************************************************
File "src/sage/doctest/control.py", line 630, in sage.doctest.control.DocTestController.test_safe_directory
Failed example:
    DC.test_safe_directory(d)
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: refusing to run doctests...
Got:
    <BLANKLINE>

I am not sure what to do :

• undo this change ?
• tag the doctest with # py2 ?

[fbissey]

That's interesting. It means two things if I am not mistaken.

• python3 does allow you to run stuff from unsafe directories just like python2
• the doctest and possibly the functionality never worked with python3. That bit must have always called python2.

[jdemeyer]

1. doctest is not tested by python3-known-passing.txt, so that doctest failure doesn't have to prevent merging this ticket. The fix makes sense, despite that it breaks a doctest.

2. I posted about this failure on ​https://groups.google.com/d/msg/sage-devel/_pH7bjQmw5A/IJz_zlMxAwAJ but that thread didn't come to a conclusion.

[chapoton]

I would prefer not to have any new py3-failing file.

[chapoton]

ok, let it be. Jeroen, I am setting to positive in your name.

[jdemeyer]

I'm also not too happy with this situation, but at least this edited test reflects better the reality that it's failing on Python 3. Note that this specific test has always been somewhat controversial.

[embray]

Please squash your commits, and maybe reference this ticket # in the commit message.

[embray]

See #26457 which discusses the possibility of removing this test altogether, or #25358 (which would conflict with this) for an alternative middle-ground.

I would rather we go ahead and do one of these than make fixes to a test that's dubious in the first place.

[chapoton]

New commits:

​e928ec7

trac #27529 py3 tiny fix in env.py (using sys.executable)

[jhpalmieri]

Why don't we split this (already tiny fix) into pieces? The change in env.py should be safe, right? The more controversial doctest can be dealt with elsewhere.

[fbissey]

Replying to jhpalmieri:

Why don't we split this (already tiny fix) into pieces? The change in env.py should be safe, right? The more controversial doctest can be dealt with elsewhere.

Works for me.

[jhpalmieri]

And just to make sure I understand correctly: there are two issues with the tests

    Check that Sage refuses to run doctests from a directory whose
    permissions are too loose.  We create a world-writable directory
    inside a safe temporary directory to test this::

        sage: d = os.path.join(tmp_dir(), "test")
        sage: os.mkdir(d)
        sage: os.chmod(d, 0o777)
        sage: (out, err, ret) = test_executable(["sage", "-t", "nonexisting.py"], cwd=d)
        sage: print(err)
        ...
        RuntimeError: refusing to run doctests...
        sage: (out, err, ret) = test_executable(["sage", "-tp", "1", "nonexisting.py"], cwd=d)
        sage: print(err)
        ...
        RuntimeError: refusing to run doctests...

right? First, they don't work at all with Python 3 because of subprocess.call(['python', ..., and second, they don't work with an unpatched Python. We can fix the second of these by tagging the doctests # build. We could fix the first in a less than ideal way by tagging the doctests # py2.

[fbissey]

They would "work" if python was python3 which can kind of happen in sage-on-distro. But yes they are the kind of tests the # build tag is suitable for. More so than # py2.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​8ebcdc6

trac #27529 py3 tiny fix in env.py (using sys.executable)

[chapoton]

Here is a branch with only the changes in env.py. Positive ?

[jhpalmieri]

I think so. Jeroen already reviewed positively before, and I can confirm that the change fixes the py3 problem with env.py.