Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update zeromq and pyzmq to recent version and new git layout #16455

Closed
sagetrac-tmonteil mannequin opened this issue Jun 7, 2014 · 37 comments
Closed

Update zeromq and pyzmq to recent version and new git layout #16455

sagetrac-tmonteil mannequin opened this issue Jun 7, 2014 · 37 comments

Comments

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Jun 7, 2014

The packages zero an pyzmq are required to let IPython notebook work within Sage. There seems to be a consensus to have those as standard packages. See also #16053.

The upstream tarballs can be found at :

CC: @seblabbe @vbraun @williamstein @dimpase

Component: packages: optional

Keywords: ipython

Author: Thierry Monteil

Branch: 184da0c

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/16455

@sagetrac-tmonteil sagetrac-tmonteil mannequin added this to the sage-6.3 milestone Jun 7, 2014
@sagetrac-tmonteil sagetrac-tmonteil mannequin added the p: major / 3 label Jun 7, 2014
@sagetrac-tmonteil

This comment has been minimized.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 7, 2014

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 7, 2014

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

524b2d2#16455 : pyzmq new git layout + update to version 14.3.0 + remove old-style-spkg changelog + update buildutils patch.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 7, 2014

Commit: 524b2d2

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 7, 2014

comment:4

Sebastien, i modified the patch that could not be applied anymore, please can you check whether this still works on Darwin ?

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 7, 2014

Changed keywords from none to ipython

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 7, 2014

Author: Thierry Monteil

@sagetrac-tmonteil

This comment has been minimized.

@williamstein
Copy link
Contributor

comment:5

For what it is worth, I strongly support including IPython notebook (and deps) in Sage standard. -- William

@seblabbe
Copy link
Contributor

comment:6

I get this after installing your branch :

10 slabbe@pol ~/Applications/sage-review $ ./sage -f http://download.zeromq.org/zeromq-4.0.4.tar.gz
Attempting to download package zeromq-4.0.4.tar.gz
>>> Trying to download http://download.zeromq.org/zeromq-4.0.4.tar.gz
[............................................................]
zeromq-4.0.4.tar.gz
====================================================
Extracting package /Users/slabbe/Applications/sage-review/upstream/zeromq-4.0.4.tar.gz
-rw-r--r--  1 slabbe  staff  2149732 Jun 10 08:55 /Users/slabbe/Applications/sage-review/upstream/zeromq-4.0.4.tar.gz
Finished extraction
/Users/slabbe/Applications/sage-review/src/bin/sage-spkg: line 594: cd: zeromq-4.0.4.tar.gz: No such file or directory
Error: after extracting, the directory zeromq-4.0.4.tar.gz does not exist

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 10, 2014

comment:7

The tarball you found at http://download.zeromq.org/zeromq-4.0.4.tar.gz is not a sage package, it is the source code of zeromq. You have to put this tarball on your /Users/slabbe/Applications/sage-review/upstream/ directory, and then do ./sage -f zeromq.

@seblabbe
Copy link
Contributor

comment:8

Replying to @sagetrac-tmonteil:

The tarball you found at http://download.zeromq.org/zeromq-4.0.4.tar.gz is not a sage package, it is the source code of zeromq. You have to put this tarball on your /Users/slabbe/Applications/sage-review/upstream/ directory, and then do ./sage -f zeromq.

But the above put the tarball in the upstream directory. So that means I can do :

./sage -i http://download.zeromq.org/zeromq-4.0.4.tar.gz   # download in upstream
./sage -i zeromq

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 10, 2014

comment:9

The first command will lead to an error, since it not only tries to download the tarball in upstream/, but also tries to install it as a sage spkg, explaining your error, hence you should consider using wget ot curl for that part of the review. Of course, once the ticket is merged, there is no need to download the tarball manually since sage will find it at http://sagemath.org/packages/upstream/

That said, after that error, the second command should work, since zeromq-4.0.4.tar.gz is now in the upstream/ directory.

@seblabbe
Copy link
Contributor

comment:10

Ok, so I just tested and it does work on my machine. I also managed to open the ipython notebook with these.

I also tested without the buildutils patch on pyzmq, and sage -i pyzmq also worked. Indeed, I remembered I read somewhere (maybe on sage-devel) that my patch was now useless since the problem was fixed in a later release... I tried to find where I read that, but I was not able.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 10, 2014

comment:11

OK, once you confirm that the patch is not needed anymore, i will remove it from the branch.

@seblabbe
Copy link
Contributor

