Opened 8 years ago

Last modified 5 years ago

#14414 needs_work defect

runsnake command broken

Reported by: nbruin Owned by: tbd
Priority: major Milestone: sage-6.4
Component: performance Keywords:
Cc: sage-combinat, saliola, aschilling, novoselt, ncohen Merged in:
Authors: Nils Bruin Reviewers:
Report Upstream: N/A Work issues:
Branch: public/ticket/14414 (Commits) Commit: 8a8a7cee9292c039034f58b43d1070a999879619
Dependencies: #9386 Stopgaps:

Description (last modified by tmonteil)

#11287 introduced a runsnake command that tries to execute runsnake using the system python. However, it does so without sage-native-execute and without clearing PYTHONHOME and PYTHONPATH, so it has very little chance of ever working. It already comes a little closer by doing

-   os.system("/usr/bin/python -E `which runsnake` %s &"%tmpfile)
+   os.system("unset PYTHONHOME; unset PYTHONPATH; sage-native-execute runsnake %s &"%tmpfile)

in sage/misc/dev_tools.py (that makes the command runsnake work on my computer).

You can't really expect the system python to work properly when the environment variables point to libraries for other python installs. See also #9386, #10286, #17735.

Attachments (1)

trac-14414.patch (1.1 KB) - added by chapoton 7 years ago.

Download all attachments as: .zip

Change History (35)

comment:1 Changed 8 years ago by nthiery

Thanks for investigating this. Please someone go ahead and make a patch!

Cheers,

Nicolas

comment:2 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

Changed 7 years ago by chapoton

comment:3 Changed 7 years ago by chapoton

Here is a patch, just as suggested.

comment:4 Changed 7 years ago by nbruin

--PING--

I just ran into this again. Can someone review this?

comment:5 follow-up: Changed 7 years ago by aschilling

  • Authors set to Frederic Chapoton
  • Branch set to public/ticket/14414
  • Commit set to f2654aea5ade94a5f43d87dc3b83906c57054545
  • Status changed from new to needs_review

New commits:

f2654aetrac #14414 runsnake and paths

comment:6 in reply to: ↑ 5 Changed 7 years ago by aschilling

For me runsnake runs with and without the patch, so I am probably not the right reviewer. But I did convert the patch to a git branch in order to play with it.

comment:7 Changed 7 years ago by chapoton

  • Authors changed from Frederic Chapoton to Frédéric Chapoton

comment:8 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 follow-up: Changed 7 years ago by darij

I can't get runsnake to work either with or without this patch.

sage: runsnake("list(SymmetricGroup(3))")
sage: Traceback (most recent call last):
  File "/usr/local/bin/runsnake", line 9, in <module>
    load_entry_point('RunSnakeRun==2.0.2b1', 'gui_scripts', 'runsnake')()
  File "/usr/local/lib/python2.7/dist-packages/RunSnakeRun-2.0.2b1-py2.7.egg/runsnakerun/runsnake.py", line 784, in main
    app = RunSnakeRunApp(0)
  File "/usr/lib/python2.7/dist-packages/wx-2.6-gtk2-unicode/wx/_core.py", line 7700, in __init__
    self._BootstrapApp()
  File "/usr/lib/python2.7/dist-packages/wx-2.6-gtk2-unicode/wx/_core.py", line 7352, in _BootstrapApp
    return _core_.PyApp__BootstrapApp(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/RunSnakeRun-2.0.2b1-py2.7.egg/runsnakerun/runsnake.py", line 722, in OnInit
    frame = MainFrame( config_parser = load_config())
  File "/usr/local/lib/python2.7/dist-packages/RunSnakeRun-2.0.2b1-py2.7.egg/runsnakerun/runsnake.py", line 205, in __init__
    self.CreateControls(config_parser)
  File "/usr/local/lib/python2.7/dist-packages/RunSnakeRun-2.0.2b1-py2.7.egg/runsnakerun/runsnake.py", line 231, in CreateControls
    square_style = True,
  File "/usr/local/lib/python2.7/dist-packages/SquareMap-1.0.1-py2.7.egg/squaremap/squaremap.py", line 135, in __init__
    self.OnSize(None)
  File "/usr/local/lib/python2.7/dist-packages/SquareMap-1.0.1-py2.7.egg/squaremap/squaremap.py", line 226, in OnSize
    self.UpdateDrawing()
  File "/usr/local/lib/python2.7/dist-packages/SquareMap-1.0.1-py2.7.egg/squaremap/squaremap.py", line 230, in UpdateDrawing
    self.Draw(dc)
  File "/usr/local/lib/python2.7/dist-packages/SquareMap-1.0.1-py2.7.egg/squaremap/squaremap.py", line 236, in Draw
    brush = wx.Brush( self.BackgroundColour  )
AttributeError: 'SquareMap' object has no attribute 'BackgroundColour'
sage: 

No window appears in reasonable time. Is this any related?

comment:10 in reply to: ↑ 9 Changed 7 years ago by nbruin

Replying to darij:

I can't get runsnake to work either with or without this patch. No window appears in reasonable time. Is this any related?

Is perhaps the version of your SquareMap mismatched with runsnake? When I installed RunSnakeRun==2.0.4 (yum install from standard Fedora 19 repositories), it complained about a version mismatch for SquareMap. At that point, the only SquareMap available from fedora's standard repositories was SquareMap-1.0.1, but SquareMap-1.0.3 was needed. After I installed that from PyPi everything worked like a charm (with the patch here).

comment:11 follow-up: Changed 7 years ago by darij

Thanks for the instructions!

I fear this will disappoint you, but I can now run the snake both with and without this branch...

comment:12 in reply to: ↑ 11 ; follow-up: Changed 7 years ago by nbruin

Replying to darij:

Thanks for the instructions!

I fear this will disappoint you, but I can now run the snake both with and without this branch...

:-) That means that your native python /usr/bin/python is binary compatible with sage's python. Good for you! However, we cannot guarantee that this is the case. In general, if we run /usr/bin/python we have to ensure that the sage library and the sage-python library (via the PYTHON path variables) do not get in the way of standard python execution.

An alternative is to try and figure out how to install runsnake for sage's python (i.e., make an spkg out of it) but that seems like a hopelessly complicated solution relative to relying on system-provided runsnake. That said, fedora still only offers python-SquareMap?-1.0.1, which is insufficient for the runsnake they offer. But at least the beast that is wxPython is available on the system Python by default ...

comment:13 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:14 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:15 in reply to: ↑ 12 ; follow-up: Changed 6 years ago by SimonKing

Replying to nbruin:

An alternative is to try and figure out how to install runsnake for sage's python (i.e., make an spkg out of it) but that seems like a hopelessly complicated solution relative to relying on system-provided runsnake. That said, fedora still only offers python-SquareMap?-1.0.1, which is insufficient for the runsnake they offer. But at least the beast that is wxPython is available on the system Python by default ...

I would appreciate to have that alternative. I tried (system-wide) sudo easy_install RunSnakeRun, but then runsnake failed with

Traceback (most recent call last):
  File "/usr/bin/runsnake", line 9, in <module>
    load_entry_point('RunSnakeRun==2.0.4', 'gui_scripts', 'runsnake')()
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 337, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 2333, in load_entry_point
    return ep.load()
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 2039, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File "/usr/lib/python2.7/site-packages/RunSnakeRun-2.0.4-py2.7.egg/runsnakerun/runsnake.py", line 4, in <module>
    import wx, sys, os, logging, traceback
ImportError: No module named wx

So, easy_install apparently did not take care of dependencies. I think I met this problem before (I am not sure if I ever managed to install runsnake on my laptop). Unfortunately, runsnake seems to be unknown to the opensuse distribution. So, I can not simply install it with yast, hoping that yast takes care of dependencies (in contrast to easy_install).

comment:16 in reply to: ↑ 15 ; follow-up: Changed 6 years ago by nbruin

Replying to SimonKing:

I would appreciate to have that alternative. I tried (system-wide) sudo easy_install RunSnakeRun, but then runsnake failed with

Traceback (most recent call last):
  File "/usr/bin/runsnake", line 9, in <module>
    load_entry_point('RunSnakeRun==2.0.4', 'gui_scripts', 'runsnake')()
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 337, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 2333, in load_entry_point
    return ep.load()
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 2039, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File "/usr/lib/python2.7/site-packages/RunSnakeRun-2.0.4-py2.7.egg/runsnakerun/runsnake.py", line 4, in <module>
    import wx, sys, os, logging, traceback
ImportError: No module named wx

So, easy_install apparently did not take care of dependencies.

If you google "suse runsnake" you'll get RPM hits, so at least third parties have packaged it for opensuse. I wouldn't go and install random rpms from the web, but for instance this page lists the requirements of the rpm. With a bit of luck THOSE can be satisfied by yast (the main one probably being python-wxWidgets) so that from that point your easily installed runsnake might work.

