#9739: Handle duplicate file basenames when testing multiple files in parallel
-------------------------------+--------------------------------------------
Reporter: mpatel | Owner: mvngu
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-4.7.1
Component: doctest | Keywords: doctest scripts
Work_issues: | Upstream: N/A
Reviewer: Robert Bradshaw | Author: Mitesh Patel, John Palmieri
Merged: | Dependencies:
-------------------------------+--------------------------------------------
Comment(by leif):
Replying to [comment:61 jhpalmieri]:
> Replying to [comment:60 leif]:
> > Replying to [comment:58 jhpalmieri]:
> > > Oops, just found a mistake. In non-Sage library code, when
doctesting "file0.py", we write a line
{{{
from file0 import *
}}}
> > > With the name mangling, this doesn't work anymore: the periods
confuse things, and so would hyphens, commas, and other symbols not
allowed in python module names.
> > Hence underscores, which I originally thought of?
>
> Well, the host name could contain all sorts of characters in it,
couldn't it? Same for the directories in the path to the file, especially
since we're talking about files not in the Sage library.
> Doing some sort of regexp search and replace is a lot of work for
perhaps minimal gain.
Well, you don't have to mangle the name in the `import` statement at all
(and the hostname is part of the ''directory'' name, not a filename,
anyway).
> It certainly doesn't have to do with the issue on this ticket.
Of course it does. You tried to also mangle the `import` name to avoid
name clashs, but that's simply not necessary. Just prepend the original
(source) directory of the file to test to `PYTHONPATH`, and keep the
[base]name of the file in `from ... import *` unmodified (and of course
without a path).
For files of the Sage library, we strip that path (at least Mitesh did),
since all necessary (root) directories are already in `PYTHONPATH`.
The only "problem" are files that have to be preparsed (`*.sage`), at
least if we don't want to create the intermediate, preparsed `.py` files
at the original location, preferably just once.
For these files, we simply can do almost what Mitesh did, namely create a
temporary file with an "arbitrary" (Python module name-conforming) but
unique basename and the extension `.py` in `SAGE_TESTDIR` (from the
perspective of `sage-doctest`, i.e. already containing the hostname and
the pid of the parent process), using either `tempfile.mkstemp()` [which
is safe here] or preferably just `sage-doctest`'s PID, import ''that'' in
the `doctest_*` file, and also append it to `tmpfiles`:
{{{
#!python
# We are in "sage-doctest",
# SAGE_TESTDIR is already ".../${hostname}-${ppid}/"
...
if not library_code:
if ext in ['.pyx','.spyx']:
s += "cython(open('%s').read())\n\n" % file_name
elif ext in ['.py', '.sage']:
source = file_name # full name with path
target_name = "%s_%d" % (name, os.getpid()) # like 'name', but
unique
target_base = os.path.join(SAGE_TESTDIR, target_name) # like
'base', also unique
if ext == '.sage':
# Unfortunately, "sage -preparse <file>.sage" doesn't have
any
# output options and always creates <file>.py in the same
# directory, so we first copy the *source* into
SAGE_TESTDIR:
os.system("cp '%s' %s.sage" % (source, target_base))
# Now create SAGE_TESTDIR/<target_name>.py:
os.system("sage -preparse %s.sage" % target_base)
# Remove the copy of the original (.sage):
# (We could also just add it to 'tmpfiles'.)
os.system("rm -f %s.sage" % target_base)
else:
# Plain Python file (".py"), just copy it:
# (If we added source's directory to PYTHONPATH,
# we wouldn't have to copy the file, but then also
# would have to import from 'name' instead of
'target_name'.)
os.system("cp '%s' %s.py" % (source, target_base))
s += "from %s import *\n\n" % target_name
tmpfiles.append(target_base + ".py") # preparsed or copied
original
tmpfiles.append(target_base + ".pyc") # compiled version of it
...
}}}
[[BR]]
A better solution, as Mitesh noted in the `TODO` comment, is to preparse
the file into a string, and directly write that string into the
`doctest_*` file where we currently have the `from ... import *`.
''That enhancement'' is most probably a thing to do on a follow-up ticket,
but not supporting (i.e. avoiding name clashes for) non-library files
would be a regression w.r.t. Mitesh's patch.
[[BR]]
> > It doesn't make sense to copy [only] each single non-library file to
doctest to the temporary directory anyway, as it might import other files
located in the original directory.
>
> Outside the scope of this ticket. If we leave that part alone, we're
not creating a new bug, just leaving a less-than-perfect implementation in
place.
See above. I there just copy the "main" file, which we still can change
later.
[[BR]]
> Meanwhile, if you want to add in some print statements, comments, some
of the relevant parts of Mitesh's patch, or anything else, go ahead. I
have to work on some other things for at least a few days.
As I said, I would have preferred having your patch based on Mitesh's,
since there are a couple of changes that could have been kept, just
changing the "mangling".
I can add a reviewer patch for ''my comments'' (and perhaps a few `print`
statements) later; rebasing Mitesh's would be a lot of work and so I don't
know if I'll do that.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9739#comment:62>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