comment:12

With the following modifications:

~/Applications/sage-review $ git diff
diff --git a/build/pkgs/pyzmq/spkg-install b/build/pkgs/pyzmq/spkg-install
index c7e1216..269f055 100755
--- a/build/pkgs/pyzmq/spkg-install
+++ b/build/pkgs/pyzmq/spkg-install
@@ -13,16 +13,6 @@ echo "include_dirs = $SAGE_LOCAL/include/" >> src/setup.cfg
 
 cd src
 
-# Apply patches.  See SPKG.txt for information about what each patch
-# does.
-for patch in ../patches/*.patch; do
-    patch -p1 <"$patch"
-    if [ $? -ne 0 ]; then
-        echo >&2 "Error applying '$patch'"
-        exit 1
-    fi
-done
-
 # Configure and install
 python setup.py configure --zmq="$SAGE_LOCAL"
 python setup.py install

I confirm that it works:

./sage -f pyzmq
Found local metadata for pyzmq-14.3.0

[...]

Successfully installed pyzmq-14.3.0
Deleting temporary build directory
Finished installing pyzmq-14.3.0.spkg

@seblabbe
Copy link
Contributor

comment:13

remove the patch for pyzmq

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 10, 2014

Changed commit from 524b2d2 to 184da0c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 10, 2014

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

184da0c#16455 remove useless buildutils patch

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 10, 2014

Reviewer: ​Sébastien Labbé

@seblabbe
Copy link
Contributor

comment:16

One question before giving a positive review. Can the spkg-install of pyzmq check that zeromq spkg was installed before? But maybe it is not a good idea, and it is better to let the installation breaks by itself....

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jun 10, 2014

comment:17

To my understanding, the dependencies are set via the build/deps file, but it seems to be reserved to standard packages only. Volker, is this correct ?

That said, since those packages are optional for more than one year and many people want them to become standard, i plan to make a ticket for that.

@vbraun
Copy link
Member

vbraun commented Jun 10, 2014

comment:18

We don't have a dependency mechanism for optional packaeges yet. But please don't roll your own just for zeromq...

@jhpalmieri
Copy link
Member

comment:20

Some optional packages do some dependency checking in their spkg-install, for example the p_group_cohomology package does this:

SMALL_GROUPS=`echo "SmallGroup(13,1); quit;" | gap -b -T | grep "13"`
if [ "$SMALL_GROUPS" = "" ]; then
   echo "It seems that GAP's SmallGroups library is missing."
   echo "One way to install it is by doing"
   echo "    sage: install_package('database_gap')"
   echo "in a Sage session."
   exit 1
fi

Will installation of pyzmq fail gracefully if zeromq is missing, or is it better to have a check like this?

@seblabbe
Copy link
Contributor

comment:21

someone installing pyzmq before zeromq will get something like :

******************************************
Configure: Autodetecting ZMQ settings...
    Custom ZMQ dir:       /Users/slabbe/Applications/sage-git/local
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -I/Users/slabbe/Applications/sage-git/local/include -Izmq/utils -Izmq/core -Izmq/devices -c detect/vers.c -o detect/vers.o
detect/vers.c:3:17: fatal error: zmq.h: No such file or directory
compilation terminated.
Fatal: 
    Failed to compile ZMQ test program.  Please check to make sure:

    * You have a C compiler installed
    * A development version of Python is installed (including header files)
    * A development version of ZMQ >= 2.1.4 is installed (including header files)
    * If ZMQ is not in a default location, supply the argument --zmq=<path>
    * If you did recently install ZMQ to a default location, 
      try rebuilding the ld cache with `sudo ldconfig`
      or specify zmq's location with `--zmq=/usr/local`
    
error: command 'gcc' failed with exit status 1
******************************************

real	0m0.622s
user	0m0.259s
sys	0m0.335s
************************************************************************
Error installing package pyzmq-2.1.11.p1
************************************************************************

@seblabbe
Copy link
Contributor

comment:22

Anyway, these two packages are aimed to become standard soon if I understand correctly, so their dependencies will be set via the build/deps as mentionned before. So I believe it OK like that without any dependency checking.

@vbraun
Copy link
Member

vbraun commented Jun 17, 2014

comment:23

delete invisible space

@vbraun
Copy link
Member

vbraun commented Jun 17, 2014

Changed reviewer from ​Sébastien Labbé to Sébastien Labbé

@vbraun
Copy link
Member

vbraun commented Jun 18, 2014

@kcrisman
Copy link
Member

Changed commit from 184da0c to none

@kcrisman
Copy link
Member

comment:25

OK, once you confirm that the patch is not needed anymore, i will remove it from the branch.

Who confirmed that this was useless? This will still not work on older Macs - see the actual note that was in the SPKG.txt!!!

   - patches/buildutils.patch: This patches the file src/buildutils/detect.py.
     It remove the option -arch for gcc which was broken on MacOSX 10.5.8.
     The reason to this is that Sage's gcc spkg does not build a compiler
     which understands the option -arch of Apple's clone of gcc.
     https://groups.google.com/forum/?fromgroups#!topic/sage-devel/CuZNKclprIQ
     https://github.com/sagemath/sage-prod/issues/13313

And now that this is standard with all the big changes at the end of 6.4, it breaks some of them. And since the machine I am currently trying to build Sage on does not have git, nor has git built yet, I will have to type this entire patch in BY HAND, or nearly so.

@seblabbe
Copy link
Contributor

comment:26

Who confirmed that this was useless?

Me. My machine is a MacOSX 10.5.8. I have read that note in the SPKG. When I did the review, I confirmed that the patch was needed with the earlier spkg but not with the new one which I gave a positive review.

I am sorry for having cause you trouble today. Maybe we should add more documentation in the patch to re-add saying for what machine exactly it is mandatory. Because to me, it is not needed for 10.5.8 anymore.

Sébastien

@kcrisman
Copy link
Member

comment:27

Me. My machine is a MacOSX 10.5.8. I have read that note in the SPKG. When I did the review, I confirmed that the patch was needed with the earlier spkg but not with the new one which I gave a positive review.

Oh! And indeed if I had been more careful I would have seen that you were responsible for #13313 and so knew what you were talking about.

I am sorry for having cause you trouble today. Maybe we should add more documentation in the patch to re-add saying for what machine exactly it is mandatory. Because to me, it is not needed for 10.5.8 anymore.

I apologize also, because I was snippy in my comment. I had left the machine compiling all night, only to discover this (relatively early) error.

I think that the question is in when they add these flags. What do you get for

import platform
platform.architecture()

on my Lion computer (10.7) I get ('64bit', ''). I will see what I get on my other one. My guess is that probably Dima's machine was already building a 64-bit Python (see e.g. the SAGE64 flag), and that in the meantime maybe your 10.5 computer also is doing that, maybe we are building Python 64-bit by default or you are using SAGE64='yes' all the time on that computer? In principle if you don't have the patch but you get if platform.architecture()[0]=='32bit': then you should be getting the bad Apple-gcc-only flags.

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Dec 15, 2014

comment:28

@kcrisman: I pinged Sebastien on that ticket because he was the author of the patch and i have no mac to test. If there is some problem with some particular version of OSX, i think this is worth opening a new ticket for that issue now, or the information you will provide on the current thread will be less visible there.

@kcrisman
Copy link
Member

comment:29

Naturally, just wanting to confirm before opening a new ticket. See #17510.

@seblabbe
Copy link
Contributor

comment:30

No problem. I should have thought of adding you in cc of this ticket by the time of the review, since I remember that #13313 was also useful for you.

@dimpase
Copy link
Member

dimpase commented Dec 15, 2014

comment:31

Replying to @kcrisman:

Me. My machine is a MacOSX 10.5.8. I have read that note in the SPKG. When I did the review, I confirmed that the patch was needed with the earlier spkg but not with the new one which I gave a positive review.

Oh! And indeed if I had been more careful I would have seen that you were responsible for #13313 and so knew what you were talking about.

I am sorry for having cause you trouble today. Maybe we should add more documentation in the patch to re-add saying for what machine exactly it is mandatory. Because to me, it is not needed for 10.5.8 anymore.

I apologize also, because I was snippy in my comment. I had left the machine compiling all night, only to discover this (relatively early) error.

I think that the question is in when they add these flags. What do you get for

import platform
platform.architecture()

on my Lion computer (10.7) I get ('64bit', ''). I will see what I get on my other one. My guess is that probably Dima's machine was already building a 64-bit Python (see e.g. the SAGE64 flag), and that in the meantime maybe your 10.5 computer also is doing that, maybe we are building Python 64-bit by default or you are using SAGE64='yes' all the time on that computer?

When I reported that (cf the link to sage-devel in #13313) I tried that on an PPC OSX 10.5.8 machine, which is 32-bit all the way. I don't think I did build anything with SAGE64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants