search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Cygwin: Fix cliquer, giac, meataxe, rw, libbraiding to build shared libraries, using `AM_LDFLAGS=-no-undefined` #29152

Closed embray closed 3 years ago

embray commented 4 years ago

Follow-up from #30396, where libtool's fallback to static library build for giac caused linker errors while building sagelib.

This is fixed by passing -no-undefined to the libtool linking flags, by setting AM_LDFLAGS=-no-undefined.

Affected packages can be found as follows:

$ grep "undefined symbols not allowed" logs/pkgs/*
cliquer-1.21.p4.log:libtool: link: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries
giac-1.5.0.87p0.log:libtool: warning: undefined symbols not allowed in x86_64-pc-cygwin shared libraries; building static only
libbraiding-1.0.p0.log:libtool: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries; building static only
rw-0.7.p0.log:libtool: link: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries
singular-4.1.1p2.p0.log:libtool: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries; building static only

In singular, only the gitfan.la library (module) is affected:

Making all in gitfan
...
/usr/bin/bash ../../../libtool  --tag=CXX   --mode=link g++ -std=gnu++11  -O2 -g  -pipe -fno-common -g0 -O3 -Wno-unused-function -Wno-trigraphs -Wno-unused-parameter -Wunknown-pragmas -Wno-unused-variable -fomit-frame-pointer -fwrapv -fvisibility=default -finline-functions -fno-exceptions -fno-threadsafe-statics -fno-enforce-eh-specs -fconserve-space -funroll-loops -fno-delete-null-pointer-checks -fno-rtti -module -export-dynamic -avoid-version -flat_namespace -weak_reference_mismatches weak -undefined dynamic_lookup -L/cygdrive/d/a/sage/sage/local/lib -Wl,-rpath,/cygdrive/d/a/sage/sage/local/lib  -pipe -fno-common -g0 -O3 -Wno-unused-function -Wno-trigraphs -Wno-unused-parameter -Wunknown-pragmas -Wno-unused-variable -fomit-frame-pointer -fwrapv -fvisibility=default -finline-functions -fno-exceptions -fconserve-space -funroll-loops -fno-delete-null-pointer-checks  -Wl,-Bdynamic -o gitfan.la -rpath /cygdrive/d/a/sage/sage/local/libexec/singular/MOD gitfan_la-gitfan.lo  -lreadline -lncurses -lmpfr -lrt
libtool: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries; building static only
libtool: link: ar cru .libs/gitfan.a  gitfan_la-gitfan.o
libtool: link: ranlib .libs/gitfan.a
libtool: link: ( cd ".libs" && rm -f "gitfan.la" && ln -s "../gitfan.la" "gitfan.la" )

The issue also affects the optional package meataxe.

[meataxe-1.0.p0] /usr/bin/bash ../libtool  --tag=CC   --mode=link gcc  -g -O2  -L/opt/sagemath-9.0/local/lib -Wl,-rpath,/opt/sagemath-9.0/local/lib  -o libmtx.la -rpath /opt/sagemath-9.0/local/lib args.lo berlekmp.lo bsand.lo bscore.lo bsdup.lo bsissub.lo bsmatch.lo bsminus.lo bsop.lo bsor.lo bsprint.lo bsread.lo bswrite.lo cfinfo.lo charpol.lo chbasis.lo error.lo ffio.lo fpcore.lo fpdup.lo fpmul.lo fpmul2.lo fpprint.lo gcd.lo genseed.lo grmaprow.lo grmatcore.lo grtable.lo homcomp.lo imatcore.lo imatread.lo imatwrite.lo init.lo intio.lo issub.lo isisom.lo kernel-0.lo ldiag.lo maddmul.lo mat2vec.lo matadd.lo matclean.lo matcmp.lo maketabF.lo matcopy.lo matcore.lo matcut.lo matdup.lo matech.lo matid.lo matins.lo matinv.lo matmul.lo matnull.lo matorder.lo matpivot.lo matprint.lo matpwr.lo matread.lo mattr.lo mattrace.lo matwrite.lo message.lo mfcore.lo mfread.lo mfreadlong.lo mfwrite.lo mfwritelong.lo minpol.lo mkendo.lo mmulscal.lo mraddgen.lo mrcore.lo mrread.lo mrtranspose.lo mrwrite.lo msclean.lo mscore.lo mtensor.lo mtxobj.lo os.lo permcmp.lo permcore.lo permdup.lo perminv.lo permmul.lo permorder.lo permprint.lo permpwr.lo permread.lo permwrite.lo poladd.lo polcmp.lo polcore.lo polderive.lo poldiv.lo poldup.lo polgcd.lo polmul.lo polprint.lo polread.lo polwrite.lo quotient.lo random.lo rdcfgen.lo saction.lo setcore.lo setinsert.lo settest.lo spinup.lo spinup2.lo split.lo stabpwr.lo stfcore.lo stfread.lo stfwrite.lo string.lo sumint.lo temap.lo tkinfo.lo vec2mat.lo wgen.lo window.lo zcleanrow.lo zcmprow.lo zgap.lo zpermrow.lo zzz2.lo
[meataxe-1.0.p0] libtool:   error: can't build x86_64-unknown-cygwin shared library unless -no-undefined is specified

This is a common issue especially when using libtool to link Windows DLLs and should hopefully be straightforward to fix.

See also:

Depends on #31064

Upstream: Reported upstream. No feedback yet.

CC: @simon-king-jena @miguelmarco @dimpase @orlitzky @antonio-rojas @kiwifb @slel @jhpalmieri @vbraun

Component: porting: Cygwin

Author: Matthias Koeppe, Miguel Marco, Simon King

Branch/Commit: 6b6ee5c

Reviewer: Matthias Koeppe, Dima Pasechnik

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

simon-king-jena commented 4 years ago
comment:40

SharedMeatAxe is upgraded as well. See https://github.com/simon-king-jena/SharedMeatAxe/releases/tag/v1.0.1

I changed the compression from gz to bz2, since the latter gives a smaller tar ball. Self tests pass, I'm running "make test" now.

New commits:

54c2584Upgrade SharedMeatAxe, to fix a build problem on Cygwin
simon-king-jena commented 4 years ago

Changed author from Matthias Koeppe, Miguel Marco to Matthias Koeppe, Miguel Marco, Simon King

simon-king-jena commented 4 years ago
comment:41

Tests pass for me. However, the point of this ticket is to fix things on cygwin, which I'm not using.

Hence, no positive review.

A question: Is it still the case that the issues are "Reported upstream. No feedback yet.", or can this be changed to "Fixed upstream"?

mkoeppe commented 4 years ago
comment:42

Thanks very much, Simon. Could you add an upstream_url field to checksums.ini please (as explained in https://wiki.sagemath.org/ReleaseTours/sage-9.1#For_developers-1)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

cd74ef3Add upstream url for SharedMeatAxe
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 54c2584 to cd74ef3

simon-king-jena commented 4 years ago
comment:44

Hope that upstream_url for SharedMeatAxe is fine.

mkoeppe commented 4 years ago

Reviewer: https://github.com/mkoeppe/sage/actions/runs/285244461

simon-king-jena commented 4 years ago
comment:46

Replying to @mkoeppe:

You added a URL as reviewer. Is that intended?

Actually it is not clear to me what is stated there. Do I understand correctly that tests on different versions of Cygwin pass, except "cygwin-stage-ii-a (standard)"? And the failed package in this Cygwin version is scipy?

mkoeppe commented 4 years ago
comment:47

Replying to @simon-king-jena:

Replying to @mkoeppe:

You added a URL as reviewer. Is that intended?

Yes, it's a trick that I have been using recently. The URL shows in ticket lists such as https://trac.sagemath.org/report/92?asc=1&page=1 and this reminds me (or other developers) to inspect these automatic builds when they are done.

They will be replaced by actual reviewer names when ready.

Actually it is not clear to me what is stated there. Do I understand correctly that tests on different versions of Cygwin pass, except "cygwin-stage-ii-a (standard)"? And the failed package in this Cygwin version is scipy?

The build on Cygwin takes very long because of various overheads compared to a build on Linux. On GH Actions there is a time limit of 6 hours per job. Therefore I have split the build into several jobs ("stages"), some of which run in parallel. The linked build https://github.com/mkoeppe/sage/actions/runs/285244461 is the cygwin-standard build, which means that I install binary packages from the Cygwin distribution first and then start the build of Sage.

stage-i-a and stage-i-b ran in parallel and succeeded. What they install in SAGE_LOCAL is carried forward to stage-ii. The 5 jobs stage-ii-a to stage-ii-e all ran in parallel, building various packages.

As you observed, stage-ii-a failed because of scipy. This is a bug unrelated to the present ticket; it is tracked in #30643. Unfortunately this bug blocks the following stages (stage-iii would go on to build the Sage library), and so because of this we cannot really check that the current ticket is working correctly.

simon-king-jena commented 4 years ago
comment:48

Replying to @mkoeppe:

Unfortunately this bug blocks the following stages (stage-iii would go on to build the Sage library), and so because of this we cannot really check that the current ticket is working correctly.

We cannot test automatically (on github), but a developer with a cygwin machine certainly could...

mkoeppe commented 4 years ago

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/285244461 to https://github.com/mkoeppe/sage/actions/runs/287249195

mkoeppe commented 4 years ago

Changed branch from u/SimonKing/cygwinmake_sure_all_packages_build_shared_librariesusing__am_ldflags__noundefined to u/mkoeppe/cygwinmake_sure_all_packages_build_shared_librariesusing__am_ldflags__noundefined

mkoeppe commented 4 years ago
comment:51

Replying to @simon-king-jena:

We cannot test automatically (on github), but a developer with a cygwin machine certainly could...

We seem to be in short supply of these...

I have modified the CI script so that the scipy failure does not stop the whole build.

The resulting build logs (https://github.com/mkoeppe/sage/suites/1285136303/artifacts/20016238) show that cliquer, giac, meataxe, and rw, i.e., all packages mentioned in the ticket description except for libbraiding (for which the branch contains an update) and singular (for which the branch does not contain a change) are OK now. Some optional packages were not built previously. In total, we now have the following remaining issues:

egret:~/Downloads/logs-commit-e41329adfa1f1e69653ba127d6460d2bd7aaaf00-cygwin-standard$ grep "undefined symbols not allowed" pkgs/*
pkgs/barvinok-0.41.1.log:libtool: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries; building static only
pkgs/e_antic-0.1.8.log:libtool: warning: undefined symbols not allowed in x86_64-pc-cygwin shared libraries; building static only
pkgs/isl-0.20.log:libtool: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries; building static only
pkgs/libbraiding-1.1.log:libtool: warning: undefined symbols not allowed in x86_64-pc-cygwin shared libraries; building static only
pkgs/polylib-5.22.5.log:libtool: link: warning: undefined symbols not allowed in x86_64-unknown-cygwin shared libraries
pkgs/singular-4.1.1p2.p0.log:libtool: warning: undefined symbols not allowed in x86_64-unknown-cy(base) eg(ba(base) egr(ba(base) egret(ba(base) (base)(base)(bas(b(bas(base)(((b((

New commits:

e41329aci-cygwin-standard.yml: More stages, continue-on-error: true
mkoeppe commented 4 years ago

Changed commit from cd74ef3 to e41329a

mkoeppe commented 4 years ago
comment:53

Let's get this in so that other tickets can use it as a dependency.

Follow up for the remaining issues in #30814.

mkoeppe commented 4 years ago

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/287249195 to https://github.com/mkoeppe/sage/actions/runs/287249195, ...

mkoeppe commented 4 years ago
comment:55

Needs review

dimpase commented 4 years ago
comment:56

there seems to be some problem with building of scipy

 In file included from /usr/include/sys/config.h:5,
                   from /usr/include/_ansi.h:11,
                   from /usr/include/sys/reent.h:13,
                   from /usr/include/math.h:5,
                   from /usr/lib/gcc/x86_64-pc-cygwin/10/include/c++/cmath:45,
                   from scipy/spatial/ckdtree/src/query.cxx:1:
  /usr/include/sys/features.h:255: note: this is the location of the previous definition
    255 | #define __BSD_VISIBLE  0
        |
  In file included from /cygdrive/d/a/sage/sage/local/include/python3.8/pyport.h:219,
                   from /cygdrive/d/a/sage/sage/local/include/python3.8/Python.h:63,
                   from /cygdrive/d/a/sage/sage/local/lib/python3.8/site-packages/numpy/core/include/numpy/npy_common.h:11,
                   from scipy/spatial/ckdtree/src/ckdtree_decl.h:10,
                   from scipy/spatial/ckdtree/src/query.cxx:13:
  /usr/include/sys/time.h:106:34: error: 'u_int' has not been declared
    106 | bintime_mul(struct bintime *_bt, u_int _x)
        |                                  ^~~~~
  g++: scipy/spatial/ckdtree/src/build.cxx
mkoeppe commented 3 years ago

Dependencies: #30154

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

0bb7fc0Merge tag '9.3.beta3' into t/29152/cygwin__make_sure_all_packages_build_shared_libraries__using__am_ldflags__no_undefined_
7244a9dci-cygwin-standard.yml: Use tar --listed-incremental
5ccf1eatar --remove-files and --listed-incremental are not compatible
9661065Merge branch 't/30154/gh_actions__cygwin__use_incremental_archives_for_the_sage_local_artifact' into t/29152/cygwin__make_sure_all_packages_build_shared_libraries__using__am_ldflags__no_undefined_
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from e41329a to 9661065

mkoeppe commented 3 years ago
comment:59

Replying to @dimpase:

there seems to be some problem with building of scipy

  /usr/include/sys/time.h:106:34: error: 'u_int' has not been declared

Probably fixed in #30643

mkoeppe commented 3 years ago

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/287249195, ... to https://github.com/mkoeppe/sage/actions/runs/412156307, ...

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

4c34a0eUpdate ci-cygwin-minimal.yml
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9661065 to 4c34a0e

mkoeppe commented 3 years ago

Work Issues: rebase on top of #31064

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 4c34a0e to 58c22df

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

cea4cd5ci-cygwin-standard.yml: More stages, continue-on-error: true
cf31b79Fixup after cherry-pick
73b0e8fbuild/pkgs/rw/SPKG.rst: Update link to upstream project
9d724bclibbraiding version bump
a67a446build/pkgs/meataxe/distros: Add fedora.txt
c79da6abuild/pkgs/cliquer: Update to 1.22
b5dcb16build/pkgs/cliquer/SPKG.rst: Update
9e6de0bbuild/pkgs/rw: Update to 0.9
32385deUpgrade SharedMeatAxe, to fix a build problem on Cygwin
58c22dfAdd upstream url for SharedMeatAxe
mkoeppe commented 3 years ago

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/412156307, ... to Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/432820484, ...

mkoeppe commented 3 years ago

Changed work issues from rebase on top of #31064 to none

mkoeppe commented 3 years ago

Changed dependencies from #30154 to #31064

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

09d1737Makefile: Add targets ptest-nodoc etc.
0dca776Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/31084/makefile__add__ptest__targets_that_do_not_depend_on_the_docbuild
073124cMerge branch 't/31084/makefile__add__ptest__targets_that_do_not_depend_on_the_docbuild' into t/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq
8769bd6.github/workflows/ci-cygwin-*.yml: Separate docbuild and ptest
c2daec7Merge branch 't/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq' into t/29152/cygwin__make_sure_all_packages_build_shared_libraries__using__am_ldflags__no_undefined_
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 58c22df to c2daec7

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

78ff9d5src/sage/misc/package.py: Add one more # optional - build
a44042fMerge branch 't/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available' into t/29124/script-packages-prereq-toolchain-bootstrap
64bde5fMerge tag '9.3.beta5' into t/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available
e9a7572src/sage/misc/package.py: Improve source formatting
c7bcda9Merge branch 't/30940/src_bin_sage_list_packages__make_it_work_if_sage_root_is_not_available' into t/29124/script-packages-prereq-toolchain-bootstrap
9988c5fci-cygwin*.yml: Adjust to new script packages _bootstrap, _prereq
d65299cMerge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq
ab19133Merge branch 't/29124/script-packages-prereq-toolchain-bootstrap' into t/30944/tox__improve_local_sudo_ubuntu_standard
a5e4051Merge branch 't/30944/tox__improve_local_sudo_ubuntu_standard' into t/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq
76804dcMerge branch 't/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq' into t/29152/cygwin__make_sure_all_packages_build_shared_libraries__using__am_ldflags__no_undefined_
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from c2daec7 to 76804dc

mkoeppe commented 3 years ago
comment:68

Let's please get this in.

dimpase commented 3 years ago
comment:69

It still doesn't fix all the Cygwin things, no? Anyway, LGTM.

dimpase commented 3 years ago

Changed reviewer from Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/432820484, ... to Matthias Koeppe, Dima Pasechnik

mkoeppe commented 3 years ago
comment:70

Thank you!

Follow up for the remaining issues in #30814.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

42f4458Merge tag '9.3.beta7' into t/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq
6b6ee5cMerge branch 't/31064/ci_cygwin__yml__adjust_to_new_script_packages__bootstrap___prereq' into t/29152/cygwin__make_sure_all_packages_build_shared_libraries__using__am_ldflags__no_undefined_
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 76804dc to 6b6ee5c

vbraun commented 3 years ago

Changed branch from u/mkoeppe/cygwinmake_sure_all_packages_build_shared_librariesusing__am_ldflags__noundefined to 6b6ee5c