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

Sort out linking with libstdc++ #29856

Closed
dimpase opened this issue Jun 14, 2020 · 25 comments
Closed

Sort out linking with libstdc++ #29856

dimpase opened this issue Jun 14, 2020 · 25 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jun 14, 2020

From the post at

it appears linking with libstdc++ might be not needed any more, and so this may be simplified.

Note that clang project has libc++, which serves the same role, but is a different implementation.

(Currently stdc++ is mentioned in src/setup.py
and in src/sage/misc/cython.py.)

Depends on #29702

CC: @antonio-rojas @mkoeppe @kiwifb @isuruf

Component: build

Author: Dima Pasechnik

Branch/Commit: afeec95

Reviewer: Matthias Koeppe

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

@dimpase dimpase added this to the sage-9.2 milestone Jun 14, 2020
@dimpase
Copy link
Member Author

dimpase commented Jun 14, 2020

comment:1

trying to see what happens with the simple

index af5f99d03e..0600603e62 100644
--- a/src/sage/misc/cython.py
+++ b/src/sage/misc/cython.py
@@ -41,7 +41,7 @@ cblas_library_dirs = list(cblas_pc['library_dirs'])
 cblas_include_dirs = list(cblas_pc['include_dirs'])
 
 standard_libs = [
-    'mpfr', 'gmp', 'gmpxx', 'stdc++', 'pari', 'm',
+    'mpfr', 'gmp', 'gmpxx', 'pari', 'm',
     'ec', 'gsl',
 ] + cblas_libs + [
     'ntl']
--- a/src/setup.py
+++ b/src/setup.py
@@ -354,8 +354,6 @@ class sage_build_cython(Command):
 
         - Add dependencies on header files for certain libraries
 
-        - Ensure that C++ extensions link with -lstdc++
-
         - Sort the libraries according to the library order
 
         - Add some default compile/link args and directories
@@ -369,10 +367,7 @@ class sage_build_cython(Command):
         lang = kwds.get('language', 'c')
         cplusplus = (lang == "c++")
 
-        # Libraries: add stdc++ if needed and sort them
         libs = kwds.get('libraries', [])
-        if cplusplus:
-            libs = libs + ['stdc++']
         kwds['libraries'] = sorted(set(libs),
                 key=lambda lib: library_order.get(lib, 0))

@dimpase

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 14, 2020

comment:3

This ticket description needs some work; what problem is this ticket addressing?

Also, setup.py does not use sage.misc.cython at all if I'm not mistaken.

And please note #29702 moves the code from src/setup.py to sage_setup - best to do changes on top of that

@dimpase

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Commit: 07010d4

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Dependencies: #29702

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Branch: u/dimpase/build/get_rid_of_stdcxx

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

comment:6

seems to work.


Last 10 new commits:

5790687build/bin/write-dockerfile.sh: Do not ADD removed file src/Makefile.in
851ab76Make sagelib a script package
f39a017Move src/fpickle_setup.py to srs/sage_setup/
363f792src/setup.py: Move class sage_install to new module sage_setup.command.sage_install
a7cde9csrc/module_list.py: Move library_order to new module sage_setup.library_order
7c4bbf6src/setup.py: Move classes sage_build_* to new modules sage_setup.command.sage_*
ddc49aasrc/setup.py: Move excepthook to new module sage_setup.excepthook
9cff0c6src/sage_setup/command/__init__.py: New
56705f8src/sage_setup/command/sage_build_cython.py: Fix adding setup.py as dependency
07010d4remove stdc++ from everywhere

@dimpase dimpase changed the title sort out linking with libstd++ sort out linking with libstdc++ Jun 15, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 15, 2020

comment:7

tested on GH Actions?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 15, 2020

comment:8

Please rebase on unrebased-then-merged #29702

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Changed commit from 07010d4 to afeec95

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

Last 10 new commits:

e810ad1Trac #29345: don't use sage's config.status for the lrcalc build.
93c9921Trac #29345: replace the function that populates the CVXOPT_* variables.
0e66a0aTrac #29345: add Dima's SPKG patches for ksh compatibility.
df3f05ebuild/make/Makefile.in [SCRIPT_PACKAGE_templ]: cd into the SPKG directory; adjust spkg-install scripts
5372065Merge branch 't/29793/script_packages_should_cd_into_the_spkg_directory' into t/29411/make_sagelib_a_script_package
c166b97Merge branch 't/29411/make_sagelib_a_script_package' into t/29702/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup
cc30471build/bin/write-dockerfile.sh: Do not ADD removed file src/Makefile.in
8a41326Merge branch 't/29411/make_sagelib_a_script_package' into t/29702/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup
a56dc35Merge tag '9.2.beta1' into t/29702/public/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup
afeec95explictly linking with libstdc++ not needed

@dimpase
Copy link
Member Author

dimpase commented Jun 15, 2020

comment:10

testing on https://github.com/dimpase/sage/pull/10 (also, this will test #29702)

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 15, 2020

comment:11

Great

@dimpase
Copy link
Member Author

dimpase commented Jun 16, 2020

comment:12

Replying to @dimpase:

testing on https://github.com/dimpase/sage/pull/10 (also, this will test #29702)

I see a lot of assembler errors (GNU 'as' version 2.24, ~6 years old)
in building ecm, and elsewhere, on minimal ubuntu-trusty

2020-06-16T00:41:24.9539407Z   [ecm-7.0.4.p1]   /tmp/ccEhz4KE.s:342: Error: operand size mismatch for `vmovdqu64'

see https://github.com/dimpase/sage/runs/774196885

Not a surprise, as we build a newish gcc, not really compatible with an old as, running on hardware not 100% supported by this as. Should we just drop this target all together?

@dimpase
Copy link
Member Author

dimpase commented Jun 16, 2020

comment:13

On ubunty-bionic standard I see

2020-06-16T01:19:21.1467385Z   [dochtml]   ImportError: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.26' not found (required by /sage/local/lib/python3.7/site-packages/sage/matrix/matrix_integer_sparse.cpython-37m-x86_64-linux-gnu.so)

which looks like a broken toolchain (`GLIBCXX_3.4.26' not found) - any idea why?

see https://github.com/dimpase/sage/runs/774196977

The rest looks OK to me.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 16, 2020

comment:14

The above two, are these from -gcc_spkg builds? Please add links to the runs

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 16, 2020

comment:15

(See #29675 for known failures of -gcc_spkg builds with "GLIBCXX not found" errors.)

@dimpase
Copy link
Member Author

dimpase commented Jun 16, 2020

comment:16

Replying to @mkoeppe:

The above two, are these from -gcc_spkg builds? Please add links to the runs

done

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

Reviewer: Matthias Koeppe

@slel

This comment has been minimized.

@slel slel changed the title sort out linking with libstdc++ Sort out linking with libstdc++ Jun 18, 2020
@vbraun
Copy link
Member

vbraun commented Jul 2, 2020

Changed branch from u/dimpase/dont_link_to_stdcxx to afeec95

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