Opened 3 years ago

Last modified 2 years ago

#25391 closed defect

SageMath fails to build on on Fedora 28 with gcc 8.1 — at Version 17

Reported by: Vaush Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: gcc8.1 python3 fedora28 compilation build
Cc: dimpase Merged in:
Authors: Dario Asprone Reviewers: Dima Pasechnik
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: u/Vaush/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1 (Commits) Commit: 35c3c33f42df5da1b00351205c1d984c06019eba
Dependencies: #25204,#25353 Stopgaps:

Description (last modified by dimpase)

SageMath 8.3.beta1 building produced 3 different errors when run on Fedora 28 64bit with gcc 8.1, specifically when building the packages:

  • python 3.6.1
  • python 2.7.14 (fixed in #25204)
  • linbox 1.5.2 (fixed in #25353)

The 1st error manifests itself as a failure to import the crypt module during the build of Python 3. It is due to an unspecified issue with Fedora 28 implementation of crypt(), which as reported in https://fedoraproject.org/wiki/Changes/Replace_glibc_libcrypt_with_libxcrypt is not the standard glibc implementation. Since I couldn't find a way to fix the crypt implementation in a reliable manner, I opted to implement the patch reported here https://bugs.python.org/issue28503 to allow python to use r_crypt(), which works.

For specifics on how the latter two errors are solved, see the sage-devel discussion at https://groups.google.com/forum/#!msg/sage-devel/NgzlZknrizg/o-_Exw8jCAAJ

Change History (21)

Changed 3 years ago by Vaush

Linbox patch

Changed 3 years ago by Vaush

Python2 patch

Changed 3 years ago by Vaush

Python3 patch - 1 of 2

Changed 3 years ago by Vaush

Python3 patch - 2 of 2

comment:1 Changed 3 years ago by Vaush

  • Dependencies set to #25204,#25353

comment:2 Changed 3 years ago by Vaush

I added dependencies to #25204,#25353 which respectively fix the python 2 issue and the linbox (and fflas, which is not described here) issue. The commit to be made will be based on them, and will only modify the python 3 package

comment:3 Changed 3 years ago by Vaush

  • Branch set to u/Vaush/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:4 Changed 3 years ago by Vaush

  • Commit set to 473f2149a035a87b0941ecab426f3b32bcd20472
  • Status changed from new to needs_review

New commits:

4c1223eFixed python 3.6.1 crypt issue on Fedora 28
cb566eafixing gcc-8.1 compilation errors.
b20fd48Merge remote-tracking branch 'trac/u/cpernet/fflas_and_linbox_broken_with_gcc_8_1_0' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1
d14acaeUpgrade to Python 2.7.15
473f214Merge remote-tracking branch 'trac/u/jdemeyer/upgrade_to_python_2_7_15' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:5 Changed 3 years ago by Vaush

  • Cc dimpase added

comment:6 Changed 3 years ago by dimpase

I'll be checking that this does not break on other systems.

comment:7 Changed 3 years ago by dimpase

Have you run tests on your branch?: make ptest (or make ptestlong for more tests)

This is something the ticket's author should do in such cases. (It would not be surprising if something unrelated to your changes fails, as gcc 8 is pretty much untested, but good to know anyway).

comment:8 Changed 3 years ago by Vaush

I haven't, but will do

comment:9 Changed 3 years ago by git

  • Commit changed from 473f2149a035a87b0941ecab426f3b32bcd20472 to 5be7adc8c90d54d53c77fa8a20a8df1a223ea577

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

5be7adcMerge branch 'master' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:10 Changed 3 years ago by dimpase

What is happening with this on the current 8.3.beta6? Note that python2 has been updated.

comment:11 Changed 3 years ago by dimpase

For Python3, it's apparently https://github.com/python/cpython/pull/4691 which is still under review.

comment:12 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_info

I am missing some context and documentation. The current branch only fixes Python 3. That is not consistent with the ticket description which mentions several packages and doesn't even mention the actual failure that the Python 3 patch is trying to fix.

comment:13 Changed 3 years ago by jdemeyer

The patch files should also contain some description, at a minimum a link to the upstream issue.

And if you add a patch to a package, you should bump the package version.

comment:14 Changed 3 years ago by Vaush

Ok, I'm sorry if the description or the execution aren't up to par, this is my first ticket, so I'm getting used to it.
The ticket was originally opened to describe the bug. After that, I succeeded in manually fixing all the bugs listed in the description, but it was brought to my attention that two of my bugs were solved in separate tickets at the same time (see the dependencies field), so I merged those tickets and added my own fix for python3, thus fixing all the issues and solving the problem stated in the ticket title, that is SageMath not compiling on Fedora 28 with gcc 8.1.
Since the commits include the fixes for the other 2 bugs, even if by merging, I thought this counted as having a fix for all 3 of them in the branch, is this wrong?
There is a description for the python 3 bug, since the bug is literally "Python tries to call crypt instead of crypt_r on Fedora, and this for some reason breaks things, so let's allow it to call crypt_r", and the reason why it breaks things is, as pointed out by dimpase, apparently that fedora has a completely different crypt implementation, as can be seen here https://fedoraproject.org/wiki/Changes/Replace_glibc_libcrypt_with_libxcrypt.
I'll expand on the description a bit, anyway, to make this clearer, and also to reflect the fact that this ticket only directly fixes the python3 bug, since it's only reported in the comments. I'll also quickly add a description and a link to the patch

comment:15 Changed 3 years ago by Vaush

  • Description modified (diff)

comment:16 Changed 3 years ago by git

  • Commit changed from 5be7adc8c90d54d53c77fa8a20a8df1a223ea577 to 35c3c33f42df5da1b00351205c1d984c06019eba

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

35c3c33Added comments to the patches, and modified the package version

comment:17 Changed 3 years ago by dimpase

  • Description modified (diff)
  • Keywords python2 linbox removed
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_info to needs_review

Jeroen, how do you feel about merging Python 3 patch which is still being out for reviewing by Python people?

Note: See TracTickets for help on using tickets.