Opened 10 years ago

Closed 10 years ago

#11447 closed defect (fixed)

python spkg build fails on various Debian systems

Reported by: pipedream Owned by: jdemeyer
Priority: blocker Milestone: sage-4.7.1
Component: packages: standard Keywords: python spkg crypt library
Cc: drkirkby, jpflori Merged in: sage-4.7.1.rc0
Authors: Jan Medlock Reviewers: David Kirkby, Bill Odefey, Jan Groenewald
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by leif)

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 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.

Attachments (5)

11447-python-mint11.patch (1.9 KB) - added by pipedream 10 years ago.
Patch for building crypt module on Linux Mint 11
11447-python-mint11b.patch (1.9 KB) - added by pipedream 10 years ago.
Replaces previous patch to build on Linux Mint 11
deb-setup.diff (1.6 KB) - added by medlock 10 years ago.
Debian patch to Python 2.6 to fix paths
11447-Python-Debian-multiarch.patch (5.0 KB) - added by medlock 10 years ago.
Patch to python-2.6.4.p10.spkg to fix multiarch build. Solution copied from Python 2.7.
trac_11447-give_hint_on_dpkg-dev.patch (9.3 KB) - added by leif 10 years ago.
SPKG patch. Adds message, some minor changes. Apply on top of Jan Medlock's patch.

Download all attachments as: .zip

Change History (48)

Changed 10 years ago by pipedream

Patch for building crypt module on Linux Mint 11

comment:1 Changed 10 years ago by pipedream

  • Status changed from new to needs_review

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? )

comment:2 Changed 10 years ago by pipedream

  • Owner changed from GeorgSWeber to pipedream

comment:3 Changed 10 years ago by kcrisman

  • Cc drkirkby added

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).

comment:4 Changed 10 years ago by drkirkby

  • Reviewers set to David Kirkby
  • Status changed from needs_review to needs_work
  • Work issues set to Remove extra | character. Create a complete package.

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

comment:5 follow-up: Changed 10 years ago by pipedream

  • Status changed from needs_work to needs_review

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

Changed 10 years ago by pipedream

Replaces previous patch to build on Linux Mint 11

comment:6 in reply to: ↑ 5 Changed 10 years ago by kcrisman

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.

comment:7 Changed 10 years ago by pipedream

  • Reviewers changed from David Kirkby to David Kirkby, Bill Odefey

comment:8 Changed 10 years ago by pipedream

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

comment:9 Changed 10 years ago by drkirkby

  • Description modified (diff)
  • Status changed from needs_review to positive_review

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.

comment:10 Changed 10 years ago by jdemeyer

  • Work issues Remove extra | character. Create a complete package. deleted

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.

comment:11 follow-up: Changed 10 years ago by pipedream

Yes, it is hidden in the original thread which lead to ticket #11243. http://trac.sagemath.org/sage_trac/ticket/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.

comment:12 in reply to: ↑ 11 ; follow-up: Changed 10 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Replying to pipedream:

If the way to check for Linux Release/Distribution? it is suboptimal, do you know of a better way?

Well, it is dead simple: don't check the Linux Release/Distribution?. Would it make sense to apply the workaround on all Linux systems or could that break things?

comment:13 in reply to: ↑ 12 ; follow-up: Changed 10 years ago by medlock

Replying to jdemeyer:

Replying to pipedream:

If the way to check for Linux Release/Distribution? it is suboptimal, do you know of a better way?

Well, it is dead simple: don't check the Linux Release/Distribution?. Would it make sense to apply the workaround on all Linux systems or could that break things?

The solution that's in Python 2.7 and in Debian's patches to Python 2.6 (patch deb-setup.diff attached) is to use dpkg-architecture -qDEB_HOST_MULTIARCH. These packages use this command to set the library paths correctly.

This requires that the optional package dpkg-dev is installed. A wrapper should probably be written that complains if lsb_release -i is (Debian|Ubuntu|Mint) and dpkg-architecture is not present, otherwise the build will fail without the user knowing why...

comment:14 in reply to: ↑ 13 ; follow-up: Changed 10 years ago by medlock

Replying to medlock:

... The solution that's in Python 2.7 and in Debian's patches to Python 2.6 (patch deb-setup.diff attached) is to use dpkg-architecture -qDEB_HOST_MULTIARCH. These packages use this command to set the library paths correctly.

This requires that the optional package dpkg-dev is installed. A wrapper should probably be written that complains if lsb_release -i is (Debian|Ubuntu|Mint) and dpkg-architecture is not present, otherwise the build will fail without the user knowing why...

All this can get ripped out when Sage upgrades to Python 2.7, where this issue is fixed...

Changed 10 years ago by medlock

Debian patch to Python 2.6 to fix paths

comment:15 in reply to: ↑ 14 Changed 10 years ago by medlock

Replying to medlock:

Replying to medlock:

... The solution that's in Python 2.7 and in Debian's patches to Python 2.6 (patch deb-setup.diff attached) is to use dpkg-architecture -qDEB_HOST_MULTIARCH. These packages use this command to set the library paths correctly.

This requires that the optional package dpkg-dev is installed. A wrapper should probably be written that complains if lsb_release -i is (Debian|Ubuntu|Mint) and dpkg-architecture is not present, otherwise the build will fail without the user knowing why...

All this can get ripped out when Sage upgrades to Python 2.7, where this issue is fixed...

See also http://lists.debian.org/debian-devel/2011/06/msg00431.html.

comment:16 Changed 10 years ago by pipedream

Note that in #11243 we realised lsb_release does not work -- it tries to import an underlying system python package; but SAGE spkg installs run in a SAGE python environment and does not know the path to import lsb_release. That is why we switched to check whether /etc/issue exists, and then to use that.

It is a good question whether the patch to use -lcrypt will break other Linux distros? Then a standard uname check for Linux could be used and the patch applied on all Linux patched while SAGE still uses python 2.6.

What is the timeline for SAGE to get to python 2.7?

Changed 10 years ago by medlock

Patch to python-2.6.4.p10.spkg to fix multiarch build. Solution copied from Python 2.7.

comment:17 follow-up: Changed 10 years ago by medlock

11447-Python-Debian-multiarch.patch uses the solution from Python 2.7 to fix build on Debian and derivatives with multiarch.

The patch to python-2.6.4.p10/src/setup.py is applied on all hosts, not just Debian and derivatives, but the new code will end up doing nothing on non-Debian-derived hosts. This is how the build is set up in Python 2.7.

Note that on Debian and derivatives, if the command dpkg-architecture, from the package dpkg-dev, is not available, the build will fail as before. Again, this is how Python 2.7 is set up.

This looks like the best solution to me.

comment:18 follow-up: Changed 10 years ago by pipedream

  • Status changed from needs_work to needs_review

Looks good to me. This does mean -lcrypt in Setup.dist is uncommented on all systems, Debian or not, Linux or not. Does that have unintended side effects?

comment:19 Changed 10 years ago by pipedream

  • Authors changed from Jan Groenewald to Jan Medlock
  • Reviewers changed from David Kirkby, Bill Odefey to David Kirkby, Bill Odefey, Jan Groenewald

comment:20 in reply to: ↑ 18 Changed 10 years ago by medlock

Replying to pipedream:

Looks good to me. This does mean -lcrypt in Setup.dist is uncommented on all systems, Debian or not, Linux or not. Does that have unintended side effects?

No, -lcrypt is not uncommented in Setup.dist on any system.

The library search directories are now set up in setup.py on Debian and derivatives to correctly find libcrypt automatically. There is no change to the library search directories on other systems. This is done by testing for the presence of the command 'dpkg-architecture'.

I don't see any side effects and this is how it is done in Python 2.7.

(Setup.dist is a work around for setup.py not automatically finding the libraries.)

comment:21 Changed 10 years ago by jpflori

  • Cc jpflori added

comment:22 in reply to: ↑ 17 ; follow-ups: Changed 10 years ago by leif

  • Keywords crypt library added
  • Status changed from needs_review to needs_work
  • Work issues set to Provide an updated spkg

Replying to medlock:

The patch to python-2.6.4.p10/src/setup.py is applied on all hosts, not just Debian and derivatives, but the new code will end up doing nothing on non-Debian-derived hosts. This is how the build is set up in Python 2.7.

Just to make it a bit clearer: The code also doesn't change the search paths on older Debian / Ubuntu systems, regardless if dpkg-architecture is installed or not, so won't break anything there.