Packaging a runsnake for sage's python would involve recompiling sage's python with wx support. That sounds painful.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 6 years ago by SimonKing

Replying to nbruin:

If you google "suse runsnake" you'll get RPM hits, so at least third parties have packaged it for opensuse. I wouldn't go and install random rpms from the web, but for instance this page lists the requirements of the rpm. With a bit of luck THOSE can be satisfied by yast (the main one probably being python-wxWidgets) so that from that point your easily installed runsnake might work.

Thank you! python-wxWidgets was the missing bit. Previously, I have not been able to find this by searching what is offered by yast. Now, runsnake starts.

comment:18 in reply to: ↑ 17 Changed 6 years ago by SimonKing

Replying to SimonKing:

Thank you! python-wxWidgets was the missing bit.

"Missing bit" in the sense of "runsnake can be installed with easy_install, even though it is not known to yast".

comment:19 Changed 6 years ago by ncohen

  • Cc ncohen added

comment:20 Changed 6 years ago by jdemeyer

What's the reason anyway that the system Python is used? Why not just use Sage's Python, like every other optional package?

comment:21 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work

In any case, the commands unset PYTHONHOME; unset PYTHONPATH; really should be moved to sage-native-execute.

comment:22 follow-up: Changed 6 years ago by jdemeyer

I just tried to install runsnake in my Sage Python and it went flawlessly.

For wxPython, I followed these instructions: all I had to do was download and extract the tarball and then

cd wxPython
python build-wxpython.py --prefix="$SAGE_LOCAL" --install

For runsnake, I used easy_install:

easy_install RunSnakeRun

(all this in a Sage shell of course)

Perhaps it used to be more complicated or it is highly system-dependent, but I don't see the problem...

comment:23 in reply to: ↑ 22 Changed 6 years ago by nbruin

Replying to jdemeyer:

I just tried to install runsnake in my Sage Python and it went flawlessly.

If failed for me in the configuration process:

checking for GST... configure: WARNING: GStreamer 0.10 not available, falling back to 0.8
checking for GST... configure: WARNING: GStreamer 0.8/0.10 not available.
configure: error: GStreamer not available
Error running configure
ERROR: failed building wxWidgets

Runsnake as packaged by Fedora works well for me (apart from the fact that SquareMap-1.0.3 was required but not packaged by Fedora. I presume this has been resolved in more modern distributions).

So, installing and building runsnake in the sage python seems a convenient solution for some people, but the building requirements are so heavy (the wxPython source tarball is 58Mb, and it requires all kinds of bells and whistles to be present on the host system that are not otherwise required for sage) that supporting a system runsnake should definitely be an option (and it's not hard to do).

The last couple of times I've just resorted to saving the profile data using "%prun -D <filename> <command" and opening it with the system runsnake afterwards (the pathnames allow the system runsnake to display all relevant source, so it doesn't need to run in the same python as where the profile comes from).


So something along these lines would probably give the best of both worlds:

  if os.path.exists( os.environ['SAGE_LOCAL']+'/bin/runsnake'):
      subprocess.call(["runsnake",tmpfile])
  else
      os.system("""sh -c "
        LD_LIBRARY_PATH=$SAGE_ORIG_LD_LIBRARY_PATH; export LD_LIBRARY_PATH
        if [ `uname` = 'Darwin' ]; then
            DYLD_LIBRARY_PATH=$SAGE_ORIG_DYLD_LIBRARY_PATH
        fi
        unset PYTHONHOME; unset PYTHONPATH;
        runsnake %s
      "    
      """%tempfile)

This is assuming that the runsnake script itself addresses the python it wants by full name, which installed scripts usually do. We definitely need to reset the PYTHON variables, since those will be referring to sage-specific python modules, which may be incompatible with the system python. And we also do not want the sage libraries get in the way.

optional: catch if running the command led to an error condition and then print some message what the possible avenues are for getting runsnake installed.

Last edited 6 years ago by nbruin (previous) (diff)

comment:24 Changed 6 years ago by jdemeyer

I like your proposal from the last comment, but some comments:

You never need os.system("sh -c ...") since os.system() already uses a Shell.

And like I said before, use sage-native-execute instead of manually messing with environment variables.

Instead of os.path.exists(), you could instead use the more Pythonic

