Opened 13 years ago

Closed 13 years ago

#4238 closed defect (fixed)

[with patch, positive review] option to create local .so file for .spyx modules

Reported by: robertwb Owned by: was
Priority: major Milestone: sage-3.1.3
Component: user interface Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Loading an .spyx file requires a recompile for each new startup of Sage. There should be a way to save the .so file locally and load it like a module.

Attachments (2)

local_so.patch (5.3 KB) - added by robertwb 13 years ago.
Original patch from David Fu
local_so_doctests.patch (1.7 KB) - added by robertwb 13 years ago.
get doctests passing

Download all attachments as: .zip

Change History (8)

Changed 13 years ago by robertwb

Original patch from David Fu

Changed 13 years ago by robertwb

get doctests passing

comment:1 Changed 13 years ago by robertwb

  • Summary changed from option to create local .so file for .spyx modules to [with patch, needs review] option to create local .so file for .spyx modules

The one question I have is if we should have a new global function for this behavior, or just provide it as an option to the cython function.

comment:2 Changed 13 years ago by mabshoff

  • Milestone set to sage-3.1.3

This is a duplicate of #909, which I am closing.

Cheers,

Michael

comment:3 Changed 13 years ago by robertwb

I searched for a similar ticket, but I guess I didn't throw in the right keywords. Crazy how I think 3-digit trac tickets are "low" now...

comment:4 Changed 13 years ago by GeorgSWeber

  • Summary changed from [with patch, needs review] option to create local .so file for .spyx modules to [with patch, positive review] option to create local .so file for .spyx modules

First and foremost: I do give this patch a positive review, get this code in!

But the only reason is that it is the first new code in the spyx/load/attach for quite some time. This area is in a sorrow state, several tickets are open addressing different symptoms of the same cause: It's all still nothing but a quick and dirty hack.

We will have problems with this new patch, consider the following scenario: Two files foo.spyx and bar.sage, where bar.sage imports sth from foo.spyx.

  1. User creates local foo.so
  2. User is happy, until he needs in bar.sage a change in sth
  3. User attaches foo.spyx and develops the needed change
  4. User exits Sage
  5. User reenters Sage
  6. User loads bar.sage and wonders why the hell sth displays the old behaviour ...

Of course "User" forgot" to re-create the local foo.so, so the old one was taken. But it is counter-intuitive that anyone should have to remember to do that manually, since the Sage had all information it needed to update the local foo.so itself!

In spite of this patch being so error-prone, without doing small steps in at least some patches, it seems that nothing would move here. So again, positive review.

At last for the question of a "new global function": Well, the Right Thing (TM) to do here would be to introduce proper dependency handling in the load/attach code, probably best done with the help of a tool like Scons. Then, using attach/load just like one does right now, "behind the scenes" there would happen a bit more magic. To the end that whenever the sourcecode of some .spyx and anything else didn't change, a respective .so would already exist and be used (which is quick, as desired); and whenever the sourcecode --- or some other dependency of the .spyx, like e.g. a C library! --- did change, everything needed would happen automatically.

So, in an ideal world, the question about "global function or not" should be utterly superfluous. This problem "always long starting times due to recompilation done every time" just wouldn't exist! But we live in a real world, with limited resources ... So let's add just another hack that at least relieves some of the pain for some of us.

comment:5 Changed 13 years ago by mabshoff

local_so_doctests.patch will likely break on Cygwin, but I guess it will be easy enough to fix.

Cheers,

Michael

comment:6 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged both patches in Sage 3.1.3.alpha3

Note: See TracTickets for help on using tickets.