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

Install fewer static libraries #28890

Closed
embray opened this issue Dec 16, 2019 · 79 comments
Closed

Install fewer static libraries #28890

embray opened this issue Dec 16, 2019 · 79 comments
Assignees
Milestone

Comments

@embray
Copy link
Contributor

embray commented Dec 16, 2019

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.

CC: @mezzarobba @dimpase @orlitzky @jhpalmieri @videlec

Component: build

Keywords: static

Author: Erik Bray, Matthias Koeppe

Branch/Commit: c8ccab6

Reviewer: Marc Mezzarobba, John Palmieri

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

@embray embray added this to the sage-9.0 milestone Dec 16, 2019
@embray

This comment has been minimized.

@embray

This comment has been minimized.

@embray

This comment has been minimized.

@embray
Copy link
Contributor Author

embray commented Dec 16, 2019

Commit: f773cd4

@embray
Copy link
Contributor Author

embray commented Dec 16, 2019

Author: Erik Bray

@embray
Copy link
Contributor Author

embray commented Dec 16, 2019

Branch: u/embray/ticket-28890

@embray
Copy link
Contributor Author

embray commented Dec 16, 2019

comment:3

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.

@dimpase
Copy link
Member

dimpase commented Dec 16, 2019

comment:4

OK, let me run tests...

@dimpase
Copy link
Member

dimpase commented Dec 16, 2019

comment:5

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

@mezzarobba
Copy link
Member

comment:6

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.

@embray
Copy link
Contributor Author

embray commented Dec 17, 2019

comment:7

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...)

@embray
Copy link
Contributor Author

embray commented Dec 17, 2019

comment:8

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.

@embray
Copy link
Contributor Author

embray commented Dec 30, 2019

comment:9

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

@embray embray modified the milestones: sage-9.0, sage-9.1 Dec 30, 2019
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 30, 2019

Changed commit from f773cd4 to f99562a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 30, 2019

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.

@embray
Copy link
Contributor Author

embray commented Dec 30, 2019

comment:11

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.

@dimpase
Copy link
Member

dimpase commented Dec 30, 2019

comment:12

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.

@embray

This comment has been minimized.

@embray
Copy link
Contributor Author

embray commented Dec 30, 2019

comment:14

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.

@embray

This comment has been minimized.

@embray

This comment has been minimized.

@embray
Copy link
Contributor Author

embray commented Dec 30, 2019

comment:17

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

@embray

This comment has been minimized.

@embray

This comment has been minimized.

@embray
Copy link
Contributor Author

embray commented Dec 30, 2019

comment:19

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2021

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2021

Changed commit from 9622d83 to 3d701f8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2021

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

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 9, 2021

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 9, 2021

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 10, 2021

Changed dependencies from #30564, #29152 to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 10, 2021

Changed author from Erik Bray to Erik Bray, Matthias Koeppe

@jhpalmieri
Copy link
Member

comment:55

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.

@jhpalmieri
Copy link
Member

comment:56

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2021

comment:57

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)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2021

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)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2021

Changed commit from 3d701f8 to 56178ab

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2021

comment:60

New test on cygwin at https://github.com/mkoeppe/sage/actions/runs/738661838

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2021

Changed commit from 56178ab to c8ccab6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2021

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

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

@jhpalmieri
Copy link
Member

Changed reviewer from Marc Mezzarobba, https://github.com/mkoeppe/sage/actions/runs/738661838 to Marc Mezzarobba, John Palmieri

@jhpalmieri
Copy link
Member

comment:62

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 21, 2021

comment:63

Thanks!

@vbraun
Copy link
Member

vbraun commented Jun 6, 2021

Changed branch from u/mkoeppe/ticket-28890 to c8ccab6

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

7 participants