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

python spkg build fails on various Debian systems #11447

Closed
pipedream opened this issue Jun 8, 2011 · 55 comments
Closed

python spkg build fails on various Debian systems #11447

pipedream opened this issue Jun 8, 2011 · 55 comments

Comments

@pipedream
Copy link

Ubuntu 11.04 derivative Mint 11 and other newer Debian GNU/Linux derivatives fail to build the Python spkg.

This is a known issue in #11243 on Ubuntu 11.04, and any derivatives such as Mint 11 will suffer from this as well.


Updated spkg: http://spkg-upload.googlecode.com/files/python-2.6.4.p11.spkg

md5sum: e808dc5edba8c82c098f0c9641b34634 python-2.6.4.p11.spkg

(With Jan Medlock's patch of June 17th applied, and now also attachment: trac_11447-give_hint_on_dpkg-dev.patch. The old one with only Jan Medlock's patch applied had md5sum 5bc63c1814fc7ae34f4fed5a8fffe380.)


No patches need to be applied to the Sage library - only the package updated. The patches attached to the ticket are for review purposes only.

CC: @sagetrac-drkirkby @jpflori

Component: packages: standard

Keywords: python spkg crypt library

Author: Jan Medlock

Reviewer: David Kirkby, Bill Odefey, Jan Groenewald

Merged: sage-4.7.1.rc0

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

@pipedream
Copy link
Author

Attachment: 11447-python-mint11.patch.gz

Patch for building crypt module on Linux Mint 11

@pipedream
Copy link
Author

comment:1

Please test http://users.aims.ac.za/~jan/python-2.6.4.p11.spkg
on Ubuntu 11.04, Linux Mint 11, and other systems.

( Dave: I changed a grep to egrep. Is this POSIX? Is grep -E better?
Is splitting it into two separate greps better? )

@kcrisman
Copy link
Member

kcrisman commented Jun 8, 2011

comment:3

Jan, if you want to make sure someone sees a ticket, you will need to cc: them like I am doing with drkirkby now.

You could also put the person for whom it worked properly (as you mentioned on sage-devel) as a reviewer - why not? They helped, after all, even if they can't check that the code is fine (which it certainly seems to me, but I don't know what etc/issue is for sure).

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jun 8, 2011

Work Issues: Remove extra | character. Create a complete package.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jun 8, 2011

comment:4

These should not be two '|' characters in the egrep expression.

drkirkby@hawk:~$ echo foo | egrep "Ubuntu 11.04||Linux Mint 11 Katya"
egrep: syntax error
drkirkby@hawk:~$ 

There should be only one.

You also need to create a new package, so we can test the complete package, but this looks pretty simple to me.

It would be good if we could attempt to find other popular Ubuntu derivatives that will get the problem and include the patch for them too. But that's not important - just nice if possible to do easily.

Dave

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jun 8, 2011

Reviewer: David Kirkby

@pipedream
Copy link
Author

comment:5

