Opened 3 years ago

Closed 20 months ago

#28890 closed task (fixed)

Install fewer static libraries

Reported by: embray Owned by: embray
Priority: major Milestone: sage-9.4
Component: build Keywords: static
Cc: mmezzarobba, dimpase, mjo, jhpalmieri, vdelecroix Merged in:
Authors: Erik Bray, Matthias Koeppe Reviewers: Marc Mezzarobba, John Palmieri
Report Upstream: N/A Work issues:
Branch: c8ccab6 (Commits, GitHub, GitLab) Commit: c8ccab680254f106ecaf1ce97393492b13813c23
Dependencies: Stopgaps:

GitHub link to the corresponding issue

Description (last modified by mjo)

As pointed out on sage-devel, by default many SPKGs install static libraries in addition to their associated dynamic libs.

I usually ignored this as harmless, but as Marc pointed out, some of them are quite large, especially libgiac.a. Almost none of these are actually needed either at build or runtime and do not need to be installed by default in SAGE_LOCAL.

I think by default we should configure the relevant packages to not install static libs, or remove them manually if the packages' build system installs them forcibly. This could be overridden with <PKGNAME>_CONFIGURE variables if someone needs the static lib for some reason.

At least the following packages are installing static libs

$ for f in local/var/lib/sage/installed/*; do if grep '\.a\"' "$f" > /dev/null; then echo "$f" | cut -d'/' -f6; fi; done
cddlib-0.94j
cliquer-1.21.p4
e_antic-0.1.3
ecl-16.1.2.p5
eclib-20190909
ecm-7.0.4.p1
fplll-5.2.1
freetype-2.9.1
gap-4.10.2.p1
gc-7.6.4.p0
gf2x-1.2.p0
giac-1.5.0.63.p0
givaro-4.1.1
gsl-2.5
iml-1.0.4p1.p2
libatomic_ops-7.6.2
libgd-2.1.1.1.p1
libpng-1.6.29.p1
lrcalc-1.2.p1
mpc-1.1.0
mpfi-1.5.2
mpfr-4.0.1.p0
mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0
ncurses-6.0.p0
normaliz-3.7.2
ntl-11.3.2
numpy-1.16.1
pari-2.11.1.p2
planarity-3.0.0.5.p0
ppl-1.2.p1
python2-2.7.15.p1
python3-3.7.3.p1
ratpoints-2.1.3.p5
rw-0.7.p0
singular-4.1.1p2.p0
sqlite-3290000
symmetrica-2.0.p11
yasm-1.3.0.p0
zeromq-4.2.5
zlib-1.2.11.p0

In at least a few cases, like libatomic_ops, I believe this is intentional. But in most cases it is probably unnecessary.

Additional notes:

  • cliquer: it already passes --disable-static but it installs a test file into share/ which happens to end with .a but is not a static lib as far as I can tell.
  • ecl: The .a modules installed with ECL seem to be pretty baked into its build process and I'm not sure if there's an easy way to omit them or if it's even desirable to.
  • ecm: apparently we deliberately build it as a static lib by default; not sure why. Need to investigate (performance reasons?)
  • gap: The SmallGrp package also installs some ".a" file that are not static libs; otherwise GAP is not installing any static libs (some things in gap_packages might but I haven't checked yet).
  • mpir: mpir/gmp is linked statically for at least some libraries such as ECM, and possibly some others (GP?).
  • ntl: NTL's build system makes static library installation pretty mandatory--I don't know if there's any good reason for that or not, so for now we can just leave it alone.
  • python2/3: The static lib installation (performed by the libainstall make target) is a part of Python's standard installation process and more trouble to remove than it's worth, although I don't think we explicitly need it for anything.
  • ratpoints: Has only ever supported being built as a static library, and is statically linked into Sage's ratpoints interface.
  • yasm: If I understand correctly, libyasm contains some special support functions and is intended to be linked statically.

Change History (64)

comment:1 Changed 3 years ago by embray

Description: modified (diff)

comment:2 Changed 3 years ago by embray

Description: modified (diff)

comment:3 Changed 3 years ago by embray

Authors: Erik Bray
Branch: u/embray/ticket-28890
Commit: f773cd4a4ef33c490163f66ddda6a36d1ac75809
Description: modified (diff)

Passing --disable-static to all autoconf-compatible packages by default takes care of most of them, narrowing the list down to:

ecl-16.1.2.p5
ecm-7.0.4.p1
libatomic_ops-7.6.2
mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0
ntl-11.3.2
numpy-1.16.1
pari-2.11.1.p2
python2-2.7.15.p1
python3-3.7.3.p1
ratpoints-2.1.3.p5
symmetrica-2.0.p11
yasm-1.3.0.p0

Left libatomic_ops static since I think gc likes to link to it statically for performance reasons, but I need to double-check that it's actually doing that.

The rest I'm not sure about yet.


New commits:

f773cd4Trac #28890: Pass --disable-static to autoconf packages by default.

comment:4 Changed 3 years ago by dimpase

OK, let me run tests...

comment:5 Changed 3 years ago by dimpase

the branch looks good, do you want it to be reviewed?

comment:6 Changed 3 years ago by mmezzarobba

Thanks a lot for taking care of that. (You are crossing an item off the depths of my todo-list!)

I just rebuilt sage from scratch with this branch. With standard packages only and using system packages when possible, the remaining static libraries are

107M    ./local/lib/libpari.a
48M     ./local/lib/libsymmetrica.a
31M     ./local/lib/python3.7/config-3.7m-x86_64-linux-gnu/libpython3.7m.a
7,7M    ./local/lib/ecl-16.1.2/libcmp.a
7,5M    ./local/lib/libbraiding.a
5,4M    ./local/lib/libyasm.a
4,5M    ./local/lib/ecl-16.1.2/libasdf.a
3,1M    ./local/lib/libecm.a
880K    ./local/lib/ecl-16.1.2/libdefsystem.a
564K    ./local/lib/ecl-16.1.2/libdeflate.a
448K    ./local/lib/ecl-16.1.2/libsockets.a
372K    ./local/lib/python3.7/site-packages/numpy/core/lib/libnpymath.a
232K    ./local/lib/ecl-16.1.2/libecl-help.a
200K    ./local/lib/ecl-16.1.2/libprofile.a
184K    ./local/lib/ecl-16.1.2/librt.a
176K    ./local/lib/libratpoints.a
172K    ./local/lib/libhomfly.a
172K    ./local/lib/ecl-16.1.2/libecl-cdb.a
140K    ./local/lib/ecl-16.1.2/libecl-curl.a
132K    ./local/lib/ecl-16.1.2/libserve-event.a
132K    ./local/lib/ecl-16.1.2/libql-minitar.a
96K     ./local/lib/ecl-16.1.2/libecl-quicklisp.a
72K     ./local/lib/ecl-16.1.2/libsb-bsd-sockets.a
32K     ./local/lib/libatomic_ops_gpl.a
24K     ./local/lib/libatomic_ops.a

So most of the large ones are gone, and those that remain may have a good reason to be there. Like Dima, I'm ready to give this branch positive review if you want.

comment:7 Changed 3 years ago by embray

I want to have a look at a few of the remaining libs. Also, I got a few test failures with this branch, but I don't know if they're at all related (shouldn't be...)

comment:8 in reply to:  7 Changed 3 years ago by embray

Replying to embray:

Also, I got a few test failures with this branch, but I don't know if they're at all related (shouldn't be...)

They weren't related. It was the problem that #28289 is intended to fix. Turns out my develop branch was a little behind.

comment:9 Changed 3 years ago by embray

Milestone: sage-9.0sage-9.1

Since we are apparently already moving 9.0 towards a release candidate, this will probably have to wait a bit.

comment:10 Changed 3 years ago by git

Commit: f773cd4a4ef33c490163f66ddda6a36d1ac75809f99562a453728f08743ce3572a5bb20971e51659

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

a8cb064Trac #28890: Pass --disable-static to autoconf packages by default.
f99562aTrac #28890: Don't install PARI's static library.

comment:11 Changed 3 years ago by embray

Updated to remove libpari.a. Apparently we had been explicitly installing it pretty much forever, so I couldn't find a clear record of why, but I don't think there's any need for it currently.

My only concern here is there no easy way (since it is not a flag that can be passed to PARI_CONFIGURE) to force installation of the static lib. But if someone asks for it for some reason it can be added later.

comment:12 in reply to:  11 ; Changed 3 years ago by dimpase

Replying to embray:

Updated to remove libpari.a. Apparently we had been explicitly installing it pretty much forever, so I couldn't find a clear record of why, but I don't think there's any need for it currently.

pari/gp upstream claims that statically linked gp is faster than dynamically linked one.

comment:13 Changed 3 years ago by embray

Description: modified (diff)

comment:14 in reply to:  12 Changed 3 years ago by embray

Replying to dimpase:

Replying to embray:

Updated to remove libpari.a. Apparently we had been explicitly installing it pretty much forever, so I couldn't find a clear record of why, but I don't think there's any need for it currently.

pari/gp upstream claims that statically linked gp is faster than dynamically linked one.

If so, it appears we are already not doing this, though we could. Regardless, this can be done during PARI/GP build: there is no need to install the static lib after the fact.

comment:15 Changed 3 years ago by embray

Description: modified (diff)

comment:16 Changed 3 years ago by embray

Description: modified (diff)

comment:17 Changed 3 years ago by embray

libbraiding and libhomfly will be handled by this as well once #28927 is in.

comment:18 Changed 3 years ago by embray

Description: modified (diff)

comment:19 Changed 3 years ago by embray

Description: modified (diff)
Status: newneeds_review

I think that with the last commit this pretty much covers everything. I looked into all the other packages that are installing static libs, and determined that it's either on purpose, or too much trouble to mess with as part of this ticket.

comment:20 Changed 3 years ago by mmezzarobba

Reviewers: Marc Mezzarobba
Status: needs_reviewpositive_review

Thank you again!

comment:21 Changed 3 years ago by vbraun

Branch: u/embray/ticket-28890f99562a453728f08743ce3572a5bb20971e51659
Resolution: fixed
Status: positive_reviewclosed

comment:22 Changed 3 years ago by vbraun

Commit: f99562a453728f08743ce3572a5bb20971e51659
Resolution: fixed
Status: closednew
****************************************************
Configuring zlib-1.2.11.p0
unknown option: --disable-static
./configure --help for help
** ./configure aborting.
unknown option: --disable-static
./configure --help for help
** ./configure aborting.
********************************************************************************
Error configuring zlib-1.2.11.p0
********************************************************************************

comment:23 Changed 3 years ago by embray

Branch: f99562a453728f08743ce3572a5bb20971e51659u/embray/ticket-28890
Commit: f99562a453728f08743ce3572a5bb20971e51659
Owner: set to embray

New commits:

a8cb064Trac #28890: Pass --disable-static to autoconf packages by default.
f99562aTrac #28890: Don't install PARI's static library.

comment:24 Changed 3 years ago by embray

Hrm, that's annoying. Thanks, zlib...

comment:25 Changed 3 years ago by git

Commit: f99562a453728f08743ce3572a5bb20971e5165929445a8dd31b81190fa7eee251ccb99c4bfd75e0

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

29445a8Trac #28890: Don't use sdh_configure to configure zlib; it does not use

comment:26 Changed 3 years ago by embray

Status: newneeds_review

New commits:

29445a8Trac #28890: Don't use sdh_configure to configure zlib; it does not use
Last edited 3 years ago by embray (previous) (diff)

comment:27 Changed 3 years ago by embray

Status: needs_reviewpositive_review

comment:28 Changed 3 years ago by vbraun

I'm getting libpng build errors on KuCalc (Debian 9 x86_64) because something is wrong with zlib dynamic linking:

libtool: link: gcc -fPIC -g -Wl,-rpath -Wl,/var/lib/buildbot/slave/sage_git/build/local/lib -o .libs/pngunknown contrib/libtests/pngunknown.o  -L/var/lib/buildbot/slave/sage_git/build/local/lib ./.libs/libpng16.so -lz -lm -Wl,-rpath -Wl,/var/lib/buildbot/slave/sage_git/build/local/lib
collect2: error: ld returned 1 exit status
Makefile:1026: recipe for target 'pngtest' failed
make[5]: *** [pngtest] Error 1
./.libs/libpng16.so: undefined reference to `inflateValidate'

comment:29 Changed 3 years ago by vbraun

Status: positive_reviewneeds_work

comment:30 Changed 3 years ago by dimpase

I presume build/pkgs/libpng/spkg-install needs LDFLAGS set to point at $SAGE_LOCAL/lib/ in the case of zlib not coming from the system. (and the same for the other zlib deps, I guess)

comment:31 in reply to:  30 Changed 3 years ago by embray

Replying to dimpase:

I presume build/pkgs/libpng/spkg-install needs LDFLAGS set to point at $SAGE_LOCAL/lib/ in the case of zlib not coming from the system. (and the same for the other zlib deps, I guess)

Don't we already do that for all packages? The failing command Volker posted already had -L/var/lib/buildbot/slave/sage_git/build/local/lib as well as -lz.

comment:32 Changed 3 years ago by dimpase

could it be the order of libs issue?

Or something to do with libpng using to carry its own zlib...

comment:33 Changed 3 years ago by mkoeppe

Milestone: sage-9.1sage-9.2

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

comment:34 Changed 3 years ago by mkoeppe

See also: #23519 - SAGE_FAT_BINARY fails on Debian 9 64-bit

comment:35 Changed 2 years ago by mkoeppe

Branch: u/embray/ticket-28890u/mkoeppe/ticket-28890

comment:36 Changed 2 years ago by mkoeppe

Commit: 29445a8dd31b81190fa7eee251ccb99c4bfd75e0fdd83972d17ae1f6765d2ee03a890c715e3b0c9b
Dependencies: #30564

New commits:

cace562Trac #28890: Pass --disable-static to autoconf packages by default.
8509b25Trac #28890: Don't install PARI's static library.
fdd8397Trac #28890: Don't use sdh_configure to configure zlib; it does not use

comment:37 Changed 2 years ago by mkoeppe

Dependencies: #30564#30564, #29152

comment:38 Changed 2 years ago by mkoeppe

Milestone: sage-9.2sage-9.3

comment:39 Changed 2 years ago by mkoeppe

Cc: mjo added

comment:40 in reply to:  36 Changed 2 years ago by mjo

Replying to mkoeppe:

New commits: ...

Should this be Needs Review now?

comment:41 Changed 2 years ago by mjo

Description: modified (diff)

I've removed the note about symmetrica from the description, since we took over maintenance and added an autotools build system.

comment:42 Changed 2 years ago by mkoeppe

Milestone: sage-9.3sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:43 Changed 22 months ago by git

Commit: fdd83972d17ae1f6765d2ee03a890c715e3b0c9bcd39c76bffd159da42e8c7aabaa297c78386b17e

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

3f4a8efTrac #28890: Pass --disable-static to autoconf packages by default.
8a8efeeTrac #28890: Don't install PARI's static library.
cd39c76Trac #28890: Don't use sdh_configure to configure zlib; it does not use

comment:44 Changed 22 months ago by mkoeppe

Dusted off this ticket. It will be helpful to reduce the size of wheels in #31396

comment:45 Changed 22 months ago by mkoeppe

Status: needs_workneeds_review

comment:46 Changed 22 months ago by mkoeppe

Reviewers: Marc MezzarobbaMarc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/732049335

comment:47 Changed 22 months ago by mkoeppe

Status: needs_reviewneeds_work

A typo in the change to zlib broke lots of platforms

comment:48 Changed 22 months ago by git

Commit: cd39c76bffd159da42e8c7aabaa297c78386b17e9622d83ba544358bf27a2054c83b3ccdfecee2ac

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

9622d83build/pkgs/zlib/spkg-install.in: Fixup

comment:49 Changed 22 months ago by git

Commit: 9622d83ba544358bf27a2054c83b3ccdfecee2ac3d701f896fbe428d70a5e9f20efd66f9a91ddb6c

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

3d701f8build/pkgs/zlib/spkg-install.in: Handle errors from configure

comment:50 Changed 22 months ago by mkoeppe

Reviewers: Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/732049335Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/733869032

comment:52 Changed 22 months ago by mkoeppe

Status: needs_workneeds_review

comment:53 Changed 22 months ago by mkoeppe

Cc: jhpalmieri vdelecroix added
Dependencies: #30564, #29152

comment:54 Changed 22 months ago by mkoeppe

Authors: Erik BrayErik Bray, Matthias Koeppe

comment:55 Changed 22 months ago by jhpalmieri

On OS X (with a bunch of homebrew packages), this prevents building of the following:

libbraiding.a
libcdd.a
libcddgmp.a
libec.a
libecm.a
libfplll.a
libgf2x.a
libgivaro.a
libhomfly.a
libiml.a
liblrcalc.a
libpari.a
libplanarity.a
libratpoints.a
librw.a
libsymmetrica.a

libecm.a and libratpoints.a are still built, as it says in the ticket description.

We should probably build without as many homebrew packages, to make sure things still work.

comment:56 Changed 22 months ago by jhpalmieri

With ./configure --with-system-python3=no --with-system-ntl=no --with-system-gmp=no, I see libgmp.a, libgmpxx.a, and libntl.a still present. I think this is still consistent with the ticket description.

Last edited 22 months ago by jhpalmieri (previous) (diff)

comment:57 Changed 22 months ago by mkoeppe

Status: needs_reviewneeds_work

The runs at https://github.com/mkoeppe/sage/actions/runs/733869032 (linux/macos), https://github.com/mkoeppe/sage/actions/runs/732049334 (optional packages), were also successful.

On cygwin-standard (https://github.com/mkoeppe/sage/runs/2309214385), there is an issue with the standard package libbraiding - for which building shared libraries is broken (#30814)

comment:58 Changed 22 months ago by git

Commit: 3d701f896fbe428d70a5e9f20efd66f9a91ddb6c56178ab6ff9dca4cf50e995387affe27a3330eed

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

56178abbuild/bin/sage-dist-helpers (sdh_configure): On Cygwin, do not use --disable-static (until #30814 is done)

comment:59 Changed 22 months ago by mkoeppe

Status: needs_workneeds_review

comment:60 Changed 22 months ago by mkoeppe

Reviewers: Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/733869032Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/738661838

comment:61 Changed 22 months ago by git

Commit: 56178ab6ff9dca4cf50e995387affe27a3330eedc8ccab680254f106ecaf1ce97393492b13813c23

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

c8ccab6build/bin/sage-dist-helpers: Fixup: = instead of ==

comment:62 Changed 21 months ago by jhpalmieri

Reviewers: Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/738661838Marc Mezzarobba, John Palmieri
Status: needs_reviewpositive_review

I'm not sure why I can't view the branch attached to this ticket, but it merges cleanly with Sage 9.3. I think we should add this to an early beta of 9.4 (or 9.5 if 9.4 is a bug-fix release) to make sure it gets tested thoroughly.

comment:63 Changed 21 months ago by mkoeppe

Thanks!

comment:64 Changed 20 months ago by vbraun

Branch: u/mkoeppe/ticket-28890c8ccab680254f106ecaf1ce97393492b13813c23
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.