Skip to content
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

sage.misc.temporary_file: Move SAGE_TMP implementation here #32986

Closed
mkoeppe opened this issue Dec 6, 2021 · 23 comments
Closed

sage.misc.temporary_file: Move SAGE_TMP implementation here #32986

mkoeppe opened this issue Dec 6, 2021 · 23 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Dec 6, 2021

We move the code for creating the temporary directory to sage.misc.temporary_file, removing the dependency on sage.misc.misc and sage.misc.lazy_string.

This is preparation for #29941, allowing us to keep sage.misc.misc out of sagemath-objects and sagemath-repl; sage.misc.temporary_file goes into sagemath-environment.

CC: @orlitzky @kiwifb

Component: refactoring

Branch/Commit: u/mkoeppe/sage_misc_temporary_file__move_sage_tmp_implementation_here @ 8c4d015

Issue created by migration from https://trac.sagemath.org/ticket/32986

@mkoeppe mkoeppe added this to the sage-9.5 milestone Dec 6, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 6, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2021

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

81d0fffsage.misc.temporary_file: Move SAGE_TMP implementation here

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2021

Commit: 81d0fff

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 7, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 7, 2021

comment:4

I'll need to revise this - @functools.cache is py>=3.9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2021

Changed commit from 81d0fff to 7e78f59

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2021

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

7e78f59src/sage/misc/temporary_file.py: Remove use of functools.cache

@kwankyu
Copy link
Collaborator

kwankyu commented Dec 10, 2021

comment:7

LGTM. Tested through #32987.

@kwankyu
Copy link
Collaborator

kwankyu commented Dec 10, 2021

Reviewer: Kwankyu Lee

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2021

comment:8

Thanks!

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 12, 2022
@vbraun
Copy link
Member

vbraun commented Jan 30, 2022

comment:10

With this ticket I'm seeing a lot of flakiness with maxima-related tests, e.g.

sage -t --long --warn-long 50.7 --random-seed=36889336955867588730901666695941730921 src/sage/interfaces/maxima_abstract.py
**********************************************************************
File "src/sage/interfaces/maxima_abstract.py", line 314, in sage.interfaces.maxima_abstract.MaximaAbstract._commands
Failed example:
    sorted(maxima._commands(verbose=False))
Exception raised:
    Traceback (most recent call last):
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.maxima_abstract.MaximaAbstract._commands[0]>", line 1, in <module>
        sorted(maxima._commands(verbose=False))
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/maxima_abstract.py", line 327, in _commands
        [self.completions(chr(65+n), verbose=verbose)+
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/maxima_abstract.py", line 327, in <listcomp>
        [self.completions(chr(65+n), verbose=verbose)+
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/maxima_abstract.py", line 296, in completions
        cmd_list = self._eval_line('apropos("%s")'%s, error_check=False)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/maxima.py", line 814, in _eval_line
        self._expect_expr(self._display_prompt)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/maxima.py", line 731, in _expect_expr
        i = self._expect.expect(expr)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/pexpect/spawnbase.py", line 343, in expect
        return self.expect_list(compiled_pattern_list,
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/pexpect/spawnbase.py", line 372, in expect_list
        return exp.expect_loop(timeout)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/pexpect/expect.py", line 179, in expect_loop
        return self.eof(e)
      File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/pexpect/expect.py", line 122, in eof
        raise exc
    pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
    Maxima with PID 1795009 running /home/release/Sage/local/bin/maxima -p /home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/sage-maxima.lisp
    command: /home/release/Sage/local/bin/maxima
    args: ['/home/release/Sage/local/bin/maxima', '-p', '/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/interfaces/sage-maxima.lisp']
    buffer (last 100 chars): b''
    before (last 100 chars): ''
    after: <class 'pexpect.exceptions.EOF'>
    match: None
    match_index: None
    exitstatus: None
    flag_eof: True
    pid: 1795009
    child_fd: 20
    closed: False
    timeout: None
    delimiter: <class 'pexpect.exceptions.EOF'>
    logfile: None
    logfile_read: None
    logfile_send: None
    maxread: 4194304
    ignorecase: False
    searchwindowsize: None
    delaybeforesend: None
    delayafterclose: 0.1
    delayafterterminate: 0.1
    searcher: searcher_re:
        0: re.compile(b'<sage-display>')
**********************************************************************

Always disappears when testing individually, but running multiple maxima-using tests in parallel triggers them quite reliably. E.g.

./sage -t -p 8 --long src/sage/interfaces/maxima_abstract.py src/sage/dynamics/complex_dynamics/mandel_julia.py src/sage/interfaces/maxima.py src/sage/tests/books/computational-mathematics-with-sagemath/sol/mpoly_doctest.py src/sage/coding/kasami_codes.pyx

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 6, 2022

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

fc509ecMerge tag '9.6.beta0' into t/32986/sage_misc_temporary_file__move_sage_tmp_implementation_here

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 6, 2022

Changed commit from 7e78f59 to fc509ec

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 13, 2022

comment:12

Confirmed the test failures when running in parallel. Not sure what's happening there, help welcome

@kiwifb
Copy link
Member

kiwifb commented Feb 13, 2022

comment:13

The only thing I think could happen, should have been happening before this ticket. You mostly moved code around. May be there was always a potential race condition on the closing of sage_tmp and it just got enhanced.

@orlitzky
Copy link
Contributor

comment:14

The diff looks OK to me too, but over the past week or so I've realized that you can't trust anything that goes on in the expect interfaces. Using lazy_string adds a layer of unnecessary complexity too; there's no need to encode the PID into the tempfile path.

I'm in the process of removing direct uses of SAGE_TMP anyway if you just want to wait. Afterwards it will be trivial to switch tmp_filename() and tmp_dir(), and SAGE_TMP can be removed from the misc module.

@orlitzky
Copy link
Contributor

comment:15

Color me unsurprised that I'm seeing random (both non-reproducible, and can't-possibly-be-related-to-anything-I-changed) pexpect failures on #33213 as well.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 1, 2022

Changed commit from fc509ec to 8c4d015

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 1, 2022

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

2e81b67sage.misc.temporary_file: Move SAGE_TMP implementation here
8c4d015src/sage/misc/temporary_file.py: Remove use of functools.cache

@mkoeppe
Copy link
Member Author

mkoeppe commented May 4, 2022

comment:18

Superseded by #33797/#33213

@mkoeppe
Copy link
Member Author

mkoeppe commented May 4, 2022

Changed author from Matthias Koeppe to none

@mkoeppe
Copy link
Member Author

mkoeppe commented May 4, 2022

Changed reviewer from Kwankyu Lee to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants