#11874 closed enhancement (fixed)
Remove twisted spkg
Reported by: | Dima Pasechnik | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-5.2 |
Component: | packages: standard | Keywords: | twisted; notebook |
Cc: | Radoslav Kirov, Keshav Kini, Punarbasu Purkayastha, Karl-Dieter Crisman, Jason Grout, Steven Trogdon | Merged in: | sage-5.2.beta0 |
Authors: | Radoslav Kirov, Jason Grout | Reviewers: | Dmitrii Pasechnik, Jason Grout |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12329, #11080, #10492 | Stopgaps: |
Description (last modified by )
The version of Twisted currently used in Sage is old and obsolete; moreover it is buggy, as our extensive tests involving running labs for 120 students show. The crashes of the notebook webapp go away after the upgrade to twisted 11.
The twisted package is now included in the flask sagenb (#11080), so this ticket just removes the twisted spkg.
See http://groups.google.com/group/sage-devel/browse_thread/thread/98c53d17166da1b9 for the sage-devel vote.
apply:
- trac-11874-remove-twisted.patch to the sage root repository
- sage-spkg-11874.patch to the Sage library.
And remove the twisted spkg from the standard spkgs.
Attachments (2)
Change History (105)
comment:1 Changed 11 years ago by
Cc: | Radoslav Kirov Keshav Kini Punarbasu Purkayastha Karl-Dieter Crisman Jason Grout added |
---|---|
Type: | PLEASE CHANGE → task |
comment:2 Changed 11 years ago by
Component: | misc → packages |
---|---|
Owner: | changed from Jason Grout to tbd |
comment:3 Changed 11 years ago by
Status: | new → needs_review |
---|
comment:4 follow-up: 5 Changed 11 years ago by
Description: | modified (diff) |
---|
See also #11497, which has a possible fix that we may need to apply, maybe?
comment:5 Changed 11 years ago by
comment:6 Changed 11 years ago by
Status: | needs_review → needs_info |
---|
Replying to dimpase:
The new sage notebook at http://code.google.com/r/jasongrout-flask-sagenb/ is needed to test this package
Please explain what you mean with this: needed to "test" this package?
comment:7 follow-up: 8 Changed 11 years ago by
The changes necessary to test this package are included in the http://code.google.com/r/jasongrout-flask-sagenb/ notebook, and not in the current notebook. Thus, to test this spkg, someone will need to install the new flask server as well.
comment:8 Changed 11 years ago by
Replying to jason:
The changes necessary to test this package are included in the http://code.google.com/r/jasongrout-flask-sagenb/ notebook, and not in the current notebook. Thus, to test this spkg, someone will need to install the new flask server as well.
You mean also to "use" this package, or only to "test" it? That's confusing. Is flask-sagenb required or only recommended? Should this be merged into Sage or not?
Also, I would really appreciate some kind of roadmap from the flask-sagenb people to eventually be merged into Sage (I've said this many times without any good answer). Because it looks more and more that flask-sagenb is a fork of sagenb with no real plan for eventually getting back into Sage.
comment:9 follow-up: 16 Changed 11 years ago by
You need the flask-sagenb to use or test this spkg (unless someone backports the necessary patches to the current sagenb, but I don't see that happening).
Sorry for the confusion about the flask-sagenb. Part of the reason is that I've been concentrating so much on fixing bugs with the limited time I had.
That bug-fixing work is winding down as we see fewer and fewer errors popping up on sagenb.org. Over the next few weeks, I'm turning my attention to packaging flask-sagenb up, with the dependencies, and submitting it to trac. The goal is to work on this at SD 35.5, if the packaging work isn't already done by then. Basically, I need to package up these packages: http://code.google.com/p/sagenb/issues/detail?id=62 and also package up the actual sagenb as well.
comment:10 Changed 11 years ago by
The start of that packaging process is this ticket. Since I saw we already had a ticket about upgrading twisted, I put our spkg we already have been testing on *.sagenb.org up here.
comment:11 follow-up: 12 Changed 11 years ago by
To be more clear, here are the priority things to finish before releasing sagenb-flask officially:
(the low priority items could be bumped to a later release)
comment:12 Changed 11 years ago by
Replying to jason:
To be more clear, here are the priority things to finish before releasing sagenb-flask officially:
I would say that [Safari not saving correctly http://code.google.com/p/sagenb/issues/detail?id=60] is a pretty important thing, though apparently it's not universal for some reason. (This still happens, as far as I can tell.)
comment:13 follow-up: 14 Changed 11 years ago by
Yes, but that's a Safari bug, as far as we can tell, and it's hard to reproduce. Is it a regression from the old notebook? If it's not a regression, it may not make it into the first sagenb-flask release.
That said, did you test it again with sagenb.org? I see that Safari has yet another update, 5.1.2, which looks like it may have fixed the underlying bug.
I sent a message to sage-notebook about these updates; we should continue our conversation there.
comment:14 Changed 11 years ago by
Replying to jason:
Yes, but that's a Safari bug, as far as we can tell, and it's hard to reproduce. Is it a regression from the old notebook? If it's not a regression, it may not make it into the first sagenb-flask release.
I thought it was a regression, otherwise I wouldn't have mentioned it. But upon reading my own original post about this, apparently it's a regression unrelated to flask. Sorry for the noise w.r.t. this ticket.
That said, did you test it again with sagenb.org? I see that Safari has yet another update, 5.1.2, which looks like it may have fixed the underlying bug.
I'll try that as well as your other comment on googlecode.
I sent a message to sage-notebook about these updates; we should continue our conversation there.
Ok.
comment:15 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:16 Changed 11 years ago by
Status: | needs_info → needs_review |
---|
Replying to jason:
That bug-fixing work is winding down as we see fewer and fewer errors popping up on sagenb.org. Over the next few weeks, I'm turning my attention to packaging flask-sagenb up, with the dependencies, and submitting it to trac. The goal is to work on this at SD 35.5, if the packaging work isn't already done by then. Basically, I need to package up these packages: http://code.google.com/p/sagenb/issues/detail?id=62 and also package up the actual sagenb as well.
Thanks for the update and apologies if I sounded a bit harsh and inpolite in my comment.
comment:17 Changed 11 years ago by
Milestone: | sage-wishlist → sage-5.0 |
---|
comment:18 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
I would like to give this a positive review.
comment:19 Changed 11 years ago by
Dependencies: | → #11080 |
---|
comment:20 Changed 11 years ago by
Authors: | → Jason Grout |
---|---|
Reviewers: | → Dmitrii Pasechnik |
comment:21 Changed 11 years ago by
Authors: | Jason Grout → Rado Kirov, Jason Grout |
---|
Rado: I think I got the original spkg from you, so I put you as an author. Please correct me if I'm wrong.
comment:22 Changed 11 years ago by
Dima seemed to be hosting the spkg: http://trac.sagemath.org/sage_trac/ticket/11874?action=diff&version=4
comment:23 Changed 11 years ago by
Reviewers: | Dmitrii Pasechnik → Dmitrii Pasechnik, Jason Grout |
---|
And I suppose I could be considered a reviewer of the original version. So: Dima and Rado, can one of you (or both of you) add yourselves as authors, if you feel appropriate about it?
comment:24 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
spkg-install
should be executable and SPKG.txt
should mention this ticket number.
comment:25 Changed 11 years ago by
Alternatively, how do you feel about just including twisted as a dependency in the sagenb.spkg. It would be a few lines change, and then we have automatic updates to the most recent twisted (for example, twisted 11.1 is already released).
Since we don't customize twisted anymore, that may be much easier than maintaining a separate twisted spkg. I'll post to sage-notebook about this.
comment:26 Changed 11 years ago by
Since my suggestion is about removing an spkg, I instead posted a vote to sage-devel.
comment:27 Changed 11 years ago by
I've posted a pull request here: https://github.com/sagemath/sagenb/pull/27
If the vote on sage-devel goes through, then I think this ticket should be changed to instead getting rid of the twisted spkg.
comment:28 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
Adding the patch to remove twisted and setting back to needs review for the new approach.
comment:29 Changed 11 years ago by
Status: | needs_review → needs_work |
---|---|
Summary: | upgrade twisted to twisted 11 → Remove twisted spkg |
Work issues: | → deps |
There seems to be a problem with dependencies:
Extracting package /mnt/usb1/scratch/jdemeyer/merger/sage-5.0.beta2/spkg/standard/jinja2-2.5.5.spkg ... -rw-r--r-- 1 jdemeyer jdemeyer 320030 Dec 4 2010 /mnt/usb1/scratch/jdemeyer/merger/sage-5.0.beta2/spkg/standard/jinja2-2.5.5.spkg Finished extraction **************************************************** Host system uname -a: Linux sage.math.washington.edu 2.6.24-28-server #1 SMP Fri Feb 11 18:08:32 UTC 2011 x86_64 GNU/Linux **************************************************** **************************************************** CC Version gcc -v Using built-in specs. Target: x86_64-linux-gnu Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zl ib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.2 --prog ram-suffix=-4.2 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-checking=release --build=x86_64-linu x-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu4) **************************************************** Traceback (most recent call last): File "setup.py", line 40, in <module> from setuptools import setup, Extension, Feature ImportError: No module named setuptools Error installing Jinja2
comment:30 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
I think my updated patch addresses the dependency issue.
comment:31 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Work issues: | deps |
comment:32 Changed 11 years ago by
Do you want to change the comment before the sagenb spkgs are installed, the comment that starts with "Note: To avoid "
comment:33 Changed 11 years ago by
Type: | task → enhancement |
---|
Clarified comment. The build with this "deps" file is still untested.
comment:34 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:35 Changed 11 years ago by
Cc: | Steven Trogdon added |
---|
comment:36 Changed 11 years ago by
This also likely fixes #12425, since the file mentioned there is not in twisted 11.1 (which is what is currently distributed with the notebook).
comment:37 Changed 11 years ago by
Dependencies: | #11080 → #12329, #11080 |
---|---|
Description: | modified (diff) |
comment:38 follow-up: 42 Changed 11 years ago by
Status: | needs_review → needs_work |
---|
The Sage library doesn't compile because it doesn't depend on Twisted anymore:
Executing 305 commands (using 6 threads) Traceback (most recent call last): File "setup.py", line 831, in <module> execute_list_of_commands(queue) File "setup.py", line 288, in execute_list_of_commands execute_list_of_commands_in_parallel(command_list, nthreads) File "setup.py", line 238, in execute_list_of_commands_in_parallel import twisted.persisted.styles #doing this import will allow instancemethods to be pickable ImportError: No module named twisted.persisted.styles sage: There was an error installing modified sage library code. ERROR installing Sage
The fact that the Sage library needs Twisted is another argument for keeping the twisted spkg separate and not moving it into sagenb. Or alternatively, the Sage library should be fixed such that it doesn't depend on Twisted.
comment:40 Changed 11 years ago by
Never noticed that call before. What happens if we just remove that line from setup.py?
comment:41 Changed 11 years ago by
I wonder if the relevant parts from twisted.persisted.styles are already included in sage/sage/misc/fpickle.pyx
. It may be that we could just import the misc.fpickle module.
comment:42 follow-up: 43 Changed 11 years ago by
Status: | needs_work → needs_info |
---|
Replying to jdemeyer:
The Sage library doesn't compile because it doesn't depend on Twisted anymore:
I cannot reproduce this on 5.0.beta7. I.e. everything builds just fine after applying that patch and removing twisted spkg from a freshly untarred release. Has it been fixed meanwhile?
And what is that test that was failing?
comment:43 follow-up: 44 Changed 11 years ago by
Replying to dimpase:
Replying to jdemeyer:
The Sage library doesn't compile because it doesn't depend on Twisted anymore:
I cannot reproduce this on 5.0.beta7. I.e. everything builds just fine after applying that patch and removing twisted spkg from a freshly untarred release. Has it been fixed meanwhile?
I haven't tried it recently, but it can succeed by chance if Twisted was installed before Sage.
comment:44 follow-up: 45 Changed 11 years ago by
Replying to jdemeyer:
Replying to dimpase:
Replying to jdemeyer:
The Sage library doesn't compile because it doesn't depend on Twisted anymore:
I cannot reproduce this on 5.0.beta7. I.e. everything builds just fine after applying that patch and removing twisted spkg from a freshly untarred release. Has it been fixed meanwhile?
I haven't tried it recently, but it can succeed by chance if Twisted was installed before Sage.
Apparently twisted 12 is installed into Python 2.7.2 by default, at the time Python is built. I see
'/usr/local/src/sage/sage-5.0.beta7/local/lib/python2.7/site-packages/Twisted-12.0.0-py2.7-linux-x86_64.egg'
in my Sage's Python path. And
$ ./sage -python Python 2.7.2 (default, Mar 10 2012, 23:33:53) [GCC 4.4.5] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import twisted >>> twisted.version Version('twisted', 12, 0, 0) >>>
I guess that during the upgrade to Python 2.7.2 it was overlooked that Twisted 12 is in it (not distributed, but getting pulled during the installation. In my install.log I see
Searching for twisted>=8.2 Reading http://pypi.python.org/simple/twisted/ ... Downloading http://pypi.python.org/packages/source/T/Twisted/Twisted-12.0.0.tar.bz2#md5=cf49a8676c21c50faf1b42b528049471
By the way, it also pulls zope and a bunch of other stuff. I don't know what will happen if the installation was running on a machine not connected to the net.
comment:45 follow-up: 46 Changed 11 years ago by
Replying to dimpase:
I guess that during the upgrade to Python 2.7.2 it was overlooked that Twisted 12 is in it (not distributed, but getting pulled during the installation. In my install.log I see
Searching for twisted>=8.2 Reading http://pypi.python.org/simple/twisted/ ... Downloading http://pypi.python.org/packages/source/T/Twisted/Twisted-12.0.0.tar.bz2#md5=cf49a8676c21c50faf1b42b528049471
sage-5.0.beta7 doesn't do this on my machine, so it must be something with your setup. Are you referring to vanilla sage-5.0.beta7 or did you install any extra packages?
comment:46 follow-up: 47 Changed 11 years ago by
Replying to jdemeyer:
sage-5.0.beta7 doesn't do this on my machine, so it must be something with your setup. Are you referring to vanilla sage-5.0.beta7 or did you install any extra packages?
Sorry, I can't reproduce this either, there was a fly in my vanilla, apparently :)
I tried Jason's suggestion to replace the twisted import with importing misc.fpickle, but it doesn't work:
... building 'sage.ext.interpreters.wrapper_el' extension Executing 309 commands (using 1 thread) Exception in thread Thread-5: Traceback (most recent call last): File "/usr/local/src/sage/sage-5.0.beta8/local/lib/python/threading.py", line 552, in __bootstrap_inner self.run() File "/usr/local/src/sage/sage-5.0.beta8/local/lib/python/threading.py", line 505, in run self.__target(*self.__args, **self.__kwargs) File "/usr/local/src/sage/sage-5.0.beta8/local/lib/python/multiprocessing/pool.py", line 314, in _handle_tasks put(task) PicklingError: Can't pickle <type 'instancemethod'>: attribute lookup __builtin__.instancemethod failed
when building sage spkg. Could it be due to fpickle being Cython? I don't know.
comment:47 follow-up: 48 Changed 11 years ago by
Replying to dimpase:
I tried Jason's suggestion to replace the twisted import with importing misc.fpickle, but it doesn't work:
I tried this by doing ./sage -f spgk/standard/sage-5.0....
, but I realized that this doesn't work as expected
with sage-... spkg.
If I instead start from clean install beta8, apply the patch, remove twisted spkg, and in sage-... spkg apply the patch
--- a/setup.py +++ b/setup.py @@ -235,7 +235,7 @@ commands may be run at the same time. """ from multiprocessing import Pool - import twisted.persisted.styles #doing this import will allow instancemethods to be pickable + import sage.misc.misc.fpickle #doing this import will allow instancemethods to be pickable p = Pool(nthreads) process_command_results(p.imap(apply_pair, command_list))
then start building sage, it ends up with
Building modified file sage/ext/interpreters/wrapper_el.pyx. Executing 308 commands (using 1 thread) Traceback (most recent call last): File "setup.py", line 832, in <module> execute_list_of_commands(queue) File "setup.py", line 289, in execute_list_of_commands execute_list_of_commands_in_parallel(command_list, nthreads) File "setup.py", line 239, in execute_list_of_commands_in_parallel import sage.misc.misc.fpickle File "/usr/local/src/sage/sage-5.0.beta8/devel/sage-main/sage/misc/misc.py", line 38, in <module> import sage.misc.prandom as random File "/usr/local/src/sage/sage-5.0.beta8/devel/sage-main/sage/misc/prandom.py", line 56, in <module> from sage.misc.randstate import current_randstate ImportError: No module named randstate Error installing modified sage library code. ERROR installing Sage
What am I doing wrong?
comment:48 follow-up: 49 Changed 11 years ago by
Replying to dimpase:
What am I doing wrong?
Actually, I guess I should try to tweak the order in which the modules are built. Hopefully it doesn't need a cyclic dependence to be resolved.
comment:49 Changed 11 years ago by
Replying to dimpase:
Replying to dimpase:
What am I doing wrong?
Actually, I guess I should try to tweak the order in which the modules are built. Hopefully it doesn't need a cyclic dependence to be resolved.
nope, now I just think that we need misc.fpickle to be moved to an spkg that is built and installed before the Sage spkg. I'll ask on sage-devel.
comment:50 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_info → needs_review |
I have successfully modified sage-***.spkg
to build with twisted removed. See the patch for review above.
I basically cut a piece of sage.misc.fpickle
, it into sage-***.spkg
, and import it instead of twisted
in setup.py
. It is only needed for building the sage python module.
comment:51 Changed 11 years ago by
make ptest
runs OK. (It produces (expected) failures in sagenb---unless you remove it, of course).
Here is a mirror of the sage-* spkg on boxen, for these in USA/Europe.
comment:52 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:53 Changed 11 years ago by
Good to finally see some movement in this ticket. I'm currently testing it.
comment:54 follow-up: 55 Changed 11 years ago by
Status: | needs_review → needs_work |
---|
It needs some more documentation though, this should be expanded a bit:
""" this is needed for setup.py only """
Also you should follow the format of http://sagemath.org/doc/developer/conventions.html#headings-of-sage-library-code-files (without the EXAMPLES
). Mention this ticket.
I'm also not too happy with the name "fpickl.py". It sounds too much like a typo. What about "fpickle_setup.py" or something?
comment:55 follow-up: 56 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
Replying to jdemeyer:
It needs some more documentation though, this should be expanded a bit:
Done.
Also you should follow the format of http://sagemath.org/doc/developer/conventions.html#headings-of-sage-library-code-files (without the
EXAMPLES
). Mention this ticket.
This file is NOT a part of Sage library code. It's basically a part of setup.py, and I followed the informal style we use for this kind of stuff (I could have inserted this code into the setup.py directly, but I felt it's cleaner to keep it separate)
I'm also not too happy with the name "fpickl.py". It sounds too much like a typo. What about "fpickle_setup.py" or something?
Duly renamed as you suggested. The updated patch is uploaded, ready for review again.
comment:56 follow-up: 59 Changed 11 years ago by
Replying to dimpase:
This file is NOT a part of Sage library code.
Agreed, but why is this a reason not to follow the coding style?
comment:57 follow-up: 58 Changed 11 years ago by
Also, your commit message should not start with
* * *
comment:58 Changed 11 years ago by
Replying to jdemeyer:
Also, your commit message should not start with
* * *
I've fixed this, sorry. It was an artefact of hg qfold
comment:59 Changed 11 years ago by
Replying to jdemeyer:
Replying to dimpase:
This file is NOT a part of Sage library code.
Agreed, but why is this a reason not to follow the coding style?
well, you provided me a link to coding guidelines for Sage library code, and this file does not belong there, as it's basically a glorified installation script. Then there are setup.py
and other files which have even less in the header than this new file, so I do not see why I should be holier than the Pope.
And the horrors of digging up the proper copyright notice for this piece of code are, well, horrors...
comment:60 follow-up: 61 Changed 11 years ago by
applying the patches etc procedure for this ticket, and then #11080, followed by ptestlong, shows one failing doctest, in sagenb (which is just very outdated code in sagenb, as the current Poset has more keyargs than expected by the test). This needs to be fixed in sagenb, not here.
sage -t --long -force_lib devel/sagenb-main/sagenb/misc/sageinspect.py ********************************************************************** File "/usr/local/src/sage/sage-5.0.beta8/devel/sagenb-main/sagenb/misc/sageinspect.py", line 653: sage: sage_getargspec(Poset) Expected: (['data', 'element_labels', 'cover_relations'], None, None, (None, None, False)) Got: (['data', 'element_labels', 'cover_relations', 'category', 'facade', 'key'], None, None, (None, None, False, None, None, None))
comment:61 follow-up: 62 Changed 11 years ago by
Replying to dimpase:
applying the patches etc procedure for this ticket, and then #11080, followed by ptestlong, shows one failing doctest, in sagenb (which is just very outdated code in sagenb, as the current Poset has more keyargs than expected by the test). This needs to be fixed in sagenb, not here.
sage -t --long -force_lib devel/sagenb-main/sagenb/misc/sageinspect.py ********************************************************************** File "/usr/local/src/sage/sage-5.0.beta8/devel/sagenb-main/sagenb/misc/sageinspect.py", line 653: sage: sage_getargspec(Poset) Expected: (['data', 'element_labels', 'cover_relations'], None, None, (None, None, False)) Got: (['data', 'element_labels', 'cover_relations', 'category', 'facade', 'key'], None, None, (None, None, False, None, None, None))
This is already fixed "upstream". :)
comment:62 follow-up: 64 Changed 11 years ago by
Replying to ppurka:
Replying to dimpase:
applying the patches etc procedure for this ticket, and then #11080, followed by ptestlong, shows one failing doctest, in sagenb (which is just very outdated code in sagenb, as the current Poset has more keyargs than expected by the test). This needs to be fixed in sagenb, not here.
This is already fixed "upstream". :)
"upstream", please remove twisted 11 from sagenb! Indeed, after all this is installed I see:
sage: import twisted sage: twisted.version Version('twisted', 12, 0, 0)
fair enough, the installation log of sagenb-0.9.0 spkg (see #11080) has the following lines:
... Searching for Flask==0.8 Best match: Flask 0.8 Processing Flask-0.8-py2.7.egg Flask 0.8 is already the active version in easy-install.pth Using /usr/local/src/sage/sage-5.0.beta8/local/lib/python2.7/site-packages/Flask-0.8-py2.7.egg Searching for Twisted==12.0.0 Best match: Twisted 12.0.0 Processing Twisted-12.0.0-py2.7-macosx-10.6-x86_64.egg Removing Twisted 11.1.0 from easy-install.pth file Adding Twisted 12.0.0 to easy-install.pth file ...
so Twisted 11.1.0 need not be bundled, and in fact I suspect that many installations of Sage that are thought of running twisted 11 are actually running twisted 12!
Thus, either remove twisted 11 from the sagenb bundle, or keep it (if there areny reasons?) and make sure that it gets used!
comment:63 follow-up: 74 Changed 11 years ago by
AFAIK we just bundle things in the sagenb spkg so that installing the spkg doesn't require network access. So either we should bundle Twisted 12, or we should set a strict version dependency on twisted in setup.py, rather than an "at least" version dependency.
comment:64 follow-up: 65 Changed 11 years ago by
Replying to dimpase:
"upstream", please remove twisted 11 from sagenb! Indeed, after all this is installed I see:
sage: import twisted sage: twisted.version Version('twisted', 12, 0, 0)
I cannot reproduce this with the latest sage-5.0.beta8 + sagenb-0.9.0. It could be some race condition of course...
comment:65 follow-ups: 66 67 Changed 11 years ago by
Replying to jdemeyer:
Replying to dimpase:
"upstream", please remove twisted 11 from sagenb! Indeed, after all this is installed I see:
sage: import twisted sage: twisted.version Version('twisted', 12, 0, 0)I cannot reproduce this with the latest sage-5.0.beta8 + sagenb-0.9.0. It could be some race condition of course...
I got this two times, on Linux and on OSX. Please see http://boxen.math.washington.edu/home/dima/tmp/sagenb090install.log for how it went on my OSX system.
It could have happened that the machine you installed sagenb-0.9.0 was off the net, or easy_install was prevented from getting Twisted 12.0.0 in some other way.
comment:66 Changed 11 years ago by
Replying to dimpase:
Replying to jdemeyer:
Replying to dimpase:
"upstream", please remove twisted 11 from sagenb! Indeed, after all this is installed I see:
sage: import twisted sage: twisted.version Version('twisted', 12, 0, 0)I cannot reproduce this with the latest sage-5.0.beta8 + sagenb-0.9.0. It could be some race condition of course...
I got this two times, on Linux and on OSX. Please see http://boxen.math.washington.edu/home/dima/tmp/sagenb090install.log for how it went on my OSX system.
It could have happened that the machine you installed sagenb-0.9.0 was off the net, or easy_install was prevented from getting Twisted 12.0.0 in some other way.
FWIW, I get:
---------------------------------------------------------------------- | Sage Version 5.0.beta8, Release Date: 2012-03-13 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** The Sage installation tree may have moved (from /data/leif/Sage/sage-5.0.beta8 to /data/leif/Sage/sage-5.0.beta8-gcc-4.4.3). Changing various hardcoded paths... (Please wait at most a few minutes.) DO NOT INTERRUPT THIS. Done resetting paths. sage: import twisted sage: twisted.version Version('twisted', 9, 0, 0)
And this is the (spkg) version currently included in Sage, not the system-wide one, since that is 10.0.0:
$ python Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41) [GCC 4.4.3] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import twisted >>> twisted.version Version('twisted', 10, 0, 0)
Oh, I just noticed Sage 5.0.beta8 ships SageNB 0.8.27, so this might be irrelevant anyway.
comment:67 follow-ups: 68 71 Changed 11 years ago by
Replying to dimpase:
I got this two times, on Linux and on OSX. Please see http://boxen.math.washington.edu/home/dima/tmp/sagenb090install.log for how it went on my OSX system.
It could have happened that the machine you installed sagenb-0.9.0 was off the net, or easy_install was prevented from getting Twisted 12.0.0 in some other way.
I'm unable to reproduce this on Linux. From the sagenb090install.log there is
gcc version 4.2.1 (Apple Inc. build 5666) (dot 3) **************************************************** Processing Twisted-11.1.0.tar.bz2 Running Twisted-11.1.0/setup.py -q bdist_egg --dist-dir /var/folders/qW/qWY+4Ku1GF0WXrOsV+IDvk+++TM/-Tmp-/easy_install-09EQjq/Twisted-11.1.0/egg-dist-tmp-DCWyoE conftest.c:1:23: error: sys/epoll.h: No such file or directory Removing Twisted 12.0.0 from easy-install.pth file Adding Twisted 11.1.0 to easy-install.pth file Installing cftp script to /usr/local/src/sage/sage-5.0.beta8/local/bin
While I get
gcc version 4.5.3 (Gentoo 4.5.3-r1 p1.0, pie-0.4.5) **************************************************** Processing Twisted-11.1.0.tar.bz2 Running Twisted-11.1.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-52PPAo/Twisted-11.1.0/egg-dist-tmp-mqRKXh In file included from /storage/sage/sage-5.0.beta-nopatch/local/include/python2.7/Python.h:8:0, from twisted/internet/_sigchld.c:9: /storage/sage/sage-5.0.beta-nopatch/local/include/python2.7/pyconfig.h:1155:0: warning: "_POSIX_C_SOURCE" redefined /usr/include/features.h:210:0: note: this is the location of the previous definition Removing Twisted 9.0.0 from easy-install.pth file Adding Twisted 11.1.0 to easy-install.pth file Installing cftp script to /storage/sage/sage-5.0.beta-nopatch/local/bin
Doesn't the former suggest that Twisted 12.0.0 was installed prior to the install of sagenb-0.9.0? It would be interesting to know what version of Twisted is reported by an unpatched version of 5.0.beta8 with sagenb-0.8.27.
comment:68 follow-up: 69 Changed 11 years ago by
Replying to strogdon:
It would be interesting to know what version of Twisted is reported by an unpatched version of 5.0.beta8 with sagenb-0.8.27.
9.0.0, see above. Or did I misunderstand you?
comment:69 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:70 Changed 11 years ago by
The indication is that I changed the description? I never intended to do this and I'm not really sure if anything was changed. This is odd.
comment:71 follow-up: 73 Changed 11 years ago by
Replying to strogdon:
Doesn't the former suggest that Twisted 12.0.0 was installed prior to the install of sagenb-0.9.0?
well, I ran the installation of sagenb-0.9.0 more than once. The result was always the same. It seems I have uploaded the wrong part of the log.
It would be interesting to know what version of Twisted is reported by an unpatched version of 5.0.beta8 with sagenb-0.8.27.
If you followed the instructions on this ticket then you would not have ANY twisted before installing sagenb-0.9.0. At least in theory. I didn't check this. I will, but not right now. It's past 2am here.
comment:72 follow-up: 78 Changed 11 years ago by
Status: | needs_review → needs_work |
---|---|
Work issues: | → MANIFEST.in |
The file fpickle_setup.py
needs to be added to MANIFEST.in
comment:73 follow-up: 75 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_info |
Replying to dimpase:
Replying to strogdon:
Doesn't the former suggest that Twisted 12.0.0 was installed prior to the install of sagenb-0.9.0?
well, I ran the installation of sagenb-0.9.0 more than once. The result was always the same. It seems I have uploaded the wrong part of the log.
It would be interesting to know what version of Twisted is reported by an unpatched version of 5.0.beta8 with sagenb-0.8.27.
The mystery is solved :-) As sagenb-0.8.27.spkg was not removed, but twisted spkg was removed, during the installation of sagenb-0.8.27.spkg Twisted-12 was pulled from the net. Sorry for noise.
Thus, this needs to be coordinated with the removal of sagenb from Sage deps. Has this been already done in a patch?
If you followed the instructions on this ticket then you would not have ANY twisted before installing sagenb-0.9.0. At least in theory. I didn't check this. I will, but not right now. It's past 2am here.
comment:74 Changed 11 years ago by
Replying to kini:
AFAIK we just bundle things in the sagenb spkg so that installing the spkg doesn't require network access. So either we should bundle Twisted 12, or we should set a strict version dependency on twisted in setup.py, rather than an "at least" version dependency.
After looking at how the SPKG is produced, I think if Jason just produces the spkg again from exactly the same sagenb code it will now contain a copy of Twisted 12. So maybe the solution is to just have Jason produce the SPKG again.
comment:75 Changed 11 years ago by
comment:76 follow-up: 80 Changed 11 years ago by
You're right that if I made a new spkg, as things are arranged now, we'd get Twisted 12.0. The version that is currently up at #11080 should contain 11.something (11.1?), and we specifically install it with a switch to easy_install that says not to download things from the net.
It sounds like you figured out why 12.0 was being downloaded. Is this issue of accidentally getting 12.0 resolved now?
comment:77 follow-up: 79 Changed 11 years ago by
(By the way, you're right that is very easy for us to require Twisted 11.1 specifically, instead of just >=11.0 (which would pull 12.0). Should we specifically require 11.1, or is it okay to produce an spkg that has Twisted 12.0? It doesn't matter to me---whichever gets the notebook in faster.
comment:78 Changed 11 years ago by
Status: | needs_info → needs_review |
---|---|
Work issues: | MANIFEST.in |
Replying to jdemeyer:
The file
fpickle_setup.py
needs to be added toMANIFEST.in
Done. Ready for review now. (The twisted version issue is a non-issue, or at least not for this ticked, as it turned out).
comment:79 Changed 11 years ago by
Replying to jason:
(By the way, you're right that is very easy for us to require Twisted 11.1 specifically, instead of just >=11.0 (which would pull 12.0). Should we specifically require 11.1, or is it okay to produce an spkg that has Twisted 12.0? It doesn't matter to me---whichever gets the notebook in faster.
IMHO 12 is a bug-fixing release; they bumped the version as they dropped support for some very old Python. it's important to upgrade to 12 and test it ASAP, on *sagenb.org too.
comment:80 Changed 11 years ago by
Replying to jason:
You're right that if I made a new spkg, as things are arranged now, we'd get Twisted 12.0. The version that is currently up at #11080 should contain 11.something (11.1?), and we specifically install it with a switch to easy_install that says not to download things from the net.
It sounds like you figured out why 12.0 was being downloaded. Is this issue of accidentally getting 12.0 resolved now?
Yes it is. Twisted 12 was sneaking in as it was getting installed before the installation of the sagenb 0.9.0, and this was due to sagenb 0.8* left to build without twisted spkg (which it did by pulling Twisted 12 from the net). . When I replaced sagenb spkg in Sage 5.0.beta8 with 0.9.0, and then followed the procedure on #11874, I got Sage built with Twisted 11.1, instead of Twisted 12, as expected.
comment:81 follow-up: 82 Changed 11 years ago by
Status: | needs_review → needs_work |
---|
Apparently, editing MANIFEST.in
is not sufficient. The script devel/sage/spkg-dist
also needs to be changed.
comment:82 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
Replying to jdemeyer:
Apparently, editing
MANIFEST.in
is not sufficient. The scriptdevel/sage/spkg-dist
also needs to be changed.
alstublieft :)
Changed 11 years ago by
Attachment: | trac-11874-remove-twisted.patch added |
---|
apply to the sage root repository
comment:83 Changed 11 years ago by
Dependencies: | #12329, #11080 → #12329, #11080, #10492 |
---|---|
Description: | modified (diff) |
Rebased to #10492.
comment:84 follow-up: 85 Changed 11 years ago by
So, how is this supposed to work? I performed the following steps:
- Downloaded and compiled sage-5.0beta11
- Applied the two patches and removed twisted-9.*spkg from spkg/standard
- Reran
sage -b
- sage and sage -n seems to work fine. But sage still shows that twisted is present.
sage: import twisted sage: twisted.version Version('twisted', 9, 0, 0)
comment:85 follow-up: 86 Changed 11 years ago by
Replying to ppurka:
So, how is this supposed to work? I performed the following steps:
- Downloaded and compiled sage-5.0beta11
"Downloaded, but not compiled!" (c) J. Bond
You should download and then apply the patch to the sage-*.spkg, the patch to the root repository, and remove the twisted spkg. THEN compile.
comment:86 Changed 11 years ago by
Replying to dimpase:
You should download and then apply the patch to the sage-*.spkg, the patch to the root repository, and remove the twisted spkg. THEN compile.
Ok. This is what I have done now. I thought this was not the desired output:
...allations/sage-5.0.beta11> ./sage ---------------------------------------------------------------------- | Sage Version 5.0.beta11, Release Date: 2012-03-28 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** sage: import twisted sage: twisted.version Version('twisted', 12, 0, 0) sage: hg_sage('qser') cd "/home/punarbasu/Installations/sage-5.0.beta11/devel/sage" && sage --hg qser sage-spkg-11874.patch 0 sage: hg_root('qser') cd "/home/punarbasu/Installations/sage-5.0.beta11" && sage --hg qser trac-11874-remove-twisted.patch 0 sage:
To apply patch, I untarred the sage-5.0beta11.spkg, cd to sage-5.0beta11 and applied the patch using hg. Then I repackaged the directory into the spkg. And then I compiled.
Edit: Sorry, I see now that I should have used sagenb-0.9.0 in order to not pull in twisted-12.
comment:87 follow-up: 88 Changed 11 years ago by
Update: with the old sagenb it is unable to launch the notebook. So, I think this patch needs the new sagenb so as to be useful.
...allations/sage-5.0.beta11> ./sage -n directory=/tmp/a.sagenb ---------------------------------------------------------------------- | Sage Version 5.0.beta11, Release Date: 2012-03-28 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** Please wait while the Sage Notebook server starts... ... notebook(directory=r'''/tmp/a.sagenb''') Traceback (most recent call last): File "/home/punarbasu/Installations/sage-5.0.beta11/local/bin/sage-notebook", line 34, in <module> exec cmd File "<string>", line 1, in <module> File "/home/punarbasu/Installations/sage-5.0.beta11/devel/sagenb/sagenb/notebook/notebook_object.py", line 217, in __call__ return self.notebook(*args, **kwds) File "/home/punarbasu/Installations/sage-5.0.beta11/devel/sagenb/sagenb/notebook/run_notebook.py", line 202, in notebook_twisted nb = notebook.load_notebook(directory) File "/home/punarbasu/Installations/sage-5.0.beta11/devel/sagenb/sagenb/notebook/notebook.py", line 1809, in load_notebook import sagenb.notebook.twist File "/home/punarbasu/Installations/sage-5.0.beta11/devel/sagenb/sagenb/notebook/twist.py", line 42, in <module> from twisted.web2 import server, http, resource, channel ImportError: No module named web2
In a different installation, I applied this ticket, #11080 and #11503 and came up with a broken jsmath/mathjax. ( It hasn't been a pleasant day for sure :( )
comment:88 follow-up: 89 Changed 11 years ago by
comment:89 Changed 11 years ago by
Replying to jdemeyer:
Replying to ppurka:
Update: with the old sagenb it is unable to launch the notebook. So, I think this patch needs the new sagenb so as to be useful.
You might notice that #11080 is listed as dependency of this ticket...
Didn't notice this comment. Actually, this ticket is listed as a dependency in #11080. And the patch is also listed in the description of #11080. That said, it is this set of tickets that will need to be merged probably at the same time: #11080 #11874 #11078 #11503
comment:90 Changed 11 years ago by
Milestone: | sage-5.0 → sage-5.1 |
---|
comment:91 Changed 11 years ago by
I followed the installation instructions at #11080, but found that the "remove-twisted" patch from here fails to apply, at least to sage-5.0.prealpha0:
(sage subshell) linux-sqwp:sage-5.0.prealpha0 simon$ ./sage -hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11874/trac-11874-remove-twisted.patch Füge trac-11874-remove-twisted.patch zur Seriendatei hinzu SAGE_ROOT=/home/simon/SAGE/sage-5.0.prealpha0 (sage subshell) linux-sqwp:sage-5.0.prealpha0 simon$ ./sage -hg qpush Wende trac-11874-remove-twisted.patch an Wende Patch auf Datei spkg/install an FEHLSCHLAG von Teilstück #1 in Zeile 220 1 von 1 Teilstücken sind FEHLGESCHLAGEN -- speichere Ausschuss in Datei spkg/install.rej Wende Patch auf Datei spkg/standard/deps an FEHLSCHLAG von Teilstück #2 in Zeile 405 1 von 2 Teilstücken sind FEHLGESCHLAGEN -- speichere Ausschuss in Datei spkg/standard/deps.rej Patch schlug fehl und Fortsetzung unmöglich (versuche -v) Patch schlug fehl, Fehlerabschnitte noch im Arbeitsverzeichnis Fehler beim Anwenden. Bitte beheben und trac-11874-remove-twisted.patch aktualisieren
Here, one has
(sage subshell) linux-sqwp:sage-5.0.prealpha0 simon$ cat spkg/install.rej --- install +++ install @@ -221,7 +221,6 @@ SYMPY=`newest_version sympy` TACHYON=`newest_version tachyon` TERMCAP=`newest_version termcap` -TWISTED=`newest_version twisted` ZLIB=`newest_version zlib` ZNPOLY=`newest_version zn_poly` ZODB=`newest_version zodb3`
Indeed, the "TWISTED" variable is now defined differently, namely
TWISTED=`$newest twisted` export TWISTED
Easy to fix.
However, the rejection at spkg/standard/deps is worse:
(sage subshell) linux-sqwp:sage-5.0.prealpha0 simon$ cat spkg/standard/deps.rej --- deps +++ deps @@ -407,44 +406,43 @@ $(INST)/$(SAGE_ROOT_REPO): $(BASE) $(INST)/$(MERCURIAL) +$(PIPE) "$(SAGE_SPKG) $(SAGE_ROOT_REPO) 2>&1" "tee -a $(SAGE_LOGS)/$(SAGE_ROOT_REPO).log" -# setuptools forgets to update easy-install.pth during parallel -# builds, so we build the relevant packages serially. Note: To avoid -# branching, we haven't given explicit dependencies, but the chain's -# order is important. -$(INST)/$(SAGENB): $(BASE) $(INST)/$(SQLALCHEMY) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(SAGENB) 2>&1" "tee -a $(SAGE_LOGS)/$(SAGENB).log" - -$(INST)/$(SQLALCHEMY): $(BASE) $(INST)/$(SPHINX) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(SQLALCHEMY) 2>&1" "tee -a $(SAGE_LOGS)/$(SQLALCHEMY).log" - -$(INST)/$(SPHINX): $(BASE) $(INST)/$(JINJA2) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(SPHINX) 2>&1" "tee -a $(SAGE_LOGS)/$(SPHINX).log" - -$(INST)/$(JINJA2): $(BASE) $(INST)/$(PYGMENTS) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(JINJA2) 2>&1" "tee -a $(SAGE_LOGS)/$(JINJA2).log" - -$(INST)/$(PYGMENTS): $(BASE) $(INST)/$(ZODB) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(PYGMENTS) 2>&1" "tee -a $(SAGE_LOGS)/$(PYGMENTS).log" - -$(INST)/$(ZODB): $(BASE) $(INST)/$(TWISTED) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(ZODB) 2>&1" "tee -a $(SAGE_LOGS)/$(ZODB).log" - -$(INST)/$(TWISTED): $(BASE) $(INST)/$(PYTHON) $(INST)/$(PYTHON_GNUTLS) \ - $(INST)/$(PEXPECT) $(INST)/$(DOCUTILS) \ - $(INST)/$(SETUPTOOLS) $(INST)/$(PATCH) - +$(PIPE) "$(SAGE_SPKG) $(TWISTED) 2>&1" "tee -a $(SAGE_LOGS)/$(TWISTED).log" - +# setuptools forgets to update easy-install.pth during parallel builds, +# so we build the relevant packages serially. In order to enforce +# this, we artificially make every package in the list below depend on +# the package under it. +# # For reference and for when the setuptools problem is solved, here # are the actual dependencies: # -# sagenb: python pexpect twisted jinja2 sphinx docutils setuptools +# sagenb: python pexpect jinja2 sphinx docutils setuptools # sqlalchemy: python setuptools # sphinx: docutils jinja2 pygments # jinja2: python docutils setuptools # pygments: python setuptools -# twisted: python python_gnutls setuptools # zodb: python setuptools +$(INST)/$(SAGENB): $(BASE) $(INST)/$(SETUPTOOLS) $(INST)/$(SQLALCHEMY) \ + $(INST)/$(PEXPECT) $(INST)/$(JINJA2) $(INST)/$(SPHINX) \ + $(INST)/$(DOCUTILS) $(INST)/$(PATCH) + +$(PIPE) "$(SAGE_SPKG) $(SAGENB) 2>&1" "tee -a $(SAGE_LOGS)/$(SAGENB).log" + +$(INST)/$(SQLALCHEMY): $(BASE) $(INST)/$(SETUPTOOLS) $(INST)/$(SPHINX) $(INST)/$(PATCH) + +$(PIPE) "$(SAGE_SPKG) $(SQLALCHEMY) 2>&1" "tee -a $(SAGE_LOGS)/$(SQLALCHEMY).log" + +$(INST)/$(SPHINX): $(BASE) $(INST)/$(DOCUTILS) $(INST)/$(JINJA2) \ + $(INST)/$(PYGMENTS) $(INST)/$(PATCH) + +$(PIPE) "$(SAGE_SPKG) $(SPHINX) 2>&1" "tee -a $(SAGE_LOGS)/$(SPHINX).log" + +$(INST)/$(JINJA2): $(BASE) $(INST)/$(SETUPTOOLS) $(INST)/$(PYGMENTS) \ + $(INST)/$(PATCH) $(INST)/$(DOCUTILS) + +$(PIPE) "$(SAGE_SPKG) $(JINJA2) 2>&1" "tee -a $(SAGE_LOGS)/$(JINJA2).log" + +$(INST)/$(PYGMENTS): $(BASE) $(INST)/$(SETUPTOOLS) $(INST)/$(ZODB) $(INST)/$(PATCH) + +$(PIPE) "$(SAGE_SPKG) $(PYGMENTS) 2>&1" "tee -a $(SAGE_LOGS)/$(PYGMENTS).log" + +$(INST)/$(ZODB): $(BASE) $(INST)/$(SETUPTOOLS) $(INST)/$(PATCH) + +$(PIPE) "$(SAGE_SPKG) $(ZODB) 2>&1" "tee -a $(SAGE_LOGS)/$(ZODB).log" + ######################################################################## # List all *build-time* dependencies of the Sage library. These are, # on the one hand, programs needed for the build/install process of the
Does that mean "needs work", or are the instructions at #11080 misleading?
comment:92 Changed 11 years ago by
Everything should apply on the latest beta: sage-5.0.beta13 (see also the dependencies on this ticket!).
comment:93 Changed 11 years ago by
Apparently the instructions at #11080 where indeed misleading: It is not stated that one should start with sage-5.0.beta13. sage-5.0.prealpha0 does not suffice.
comment:94 Changed 11 years ago by
So Simon, does everything seem to work with a recent beta (or rc0)?
comment:96 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
everything works on Sage 5.0.
comment:97 Changed 11 years ago by
Milestone: | sage-5.1 → sage-5.2 |
---|
comment:98 Changed 10 years ago by
Merged in: | → sage-5.2.beta0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:99 Changed 10 years ago by
Merged in: | sage-5.2.beta0 |
---|---|
Milestone: | sage-5.2 → sage-pending |
Resolution: | fixed |
Status: | closed → new |
Unmerging this from sage-5.2 due to the serious security issue at #13270.
comment:100 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:101 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
comment:102 Changed 10 years ago by
Merged in: | → sage-5.2.beta0 |
---|---|
Milestone: | sage-pending → sage-5.2 |
Resolution: | → fixed |
Status: | positive_review → closed |
comment:103 Changed 7 years ago by
Authors: | Rado Kirov, Jason Grout → Radoslav Kirov, Jason Grout |
---|
See also #11497, which has a possible fix that we may need to apply, maybe?