Thanks Dave, new patch following. I made a typo with the pipe, but it
did not throw a syntax error on Ubuntu (I think it parsed
"Ubuntu 11.04" or "" or "Linux Mint 11 Katya". The empty string matched
every line though which is another problem. This is however why
the spkg compiled properly for the Linux Mint tester/reviewer.

I'm not sure he has a trac account (for reviewing).

New patch attached for review purposes, and spkg at
http://users.aims.ac.za/~jan/python-2.6.4.p11.spkg

@pipedream
Copy link
Author

Replaces previous patch to build on Linux Mint 11

@kcrisman
Copy link
Member

kcrisman commented Jun 9, 2011

comment:6

Attachment: 11447-python-mint11b.patch.gz

I'm not sure he has a trac account (for reviewing).

What I meant was that you could put his name down as a reviewer, assuming you do not have actual access to a Mint machine and neither does David.

@pipedream
Copy link
Author

Changed reviewer from David Kirkby to David Kirkby, Bill Odefey

@pipedream
Copy link
Author

comment:8

Bill Odefey reports that the new spkg worked for him again on Linux Mint 11.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jun 10, 2011

comment:9

egrep is not a POSIX command, but I think in this instance it is acceptable to use it.

The use of 'grep -E' confirms to the latest POSIX, but would fail on Solaris if someone has /usr/bin in their path before /usr/xpg4/bin. This is because to maintain backwards compatibility with older Solaris releases, whilst still conforming to the various POSIX versions, there are two or sometimes three versions of some commands on Solaris.

More portable would be to split the two greps up, but I think that would unnecessarily complicate things.

Positive review.

@sagetrac-drkirkby

This comment has been minimized.

@jdemeyer
Copy link

Changed work issues from Remove extra | character. Create a complete package. to none

@jdemeyer
Copy link

comment:10

Looking at /etc/issue is a very bad way of checking the system. This is a file which is sometimes customized by system administrators. You should try to understand why Python doesn't build on such a system and check for the true cause.

@pipedream
Copy link
Author

comment:11

Yes, it is hidden in the original thread which lead to ticket #11243.
#11243
http://groups.google.com/group/sage-devel/browse_thread/thread/593b9a4124f5075d

If the way to check for Linux Release/Distribution it is suboptimal, do you know of a better way?
(It is a patch that Debian and Ubuntu and therefore all derivatives apply to their own python debs for python to build.)

It comes down to this http://bugs.python.org/issue9762 .
It may be fixed in python 2.7 http://hg.python.org/cpython/rev/bd0f73a9538e
but those are beyond my experience.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title python spkg build fails on Ubuntu 11.04 derivative Mint 11 python spkg build fails on various Debian systems Jul 5, 2011
@jdemeyer
Copy link

jdemeyer commented Jul 5, 2011

comment:27

I will test the new package on the buildbots and give positive_review if these are successful.

@jdemeyer
Copy link

jdemeyer commented Jul 5, 2011

Changed keywords from crypt library to python spkg crypt library

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:29

Replying to @jdemeyer:

I will test the new package on the buildbots and give positive_review if these are successful.

So adding a note on installing dpkg-dev to the "Known Issues" / wiki page(s) is sufficient? (I can't judge how common it is to not have it installed.)

It's perhaps to specific to get into the Sage Installation Guide.

P.S.: You don't need to add keywords that are already in the ticket's title.

@pipedream
Copy link
Author

comment:30

It would be great if the patch included a suggestion to "sudo apt-get install dpkg-dev" if it is not installed.

@pipedream pipedream assigned jdemeyer and unassigned pipedream Jul 5, 2011
@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:32

P.S.:

To detect Debian and any of its derivatives (in order to give an appropriate message if importing crypt fails) testing command -v dpkg should suffice.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:33

Replying to @pipedream:

It would be great if the patch included a suggestion to "sudo apt-get install dpkg-dev" if it is not installed.

Patch is on the way...

Cannot really test it myself though, as I have no newer Debian here.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

SPKG patch. Adds message, some minor changes. Apply on top of Jan Medlock's patch.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:34

Attachment: trac_11447-give_hint_on_dpkg-dev.patch.gz

Patch is up. Relevant part:

diff --git a/spkg-install b/spkg-install
--- a/spkg-install
+++ b/spkg-install
@@ -242,12 +248,28 @@
 # All these modules are important and if any one 
 # fails to build, Sage will not work. 
 
+import_errors=false
 for module in math hashlib crypt ; do 
    "$SAGE_LOCAL/bin/python" -c "import $module"
    if [ $? -eq 0 ] ; then
       echo "$module module imported OK"
    else
-      echo "$module module failed to import"
-      exit 1
+      echo >&2 "$module module failed to import"
+      import_errors=true
+      # exit 1 # not yet
    fi
 done
+
+if $import_errors; then
+    echo >&2 "Error: One or more modules failed to import."
+    # Check if we are on Debian or one of its derivatives:
+    if command -v dpkg && ! command -v dpkg-architecture >/dev/null; then
+        echo >&2 "You may have to install 'dpkg-architecture'"
+        echo >&2 "which is part of the Debian package 'dpkg-dev'."
+        echo >&2 "Try installing it by typing"
+        echo >&2 "    sudo apt-get install dpkg-dev"
+        echo >&2 "and rerun 'make'."
+    fi
+    exit 1
+fi

I haven't uploaded the new spkg to Ggle code yet.**

Maybe Jeroen wants to create it on sage.math. (My patch attachment: trac_11447-give_hint_on_dpkg-dev.patch is based on the p11 I previously uploaded.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:35

Ooops, I just noticed the output of command -v dpkg is not redirected. But IMHO negligible.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:37

Replying to @nexttime:

I haven't uploaded the new spkg to Ggle code yet.**

Ok, will do now...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 5, 2011

comment:38

Replying to @nexttime:

Replying to @nexttime:

I haven't uploaded the new spkg to Ggle code yet.**

Ok, will do now...

Done. New md5sum: e808dc5edba8c82c098f0c9641b34634 (Same name, same place.)

@nexttime

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Jul 7, 2011

comment:39

Messy in various places, but at least it improves over the existing version, so positive_review.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 7, 2011

comment:40

Replying to @jdemeyer:

Messy in various places,

Details, please. ;-)

@jdemeyer
Copy link

jdemeyer commented Jul 7, 2011

comment:41

Replying to @nexttime:

Replying to @jdemeyer:
Details, please. ;-)

if command -v dpkg && ! command -v dpkg-architecture >/dev/null; then

should be

if command -v dpkg >/dev/null && ! command -v dpkg-architecture >/dev/null; then

or

if ( command -v dpkg && ! command -v dpkg-architecture ) >/dev/null; then

Mixed usage of cp of patch, better always use patch.

On SunOS, you first patch setup.py and then copy a different version over it.

Patching is better done using one for loop to prevent an error-prone laundry list of patches to be applied.

Why call ./configure with arguments CC="$CC $CFLAGS" CXX="$CXX $CXXFLAGS"? Any why only on certain systems?

MAKE=make; export MAKE
make -i install

should be

$MAKE -j1 -i install

Remove this:

# Do not exit script if there is an error, but instead print an
# informative error message. This is helps in determining why the
# configuration, compilation or installation failed. So put this before the
# build() function.
set +e # This is redundant here, but doesn't hurt to keep it... ;-)

Why this:

echo "Sleeping for three seconds before testing python"
sleep 3

I guess EXTRAFLAGS is only used inside the spkg-install script, so why export it?

On SunOS, do we need the configure flag --with-gcc="gcc -m64" given that -m64 is already inside $OPT? Or, if $OPT does nothing, why specify $OPT?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 7, 2011

comment:42

Replying to @jdemeyer:

Replying to @nexttime:

Replying to @jdemeyer:
Details, please. ;-)

