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:  sage8.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: 
Description (last modified by )
Change History (29)
comment:1 Changed 4 years ago by
 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
 Branch set to u/Konrad127123/add_missing_dependencies_for_building_with_sage_check_yes
comment:3 Changed 4 years ago by
 Commit set to e00134bb3223e52887345c13618de2be665d8e03
 Status changed from new to needs_review
New commits:
e00134b  Fix dependencies for cypari, pycrypto and sagetex, so that they build successfully when SAGE_CHECK=yes.

comment:4 Changed 4 years ago by
 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 followup: ↓ 9 Changed 4 years ago by
 Status changed from needs_review to needs_work
pycrypto
works for me without your patch.
comment:6 Changed 4 years ago by
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
scandir
is actually a dependency of pathlib2
, I fixed that in #24996.
comment:8 Changed 4 years ago by
 Dependencies set to #24996
comment:9 in reply to: ↑ 5 ; followup: ↓ 10 Changed 4 years ago by
Replying to jdemeyer:
pycrypto
works for me without your patch.
Do you have system mpir or gmp installed? If I install the libgmpdev 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/sage8.1/local/lib/python2.7/sitepackages/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/sage8.1/local/lib/python2.7/distutils/core.py", line 151, in setup dist.run_commands() File "/home/sage/build_experiments/sage8.1/local/lib/python2.7/distutils/dist.py", line 953, in run_commands self.run_command(cmd) File "/home/sage/build_experiments/sage8.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/sage8.1/local/lib/python2.7/sitepackages/Crypto/SelfTest/__init__.py", line 74, in run raise SelfTestError("Selftest failed", result) Crypto.SelfTest.SelfTestError: ('Selftest 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 libgmpdev is not installed.
comment:10 in reply to: ↑ 9 Changed 4 years ago by
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 orderonly dependency (on the right of the 
symbol).
comment:11 Changed 4 years ago by
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
 Commit changed from e00134bb3223e52887345c13618de2be665d8e03 to 88d2d366fb62328116bc7890cb3e8f9a11da9b68
Branch pushed to git repo; I updated commit sha1. New commits:
88d2d36  Fix cypari doctest and pycrypto dependencies.

comment:13 followup: ↓ 14 Changed 4 years ago by
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
 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
 Commit changed from 88d2d366fb62328116bc7890cb3e8f9a11da9b68 to 2031aef0f55b8e23700693daf064e6bb5a0f13f8
Branch pushed to git repo; I updated commit sha1. New commits:
2031aef  Remove unneeded dependency.

comment:16 Changed 4 years ago by
 Status changed from needs_work to needs_review
Ready to review again, I think.
comment:17 followup: ↓ 18 Changed 4 years ago by
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
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
 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
 Description modified (diff)
 Keywords sagetex matplotlib added; cypari pycrypto removed
 Milestone changed from sage8.2 to sage8.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
 Commit changed from 2031aef0f55b8e23700693daf064e6bb5a0f13f8 to 93a97e9a65e4552568be2f8449c2f8cba6926677
comment:22 Changed 4 years ago by
Another SageTeX issue: #26343.
comment:23 followup: ↓ 26 Changed 4 years ago by
 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
 Status changed from positive_review to needs_review
comment:25 Changed 4 years ago by
Sorry, should have set to needs_review above
comment:26 in reply to: ↑ 23 Changed 4 years ago by
 Milestone changed from sage8.4 to sage8.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.
comment:27 Changed 4 years ago by
Patch still works for 8.5beta1 and is still the minimum set of dependencies needed.
comment:28 Changed 4 years ago by
 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
 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
This patch adds the minimal list of missing packages needed to fix things. (Tested with make distclean && make package_name )