Opened 8 years ago
Closed 8 years ago
#17044 closed defect (fixed)
fix pexpect interfaces with a system-wide sage install
Reported by: | was | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | interfaces | Keywords: | |
Cc: | novoselt | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | 00fb8e9 (Commits, GitHub, GitLab) | Commit: | 00fb8e97511ac881279cf5bc5c59eb7d85407126 |
Dependencies: | Stopgaps: |
Description (last modified by )
If you install Sage system-wide (e.g., in /usr/local) for all users on your system, most of the pexpect interfaces for optional packages will be completely massively broken. For example, on a system which doesn't have scilab installed:
sage: scilab('2+3') --------------------------------------------------------------------------- OSError Traceback (most recent call last) <ipython-input-1-c7d52e4ba13e> in <module>() ----> 1 scilab('2+3') /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/interface.pyc in __call__(self, x, name) 197 198 if isinstance(x, basestring): --> 199 return cls(self, x, name=name) 200 try: 201 return self._coerce_from_special_method(x) /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/expect.pyc in __init__(self, parent, value, is_name, name) 1310 else: 1311 try: -> 1312 self._name = parent._create(value, name=name) 1313 # Convert ValueError and RuntimeError to TypeError for 1314 # coercion to work properly. /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/interface.pyc in _create(self, value, name) 387 def _create(self, value, name=None): 388 name = self._next_var_name() if name is None else name --> 389 self.set(name, value) 390 return name 391 /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/scilab.pyc in set(self, var, value) 307 """ 308 cmd = '%s=%s;'%(var,value) --> 309 out = self.eval(cmd) 310 if out.find("error") != -1: 311 raise TypeError("Error executing code in Scilab\nCODE:\n\t%s\nScilab ERROR:\n\t%s"%(cmd, out)) /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/scilab.pyc in eval(self, command, *args, **kwds) 274 'd =\n \n 44.' 275 """ --> 276 s = Expect.eval(self, command, **kwds).replace("\x1b[?1l\x1b>","").strip() 277 return s 278 /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/expect.pyc in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds) 1227 elif split_lines: 1228 return '\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds) -> 1229 for L in code.split('\n') if L != '']) 1230 else: 1231 return self._eval_line(code, allow_use_file=allow_use_file, **kwds) /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/expect.pyc in _eval_line(self, line, allow_use_file, wait_for_prompt, restart_if_needed) 821 try: 822 if self._expect is None: --> 823 self._start() 824 E = self._expect 825 try: /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/scilab.pyc in _start(self) 261 sage: scilab._start() # optional - scilab 262 """ --> 263 Expect._start(self) 264 self.eval("mode(0)") 265 /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/interfaces/expect.pyc in _start(self, alt_message, block_during_init) 389 current_path = os.path.abspath('.') 390 dir = self.__path --> 391 sage_makedirs(dir) 392 os.chdir(dir) 393 /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/misc/misc.pyc in sage_makedirs(dir) 79 """ 80 try: ---> 81 os.makedirs(dir) 82 except OSError: 83 if not os.path.isdir(dir): /usr/local/src/sage-git/local/lib/python/os.pyc in makedirs(name, mode) 148 if head and tail and not path.exists(head): 149 try: --> 150 makedirs(head, mode) 151 except OSError, e: 152 # be happy if someone already created the path /usr/local/src/sage-git/local/lib/python/os.pyc in makedirs(name, mode) 155 if tail == curdir: # xxx/newdir/. exists if xxx/newdir exists 156 return --> 157 mkdir(name, mode) 158 159 def removedirs(name): OSError: [Errno 13] Permission denied: '/usr/local/src/sage-git/local/share/sage/ext/scilab'
Change History (19)
comment:1 Changed 8 years ago by
- Branch set to u/chapoton/17044
- Commit set to 33e6ec3d8d907d3cc04cd90a60db29d414c64a6b
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 8 years ago by
I would like to understand the problem better. So William, could you please 1) say which version of Sage this refers to 2) describe the actual input and error you are getting 3) if any doctests fail in your setup (without this patch)
I am asking because you're certainly not the first person ever to install Sage system-wide but it is the first time I see this problem reported. So the problem is probably more subtle than you think.
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 5 Changed 8 years ago by
Replying to jdemeyer:
I would like to understand the problem better. So William, could you please 1) say which version of Sage this refers to
At least any Sage-6.x.
2) describe the actual input and error you are getting
Install Sage system-wide, then try to do anything with *some* of the pexpect-based interfaces as a normal user who doesn't own the sage install, especially ones that use optional software. E.g.,
sage: scilab('2+3')
3) if any doctests fail in your setup (without this patch)
Nope. We don't do doctesting that tests permissions, since doctesting is always run as the user that owns the Sage install.
I am asking because you're certainly not the first person ever to install Sage system-wide but it is the first time I see this problem reported.
This problem has popped on the mailing lists, and basically got ignored, probably because for the most part our Sage developers usually own their own Sage installs. However, with SageMathCloud?, which has now 7000 *active users* per week all using a system-wide Sage install, this problem comes up more. I had dome some horrible hacks to try to get around it, e.g., pointing SAGE_ROOT/local/share at something world writeable, but even this fails miserable if *two* regular users use the same interface -- one creates the user directory as their own, and the other then can't write to it.
I think much of this boils down to a stupid coding mistake (surely made by me) involving =""
versus =None
.
So the problem is probably more subtle than you think.
It probably is. I wouldn't be surprised at all if this small patch is not the whole story, and there are similar bugs still lurking, since any classes that derive from the base Expect class (in expect.py) could cause their own related trouble...
comment:4 Changed 8 years ago by
- Cc novoselt added
comment:5 in reply to: ↑ 3 Changed 8 years ago by
Replying to was:
3) if any doctests fail in your setup (without this patch)
Nope. We don't do doctesting that tests permissions, since doctesting is always run as the user that owns the Sage install.
I meant: do any doctests fail when running doctests as the user who doesn't own the Sage install? That should work (see #5155), although I don't know if this is still tested on a regular basis (I did it as part of my release management scripts, I have no idea if Volker does it).
comment:6 Changed 8 years ago by
- Description modified (diff)
comment:7 Changed 8 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to needs_work
Patch doesn't solve the issue.
comment:8 Changed 8 years ago by
My proposal to fix this ticket would be to get rid completely of the "script_subdirectory" argument, it's a broken design.
comment:9 Changed 8 years ago by
OK, perhaps not get rid of "script_directory" completely, but only use it for scripts which are known to be shipped with Sage, e.g. some PARI/GP scripts.
comment:10 Changed 8 years ago by
- Reviewers changed from Jeroen Demeyer to Frédéric Chapoton
comment:11 Changed 8 years ago by
- Branch changed from u/chapoton/17044 to u/jdemeyer/ticket/17044
- Created changed from 09/25/14 23:34:38 to 09/25/14 23:34:38
- Modified changed from 09/26/14 18:26:28 to 09/26/14 18:26:28
comment:12 Changed 8 years ago by
- Commit changed from 33e6ec3d8d907d3cc04cd90a60db29d414c64a6b to 85f489ceac24da6e79484ca2d4abc8d6c47ce801
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
85f489c | Only use script_directory for existing directories
|
comment:13 Changed 8 years ago by
- Status changed from needs_work to needs_review
This fixes the bug, but I still have to run doctests.
comment:14 Changed 8 years ago by
- Status changed from needs_review to positive_review
Doctests pass.
comment:15 Changed 8 years ago by
- Status changed from positive_review to needs_review
comment:16 Changed 8 years ago by
- Branch changed from u/jdemeyer/ticket/17044 to public/ticket/17044
- Commit 85f489ceac24da6e79484ca2d4abc8d6c47ce801 deleted
Hello,
I am not sure at all to be competent to review this ticket. As far as I can tell, this looks good.
I allowed myself to remove a few unused imports (except one which is in fact used in an indirect way) and one unused variable.
Jeroen, you can set this to positive review if you want.
comment:17 Changed 8 years ago by
- Commit set to 00fb8e97511ac881279cf5bc5c59eb7d85407126
Branch pushed to git repo; I updated commit sha1. New commits:
33e6ec3 | trac #17044 fix to pexpect interface, as required by wstein
|
85f489c | Only use script_directory for existing directories
|
3f76959 | Merge with 6.4.beta4
|
9965182 | trac #17004 remove a few unused imports
|
00fb8e9 | trac #17004 putting one import back
|
comment:18 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:19 Changed 8 years ago by
- Branch changed from public/ticket/17044 to 00fb8e97511ac881279cf5bc5c59eb7d85407126
- Resolution set to fixed
- Status changed from positive_review to closed
Done
New commits:
trac #17044 fix to pexpect interface, as required by wstein