if command -v dpkg && ! command -v dpkg-architecture >/dev/null; then

should be

if command -v dpkg >/dev/null && ! command -v dpkg-architecture >/dev/null; then

Yes, I mentioned on the ticket that I forgot it; minor.

Mixed usage of cp of patch, better always use patch.

Agreed, but didn't want to change too much, since it is a blocker and needs quick review.

On SunOS, you first patch setup.py and then copy a different version over it.

No idea. Ask Dave. (The real patch is only necessary on Debian Linuces, so at least that shouldn't break anything.)

Patching is better done using one for loop to prevent an error-prone laundry list of patches to be applied.

Hmmm, depends on the situation. Order might matter, and not all patches are applied on all systems. You'd also have to delete (or rename) patches that [currently] don't get applied anymore. Having a bit of redundancy in spkg-install and SPKG.txt also isn't that bad, as it should reduce the risk of unintentional changes.

But we still have the cps in there anyway, so this should be changed when all patches get real patches.

Why call ./configure with arguments CC="$CC $CFLAGS" CXX="$CXX $CXXFLAGS"? Any why only on certain systems?

Guess that's some libtool issue, which sometimes drops e.g. -m64, but apparently not on all systems (or it doesn't matter on all).

MAKE=make; export MAKE
make -i install

should be

$MAKE -j1 -i install

I know. I always get beaten when I change such, as some people consider it to be a "dangerous change"... (I put a comment on that in.)

Remove this:

# Do not exit script if there is an error, but instead print an
# informative error message. This is helps in determining why the
# configuration, compilation or installation failed. So put this before the
# build() function.
set +e # This is redundant here, but doesn't hurt to keep it... ;-)

Orrr.

Why this:

echo "Sleeping for three seconds before testing python"
sleep 3

See above ("dangerous change"). I added a comment that this is certainly no longer necessary, on the other hand it doesn't hurt much.

I guess EXTRAFLAGS is only used inside the spkg-install script, so why export it?

Typical stupidity. Only one of N instances of that.

On SunOS, do we need the configure flag --with-gcc="gcc -m64" given that -m64 is already inside $OPT? Or, if $OPT does nothing, why specify $OPT?

That's again related to libtool I think.


If you come across such things and don't want to fix them immediately, it's IMHO best to simply put comments into the files or SPKG.txt. Comments don't hurt and don't have to be tested, so no reviewer should complain about them (as opposed to too many or "difficult" functional changes).

Also, the next one touching the spkg (or files in general) hopefully will take them into account, and doesn't have to search trac for older tickets with open issues.

@jdemeyer
Copy link

jdemeyer commented Jul 8, 2011

Merged: sage-4.7.1.rc0

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

4 participants