(Tested with Ubuntu 9.04 and 10.04; dpkg-architecture there doesn't know DEB_HOST_MULTIARCH and therefore exits with a non-zero status such that nothing gets added to the paths.)


Note that on Debian and derivatives, if the command dpkg-architecture, from the package dpkg-dev, is not available, the build will fail as before.

Again, of course the build will only fail on newer Debian derivatives which require a change (more precisely: additions) to the search paths.


Could someone please update the p11 / provide a new p11 spkg (by applying only 11447-Python-Debian-multiarch.patch to p10 I guess)?

http://users.aims.ac.za/~jan/python-2.6.4.p11.spkg (and also the ticket description) is not current.

comment:23 Changed 10 years ago by leif

P.S.:

I think if the test importing the crypt module (in spkg-install) fails and dpkg-architecture isn't installed, the user should get a hint on installing it (i.e., dpkg-dev; maybe only on newer Debian derivatives, but this could also just be part of the message.)

Otherwise we should at least document this in the Sage Installation Guide, and perhaps also on one of the wiki pages.

comment:24 in reply to: ↑ 22 Changed 10 years ago by leif

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues Provide an updated spkg deleted

Replying to leif:

Could someone please update the p11 / provide a new p11 spkg (by applying only 11447-Python-Debian-multiarch.patch to p10 I guess)?

http://users.aims.ac.za/~jan/python-2.6.4.p11.spkg (and also the ticket description) is not current.

Ok, did it myself:

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

md5sum: 5bc63c1814fc7ae34f4fed5a8fffe380 python-2.6.4.p11.spkg

(I haven't made any changes, just applied Jan Medlock's patch of June 17th.)

comment:25 in reply to: ↑ 22 Changed 10 years ago by medlock

Replying to leif:

Replying to medlock:

The patch to python-2.6.4.p10/src/setup.py is applied on all hosts, not just Debian and derivatives, but the new code will end up doing nothing on non-Debian-derived hosts. This is how the build is set up in Python 2.7.

Just to make it a bit clearer: The code also doesn't change the search paths on older Debian / Ubuntu systems, regardless if dpkg-architecture is installed or not, so won't break anything there.

(Tested with Ubuntu 9.04 and 10.04; dpkg-architecture there doesn't know DEB_HOST_MULTIARCH and therefore exits with a non-zero status such that nothing gets added to the paths.)


Note that on Debian and derivatives, if the command dpkg-architecture, from the package dpkg-dev, is not available, the build will fail as before.

Again, of course the build will only fail on newer Debian derivatives which require a change (more precisely: additions) to the search paths.


Could someone please update the p11 / provide a new p11 spkg (by applying only 11447-Python-Debian-multiarch.patch to p10 I guess)?

http://users.aims.ac.za/~jan/python-2.6.4.p11.spkg (and also the ticket description) is not current.

Thanks for the clarification. I can confirm for the record that this is the intended behavior.

comment:26 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from python spkg build fails on Ubuntu 11.04 derivative Mint 11 to python spkg build fails on various Debian systems

comment:27 follow-up: Changed 10 years ago by jdemeyer

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

comment:28 Changed 10 years ago by jdemeyer

  • Component changed from build to packages
  • Keywords python spkg added
  • Priority changed from major to blocker

comment:29 in reply to: ↑ 27 Changed 10 years ago by leif

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.

comment:30 follow-up: Changed 10 years ago by pipedream

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

comment:31 Changed 10 years ago by pipedream

  • Owner changed from pipedream to jdemeyer

comment:32 Changed 10 years ago by leif

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.

comment:33 in reply to: ↑ 30 Changed 10 years ago by leif

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.

Changed 10 years ago by leif

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

comment:34 follow-up: Changed 10 years ago by leif

Patch is up. Relevant part:

  • spkg-install

    diff --git a/spkg-install b/spkg-install
    a b  
    242248# All these modules are important and if any one
    243249# fails to build, Sage will not work.
    244250
     251import_errors=false
    245252for module in math hashlib crypt ; do
    246253   "$SAGE_LOCAL/bin/python" -c "import $module"
    247254   if [ $? -eq 0 ] ; then
    248255      echo "$module module imported OK"
    249256   else
    250       echo "$module module failed to import"
    251       exit 1
     257      echo >&2 "$module module failed to import"
     258      import_errors=true
     259      # exit 1 # not yet
    252260   fi
    253261done
     262
     263if $import_errors; then
     264    echo >&2 "Error: One or more modules failed to import."
     265    # Check if we are on Debian or one of its derivatives:
     266    if command -v dpkg && ! command -v dpkg-architecture >/dev/null; then
     267        echo >&2 "You may have to install 'dpkg-architecture'"
     268        echo >&2 "which is part of the Debian package 'dpkg-dev'."
     269        echo >&2 "Try installing it by typing"
     270        echo >&2 "    sudo apt-get install dpkg-dev"
     271        echo >&2 "and rerun 'make'."
     272    fi
     273    exit 1
     274fi

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

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

comment:35 Changed 10 years ago by leif

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

comment:36 Changed 10 years ago by leif

  • Description modified (diff)

comment:37 in reply to: ↑ 34 ; follow-up: Changed 10 years ago by leif

Replying to leif:

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

Ok, will do now...

comment:38 in reply to: ↑ 37 Changed 10 years ago by leif

  • Description modified (diff)

Replying to leif:

Replying to leif:

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

Ok, will do now...

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

comment:39 follow-up: Changed 10 years ago by jdemeyer

  • Status changed from needs_review to positive_review

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

comment:40 in reply to: ↑ 39 ; follow-up: Changed 10 years ago by leif

Replying to jdemeyer:

Messy in various places,

Details, please. ;-)

comment:41 in reply to: ↑ 40 ; follow-up: Changed 10 years ago by jdemeyer

Replying to leif:

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?

comment:42 in reply to: ↑ 41 Changed 10 years ago by leif

Replying to jdemeyer:

Replying to leif:

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.

comment:43 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.1.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.