Opened 6 years ago

Last modified 6 years ago

#17735 needs_work enhancement

runsnake() cleanup

Reported by: tmonteil Owned by:
Priority: major Milestone: sage-6.5
Component: misc Keywords:
Cc: ncohen Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/tmonteil/runsnake___cleanup (Commits) Commit: ecaf0528f278c20f6c6c7497d311f11594e8a02d
Dependencies: #9386, #14414 Stopgaps:

Description (last modified by tmonteil)

This ticket aims at cleaning runsnake() function to:

  • update the installation documentation
  • check that runsnakerun is installed before failing silently
  • behave consistently with the state of the preparser

See also #14414 and #17689.

Change History (12)

comment:1 Changed 6 years ago by ncohen

  • Cc ncohen added

comment:2 Changed 6 years ago by nbruin

See also #14414. In fact, I'm inclined to close this ticket as "dupe" and address the issues mentioned here on that ticket.

comment:3 Changed 6 years ago by tmonteil

  • Branch set to u/tmonteil/runsnake___cleanup

comment:4 Changed 6 years ago by tmonteil

  • Commit set to ecaf0528f278c20f6c6c7497d311f11594e8a02d
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

f21efcc#17735 preparsing behaves consistently with the state of the preparser.
8b6a711#17735 check that the RunSnakeRun program is available.
3640950#17735 string formatting.
6aead48#17735 update installation documentation.
185fa56#17735 missing dot.
ecaf052#17735 link to the profiling thematic tutorial.

comment:5 follow-up: Changed 6 years ago by tmonteil

Replying to nbruin:

See also #14414.

Thanks for the pointer, i did not see it.

In fact, I'm inclined to close this ticket as "dupe" and address the issues mentioned here on that ticket.

This ticket adresses different (and more trivial) issues (doc, preparsing, checks) that do not require to wait for a wxPython spkg to be created. It can be merged soon, and rebasing #14414 on this one when it becomes ready should not be hard. Actually, this ticket was first related to #17689 so that we have a simple installation procedure somewhere.

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

Replying to tmonteil:

This ticket adresses different (and more trivial) issues (doc, preparsing, checks) that do not require to wait for a wxPython spkg to be created. It can be merged soon, and rebasing #14414 on this one when it becomes ready should not be hard. Actually, this ticket was first related to #17689 so that we have a simple installation procedure somewhere.

OK, it's also possible to repurpose #14414 for providing the infrastructure to install runsnake in sage itself.

In that case, you would need to do something along the lines of http://trac.sagemath.org/ticket/14414#comment:23 because checking if runsnake is available and then invoking it in an inappropriate fashion is not an improvement. I have tested that the following works for me (the code on the #14414 comment needed some adjustments)

    ...
    import os, subprocess
    cProfile.runctx(preparse(command.lstrip().rstrip()), get_main_globals(), locals(), filename=tmpfile)
    if os.path.exists(os.path.join(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
        "    
        """%tmpfile)

I really need all of PYTHONHOME, PYTHONPATH, LD_LIBRARY_PATH reset.

Failure depends on /usr/bin/python and $SAGE_LOCAL/bin/python being binary incompatible, so this is something that needs to be tested on a bunch of platforms.

I can help testing/reviewing if you're ok with that approach (but we need to test on multiple platforms).

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

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

Replying to nbruin:

OK, it's also possible to repurpose #14414 for providing the infrastructure to install runsnake in sage itself.

Why not.

I really need all of PYTHONHOME, PYTHONPATH, LD_LIBRARY_PATH reset.

I like the sage-native-execute approach of Jeroen since it solves the issue for other similar calls, and i wonder whether we could also take the opportunity to also reset the PATH to $SAGE_ORIG_PATH there.

comment:8 Changed 6 years ago by nbruin

  • Dependencies set to 17746
  • Status changed from needs_review to needs_work

Setting #9386 as a dependency, since that now offers a saner sage-native-execute. Branch here needs rebasing, by the way. Otherwise, should be good to go. It's probably easiest to rebase on #14414 right away, since that already has the rather small fix in place to properly run runsnake either installed in SAGE_LOCAL or via sage-native-execute.

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

comment:9 Changed 6 years ago by nbruin

  • Dependencies changed from 17746 to 9386

comment:10 Changed 6 years ago by nbruin

  • Dependencies changed from 9386 to #9386

comment:11 Changed 6 years ago by nbruin

  • Dependencies changed from #9386 to #9386, #14414

comment:12 Changed 6 years ago by nbruin

Looking at the implementation of sage.misc.sage_ostools.have_program, we might want to change:

-           if os.access(os.path.join(p, program), os.X_OK):
+           pth=os.path.join(p, program)
+           if os.path.isfile(pth) and os.access(pth, os.X_OK):

since os.access(..., os.X_OK) might return true for a directory and yet this would not be picked up as an executable by the operating system.

Last edited 6 years ago by nbruin (previous) (diff)
Note: See TracTickets for help on using tickets.