Opened 3 years ago
Last modified 4 weeks ago
#26968 new defect
ECL test sometimes fails to create maxima directory
Reported by: | gh-timokau | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.7 |
Component: | doctest framework | Keywords: | random_fail ecl |
Cc: | embray, saraedum, jdmeyer | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
I just got this test failure (and I'm pretty sure I've seen it before):
File "/nix/store/br51f1j23cnxmbf19yn62rn5pp2rmqdh-sage-src-8.5/src/doc/ca/intro/index.rst", line 492, in doc.ca.intro.index Failed example: solve([x + y == 6, x - y == 4], x, y) Exception raised: Traceback (most recent call last): File "/nix/store/mfbhcnzdxiyj831pr3hg1wwh556a6lv3-python-2.7.15-env/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/nix/store/mfbhcnzdxiyj831pr3hg1wwh556a6lv3-python-2.7.15-env/lib/python2.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute exec(compiled, globs) File "<doctest doc.ca.intro.index[1]>", line 1, in <module> solve([x + y == Integer(6), x - y == Integer(4)], x, y) File "/nix/store/mfbhcnzdxiyj831pr3hg1wwh556a6lv3-python-2.7.15-env/lib/python2.7/site-packages/sage/symbolic/relation.py", line 1107, in solve m = maxima(f) File "sage/misc/lazy_import.pyx", line 354, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3682) return self.get_object()(*args, **kwds) File "sage/misc/lazy_import.pyx", line 189, in sage.misc.lazy_import.LazyImport.get_object (build/cythonized/sage/misc/lazy_import.c:2347) return self._get_object() File "sage/misc/lazy_import.pyx", line 221, in sage.misc.lazy_import.LazyImport._get_object (build/cythonized/sage/misc/lazy_import.c:2584) self._object = getattr(__import__(self._module, {}, {}, [self._name]), self._name) File "/nix/store/mfbhcnzdxiyj831pr3hg1wwh556a6lv3-python-2.7.15-env/lib/python2.7/site-packages/sage/interfaces/maxima_lib.py", line 113, in <module> ecl_eval("(set-pathnames)") File "sage/libs/ecl.pyx", line 1326, in sage.libs.ecl.ecl_eval (build/cythonized/sage/libs/ecl.c:10224) cpdef EclObject ecl_eval(str s): File "sage/libs/ecl.pyx", line 1341, in sage.libs.ecl.ecl_eval (build/cythonized/sage/libs/ecl.c:10163) o=ecl_safe_eval(o) File "sage/libs/ecl.pyx", line 350, in sage.libs.ecl.ecl_safe_eval (build/cythonized/sage/libs/ecl.c:5221) raise RuntimeError("ECL says: {}".format( RuntimeError: ECL says: Could not create directory "/build/sage-home/.sage/maxima" C library error: "File exists" **********************************************************************
This cannot be reliably reproduced. Probably some parallelism issue (tests were run with 4 threads).
Change History (27)
comment:1 Changed 3 years ago by
- Cc embray saraedum added
comment:2 Changed 3 years ago by
I have also argued in the past that the tests should use a clean .sage
. It's a problem for some other tests as well. I don't know if there's a good reason not to, other than "nobody's done it yet".
comment:3 follow-up: ↓ 4 Changed 3 years ago by
- Cc jdmeyer added
The only reason I could think of is that some weird configurations wouldn't be tested. But I would think that is more of a feature: Having doctests depend on the user configuration makes everything much more difficult and relying on users to pick up issues when testing with their configuration doesn't sound like a particularly good practice either.
comment:4 in reply to: ↑ 3 Changed 3 years ago by
Replying to gh-timokau:
The only reason I could think of is that some weird configurations wouldn't be tested. But I would think that is more of a feature: Having doctests depend on the user configuration makes everything much more difficult and relying on users to pick up issues when testing with their configuration doesn't sound like a particularly good practice either.
Yes, if there are any bugs resulting from user configuration those should be then reported by users and captured as a regression test: Tests should be isolated as possible and reproducible.
comment:5 Changed 3 years ago by
Do you happen to know how practical it would be to implement then? As I mentioned earlier, it seems like DOCTEST_MODE
is only set to true *after* env.py
was evaluated.
comment:6 follow-up: ↓ 10 Changed 3 years ago by
I think it would be quite practical; just nobody's been sufficiently motivated enough before now.
Probably somewhere in sage.doctest.forker
we would just create a tempdir and set sage.env.DOT_SAGE
to that tempdir before running any examples in a test file.
comment:7 Changed 3 years ago by
But that would set sage.env.DOT_SAGE
for all processes right?
comment:8 Changed 3 years ago by
No, not if it were done after fork.
comment:9 Changed 3 years ago by
- Milestone changed from sage-8.6 to sage-8.7
Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.
comment:10 in reply to: ↑ 6 Changed 3 years ago by
Replying to embray:
I think it would be quite practical; just nobody's been sufficiently motivated enough before now.
Probably somewhere in
sage.doctest.forker
we would just create a tempdir and setsage.env.DOT_SAGE
to that tempdir before running any examples in a test file.
I now remembered why that wouldn't work. There are other variables that depend on sage.env.DOT_SAGE
which wouldn't be re-evaluated. We'd probably need to either change env.py
to use @property
s or spawn completely independent sage instances.
comment:11 Changed 3 years ago by
Although that wouldn't even fix this issue since MAXIMA_USERDIR
is set in sage-env
, not env.py
.
comment:12 Changed 3 years ago by
I still haven't looked at it that deeply but I suspect you're overthinking it. Though the concern about MAXIMA_USERDIR
is definitely valid. That's the kind of problem I wanted to address in #22652, but every time it's come up it's been easier to just implement a narrow fix rather than a general solution.
comment:13 follow-up: ↓ 15 Changed 3 years ago by
Repeating my comment I accidentally posted in #22652 here:
Replying to embray:
I still haven't looked at it that deeply but I suspect you're overthinking it.
env.py
contains these lines:
_add_variable_or_fallback('PYTHON_EGG_CACHE', opj('$DOT_SAGE', '.python-eggs')) _add_variable_or_fallback('SAGE_STARTUP_FILE', opj('$DOT_SAGE', 'init.sage'))
Which is admittedly way less occurrences than I expected. Still, those would need to be re-evaluated or their usage changed.
Though the concern about
MAXIMA_USERDIR
is definitely valid. That's the kind of problem I wanted to address in #22652,
#22652 would be amazing. I don't see why it would necessary fix these issues though. Of course it would make sense to tackle both issues at the same time.
but every time it's come up it's been easier to just implement a narrow fix rather than a general solution.
Speaking of narrow fixes, for this specific use-case, for this particular issue that should do it (how do I get that nice patch highlighting on trac?):
diff --git a/src/sage/doctest/forker.py b/src/sage/doctest/forker.py index 02e18e67e7..115f748727 100644 --- a/src/sage/doctest/forker.py +++ b/src/sage/doctest/forker.py @@ -668,7 +668,14 @@ class SageDocTestRunner(doctest.DocTestRunner, object): try: # Don't blink! This is where the user's code gets run. + # print("MAXIMA_USERDIR is: {}".format(os.environ['MAXIMA_USERDIR'])) + os.environ['MAXIMA_USERDIR'] = "/tmp/maxima-userdir-{}".format(os.getpid()) self.compile_and_execute(example, compiler, test.globs) except SystemExit: raise except BaseException:
I'll probably adopt that for nix. I don't want to fix the same issue on our build server every week. Should I submit that upstream or is that too "hacky"?
comment:14 Changed 3 years ago by
I ended up with https://github.com/NixOS/nixpkgs/pull/54285.
comment:15 in reply to: ↑ 13 Changed 3 years ago by
Replying to gh-timokau:
(how do I get that nice patch highlighting on trac?)
On the line right after the triple braces {{{
, add the line #!diff
. (See also https://trac.sagemath.org/wiki/WikiProcessors for more options like #!sh
, #!html
, etc.)
comment:16 follow-up: ↓ 21 Changed 3 years ago by
You seem too much focusing on the doctest framework here: this is not a bug in the doctest framework, so it shouldn't be fixed in the doctest framework.
Ideally, this should be reported to ECL upstream.
comment:17 Changed 3 years ago by
They seem to assume only one ecl process is running at a time, which may or may not be a bug.
comment:18 Changed 3 years ago by
- Milestone changed from sage-8.7 to sage-8.8
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:19 Changed 3 years ago by
- Milestone sage-8.8 deleted
As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).
comment:20 Changed 2 years ago by
- Milestone set to sage-9.1
I'm getting a similar failure while building doc-html
comment:21 in reply to: ↑ 16 Changed 2 years ago by
The error seems to come from the ECL function SYSTEM::ENSURE-DIRECTORIES-EXIST
(https://gitlab.com/embeddable-common-lisp/ecl/-/blob/develop/src/lsp/mislib.lsp#L277), which should ignore EEXIST because of possible races but does not. But this is likely called via some defsystem facility that is probably also not protected against concurrent use.
comment:22 Changed 2 years ago by
- Milestone changed from sage-9.1 to sage-9.2
comment:23 Changed 19 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:24 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
comment:25 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:26 Changed 5 months ago by
- Milestone changed from sage-9.5 to sage-9.6
comment:27 Changed 4 weeks ago by
- Milestone changed from sage-9.6 to sage-9.7
It would be nice to give every test process its own
DOT_SAGE
directory. Would that be possible? I tried doing that, but got confused as to how the doctest processes are actually started and how isolated they are from the "master" process. I tried settingDOT_SAGE
inenv.py
based onDOCTEST_MODE
, but that apparently always isFalse
.