Opened 7 years ago

Closed 7 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:

Status badges

Description (last modified by jdemeyer)

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 7 years ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to u/chapoton/17044
  • Commit set to 33e6ec3d8d907d3cc04cd90a60db29d414c64a6b
  • Status changed from new to needs_review

Done


New commits:

33e6ec3trac #17044 fix to pexpect interface, as required by wstein

comment:2 follow-up: Changed 7 years ago by jdemeyer

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: Changed 7 years ago by was

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...

Last edited 7 years ago by was (previous) (diff)

comment:4 Changed 7 years ago by novoselt

  • Cc novoselt added

comment:5 in reply to: ↑ 3 Changed 7 years ago by jdemeyer

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 7 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 7 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

Patch doesn't solve the issue.

comment:8 Changed 7 years ago by jdemeyer

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 7 years ago by jdemeyer

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 7 years ago by jdemeyer

  • Authors changed from Frédéric Chapoton to Jeroen Demeyer
  • Reviewers changed from Jeroen Demeyer to Frédéric Chapoton

comment:11 Changed 7 years ago by jdemeyer

  • 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 7 years ago by git

  • Commit changed from 33e6ec3d8d907d3cc04cd90a60db29d414c64a6b to 85f489ceac24da6e79484ca2d4abc8d6c47ce801

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

85f489cOnly use script_directory for existing directories

comment:13 Changed 7 years ago by jdemeyer

  • Status changed from needs_work to needs_review

This fixes the bug, but I still have to run doctests.

comment:14 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Doctests pass.

comment:15 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_review

comment:16 Changed 7 years ago by chapoton

  • 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 7 years ago by git

  • Commit set to 00fb8e97511ac881279cf5bc5c59eb7d85407126

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

33e6ec3trac #17044 fix to pexpect interface, as required by wstein
85f489cOnly use script_directory for existing directories
3f76959Merge with 6.4.beta4
9965182trac #17004 remove a few unused imports
00fb8e9trac #17004 putting one import back

comment:18 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:19 Changed 7 years ago by vbraun

  • Branch changed from public/ticket/17044 to 00fb8e97511ac881279cf5bc5c59eb7d85407126
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.