try:
    subprocess.call(os.path.join(SAGE_LOCAL, "bin", "runsnake"...)
except OSError:
    ...

comment:25 follow-up: Changed 6 years ago by jdemeyer

To elaborate on os.path.exists(file):

  1. it checks only whether something by that name exists, not that it's an ordinary file. So at least use os.path.isfile(file) instead.
  1. it doesn't check whether the file is executable. Removing executable bits from a program is a valid way to temporarily "disable" it, so you should also check os.access(file, X_OK).

But really, the try/except style is preferred by Python...

Last edited 6 years ago by jdemeyer (previous) (diff)

comment:26 in reply to: ↑ 25 ; follow-up: Changed 6 years ago by nbruin

Replying to jdemeyer:

But really, the try/except style is preferred by Python...

Yes, I know that as a general rule, pythonistas seem to promote it, but one shouldn't apply this blindly. The problem is that checking that it's hard to write and "except" class that guarantees it only catches the particular OSError resulting from the relevant file not existing.

A priori, there may be loads of reasons why os.system might return an OSError, and I only want to try something else if runsnake is not installed in sage; otherwise I'd prefer to see an error. I certainly get that effect by testing the presence of runsnake. If $SAGE_LOCAL/bin/runsnake exists but is not executable, an error is also more appropriate than silently trying something else, so not testing executability also leads to better error reporting.

I think it's good to be aware that try/except can be quite efficient in python and for some operations is less prone to race conditions, but one shouldn't apply it blindly. It's simply too hard to get a sufficiently fine filtering in the except clause.

comment:27 in reply to: ↑ 26 ; follow-up: Changed 6 years ago by jdemeyer

Replying to nbruin:

A priori, there may be loads of reasons why os.system might return an OSError

You mean subprocess.call, not os.system. Yes, this is an important difference!

I would say it's highly unlikely that subprocess.call raises an OSError which is not because the file does not exist or is not executable. The only thing I can think of is running out of memory...

If $SAGE_LOCAL/bin/runsnake exists but is not executable, an error is also more appropriate

I completely disagree with this, see previous comment.

comment:28 in reply to: ↑ 27 ; follow-up: Changed 6 years ago by nbruin

Replying to jdemeyer:

If $SAGE_LOCAL/bin/runsnake exists but is not executable, an error is also more appropriate

I completely disagree with this, see previous comment.

OK, executable lookup indeed ignores non-executable files that occur earlier in the path, so we should too. Strictly speaking the check should be

sage_runsnake=os.path.join(os.eviron("SAGE_LOCAL"),"bin","runsnake")
if os.path.isfile(sage_runsnake) and os.access(sage_runsnake,os.X_OK):
    ...

to emulate normal PATH behaviour (where lookup skips directories with the relevant name). Also, there's a comment on os.access that it tests against real uid rather than effective gid. Sigh ... it's almost worth running the risk of an improperly caught OSError.

comment:29 in reply to: ↑ 28 Changed 6 years ago by jdemeyer

Replying to nbruin:

Also, there's a comment on os.access that it tests against real uid rather than effective gid.

That's true, but I would think it's unlikely that people run Sage as suid. But it's certainly odd that there is no system call to check whether a file is executable for the current user.

comment:30 Changed 6 years ago by tmonteil

  • Description modified (diff)

See also #17735 that was primarily about docs, checks and preparsing for runsnake(), but discuss similar issues.

comment:31 Changed 6 years ago by git

  • Commit changed from f2654aea5ade94a5f43d87dc3b83906c57054545 to b5f96c35a44f83dbad81a74ed1fbaaddecdd8cf3

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

b0178cftrac #9386: improve environment variable saving infrastructure for sage-env/sage-native-execute
c4fdc67trac #9386: stricter error checking on parameters in sage_export and other cleanup
b5f96c3trac #14414: let runsnake properly test for SAGE_LOCAL install of runsnake and use sage-native-execute otherwise

comment:32 Changed 6 years ago by nbruin

  • Authors changed from Frédéric Chapoton to Nils Bruin
  • Dependencies set to #9386
  • Status changed from needs_work to needs_review

OK, the current branch implements the proposed strategy

edit: D'oh. sage wasn't waiting for the runsnake process to complete before, and there's no reason to change that. Should be fixed now (just call Popen rather than call)

Last edited 6 years ago by nbruin (previous) (diff)

comment:33 Changed 6 years ago by git

  • Commit changed from b5f96c35a44f83dbad81a74ed1fbaaddecdd8cf3 to 8a8a7cee9292c039034f58b43d1070a999879619

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

8a8a7cetrac #14414: let runsnake properly test for SAGE_LOCAL install of runsnake and use sage-native-execute otherwise

comment:34 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

does not apply

Note: See TracTickets for help on using tickets.