Opened 4 years ago

Closed 4 years ago

#24798 closed defect (fixed)

Missing dependencies cause sagetex to fail to build when SAGE_CHECK=yes

Reported by: Konrad127123 Owned by:
Priority: minor Milestone: sage-8.5
Component: packages: standard Keywords: sagetex, matplotlib, dependencies
Cc: slelievre Merged in:
Authors: Konrad K. Dabrowski Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 93a97e9 (Commits, GitHub, GitLab) Commit: 93a97e9a65e4552568be2f8449c2f8cba6926677
Dependencies: #24995 Stopgaps:

Status badges

Description (last modified by Konrad127123)

sagetex can fail to build if SAGE_CHECK=yes due to missing dependencies

To reproduce: make distclean && make sagetex

(An earlier version of this bug report dealt with issues building cypari and pycrypto if SAGE_CHECK=yes. These were fixed in #25813 and #25844, respectively.)

Change History (29)

comment:1 Changed 4 years ago by Konrad127123

  • Authors set to Konrad K. Dabrowski
  • Component changed from PLEASE CHANGE to packages: standard
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 4 years ago by Konrad127123

  • Branch set to u/Konrad127123/add_missing_dependencies_for_building_with_sage_check_yes

This patch adds the minimal list of missing packages needed to fix things. (Tested with make distclean && make package_name )

Last edited 4 years ago by Konrad127123 (previous) (diff)

comment:3 Changed 4 years ago by Konrad127123

  • Commit set to e00134bb3223e52887345c13618de2be665d8e03
  • Status changed from new to needs_review

New commits:

e00134bFix dependencies for cypari, pycrypto and sagetex, so that they build successfully when SAGE_CHECK=yes.

comment:4 Changed 4 years ago by Konrad127123

  • Summary changed from Add missing dependencies for building with SAGE_CHECK=yes to Missing dependencies for cypari, pycrypto and sagetex when SAGE_CHECK=yes

comment:5 follow-up: Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

pycrypto works for me without your patch.

comment:6 Changed 4 years ago by jdemeyer

And I consider the dependency of cypari on six an upstream bug (it doesn't really need six) which is fixed now.

comment:7 Changed 4 years ago by jdemeyer

scandir is actually a dependency of pathlib2, I fixed that in #24996.

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:8 Changed 4 years ago by jdemeyer

  • Dependencies set to #24996

comment:9 in reply to: ↑ 5 ; follow-up: Changed 4 years ago by Konrad127123

Replying to jdemeyer:

pycrypto works for me without your patch.

Do you have system mpir or gmp installed? If I install the libgmp-dev package (on Debian Unstable)

make distclean  && make pycrypto

passes. If I don't have it installed then testing fails with:

ERROR: test_negative_number_roundtrip_mpzToLongObj_longObjToMPZ (Crypto.SelfTest.Util.test_number.MiscTests)
Test that mpzToLongObj and longObjToMPZ (internal functions) roundtrip negative numbers correctly.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sage/build_experiments/sage-8.1/local/lib/python2.7/site-packages/Crypto/SelfTest/Util/test_number.py", line 283, in test_negative_number_round
trip_mpzToLongObj_longObjToMPZ
    k = number._fastmath.rsa_construct(n, e)
AttributeError: 'NoneType' object has no attribute 'rsa_construct'

----------------------------------------------------------------------
Ran 1033 tests in 48.713s

FAILED (errors=1)
Traceback (most recent call last):
  File "setup.py", line 456, in <module>
    core.setup(**kw)
  File "/home/sage/build_experiments/sage-8.1/local/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/home/sage/build_experiments/sage-8.1/local/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/home/sage/build_experiments/sage-8.1/local/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "setup.py", line 336, in run
    SelfTest.run(module=moduleObj, verbosity=self.verbose, stream=sys.stdout, config=self.config)
  File "/home/sage/build_experiments/sage-8.1/local/lib/python2.7/site-packages/Crypto/SelfTest/__init__.py", line 74, in run
    raise SelfTestError("Self-test failed", result)
Crypto.SelfTest.SelfTestError: ('Self-test failed', <unittest.runner.TextTestResult run=1033 errors=1 failures=0>)
Error: PyCrypto failed to pass its test suite.

With my patch, testing succeeds for me even if system libgmp-dev is not installed.

comment:10 in reply to: ↑ 9 Changed 4 years ago by jdemeyer

Replying to Konrad127123:

Replying to jdemeyer:

pycrypto works for me without your patch.

Do you have system mpir or gmp installed?

You are right. pycrypto does require GMP/MPIR. But since it links against it, it should be a real dependency (on the left of the | symbol) instead of an order-only dependency (on the right of the | symbol).

comment:11 Changed 4 years ago by jdemeyer

I would prefer to revert the change to the dependencies of cypari. It will be fixed in the next release.

comment:12 Changed 4 years ago by git

  • Commit changed from e00134bb3223e52887345c13618de2be665d8e03 to 88d2d366fb62328116bc7890cb3e8f9a11da9b68

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

88d2d36Fix cypari doctest and pycrypto dependencies.

comment:13 follow-up: Changed 4 years ago by Konrad127123

How's the above for a solution for cypari and pycrypto?

In the latest beta sagetex (even with my patch) is failing with

************************************************************************
It seems that you are attempting to run Sage from an unpacked source
tarball, but you have not compiled it yet (or maybe the build has not
finished). You should run `make` in the Sage root directory first.
If you did not intend to build Sage from source, you should download
a binary tarball instead. Read README.txt for more information.
************************************************************************

I don't think that happened with 8.2.beta.6 and I haven't tried to find the cause yet. (As an aside, neither scandir nor pathlib2 get pulled in as dependencies when doing make distclean && make sagetex.)

comment:14 in reply to: ↑ 13 Changed 4 years ago by jdemeyer

  • Dependencies changed from #24996 to #24995

Replying to Konrad127123:

How's the above for a solution for cypari and pycrypto?

In the latest beta sagetex (even with my patch) is failing with

************************************************************************
It seems that you are attempting to run Sage from an unpacked source
tarball, but you have not compiled it yet (or maybe the build has not
finished). You should run `make` in the Sage root directory first.
If you did not intend to build Sage from source, you should download
a binary tarball instead. Read README.txt for more information.
************************************************************************

Yes, that is #24995.

comment:15 Changed 4 years ago by git

  • Commit changed from 88d2d366fb62328116bc7890cb3e8f9a11da9b68 to 2031aef0f55b8e23700693daf064e6bb5a0f13f8

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

2031aefRemove unneeded dependency.

comment:16 Changed 4 years ago by Konrad127123

  • Status changed from needs_work to needs_review

Ready to review again, I think.

comment:17 follow-up: Changed 4 years ago by jdemeyer

Looks good to me on first sight. I still need to test it though.

comment:18 in reply to: ↑ 17 Changed 4 years ago by Konrad127123

Have you had a chance to test it yet? Since it's a build issue, it would be nice to get this into 8.2.

comment:19 Changed 4 years ago by slelievre

  • Cc slelievre added
  • Keywords cypari pycrypto dependencies added

We might no longer need pycrypto, see #25844 (Remove package pycrypto).

comment:20 Changed 4 years ago by Konrad127123

  • Description modified (diff)
  • Keywords sagetex matplotlib added; cypari pycrypto removed
  • Milestone changed from sage-8.2 to sage-8.4
  • Status changed from needs_review to needs_work
  • Summary changed from Missing dependencies for cypari, pycrypto and sagetex when SAGE_CHECK=yes to Missing dependencies cause sagetex to fail to build when SAGE_CHECK=yes

comment:21 Changed 4 years ago by git

  • Commit changed from 2031aef0f55b8e23700693daf064e6bb5a0f13f8 to 93a97e9a65e4552568be2f8449c2f8cba6926677

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a1bf82ematplotlib should have a runtime dependency on kiwisolver
93a97e9Add missing dependencies to make sagtex build when SAGE_CHECK=yes

comment:22 Changed 4 years ago by jhpalmieri

Another SageTeX issue: #26343.

comment:23 follow-up: Changed 4 years ago by Konrad127123

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

Dependencies updated for 8.4rc0:

Doing make distclean && make sagetex fails unless kiwisolver, sympy, elliptic_curves and jmol are installed.

kiwisolver should really get pulled in by matplotlib, not sagetex, so I've added it to matplotlib's runtime dependencies (this got missed in #25702) and added to the rest to sagetex.

I tested with 8.4rc0 and confirmed that sagetex now builds with SAGE_CHECK=yes. I've also confirmed that adding any combination of three of the dependencies is not sufficient.

comment:24 Changed 4 years ago by Konrad127123

  • Status changed from positive_review to needs_review

comment:25 Changed 4 years ago by Konrad127123

Sorry, should have set to needs_review above

comment:26 in reply to: ↑ 23 Changed 4 years ago by Konrad127123

  • Milestone changed from sage-8.4 to sage-8.5

I tested with 8.4rc0 and confirmed that sagetex now builds with SAGE_CHECK=yes. I've also confirmed that adding any combination of three of the dependencies is not sufficient.

I have confirmed that this is still the case with 8.5beta0.

Last edited 4 years ago by Konrad127123 (previous) (diff)

comment:27 Changed 4 years ago by Konrad127123

Patch still works for 8.5beta1 and is still the minimum set of dependencies needed.

comment:28 Changed 4 years ago by jhpalmieri

  • Priority changed from major to minor
  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Looks okay to me.

comment:29 Changed 4 years ago by vbraun

  • Branch changed from u/Konrad127123/add_missing_dependencies_for_building_with_sage_check_yes to 93a97e9a65e4552568be2f8449c2f8cba6926677
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.