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

[wip] multiple-outputs: always build static libraries #41935

Closed
wants to merge 1 commit into from

Conversation

matthewbauer
Copy link
Member

Using multiple outputs we can easily build static libraries. This is a proposal to always statically link things. Needs testing & discussion before merging.

/cc @orivej @nlewo @xeji

Using multiple outputs we can easily build static libraries. This is a proposal to always statically link things. Needs testing & discussion before merging.

/cc @orivej @nlewo @xeji
@Ericson2314
Copy link
Member

Ericson2314 commented Jun 13, 2018

@matthewbauer https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/setup.sh#L960-L965 and any other uses of dontDisableStatic would be better controlling this than messing with stdenv itself.

@matthewbauer
Copy link
Member Author

matthewbauer commented Jun 13, 2018

@orivej Yeah it shouldn't disable shared libraries but I could see how some packages would think that.

@Ericson2314 shouldn't we have a "static" output anyway though? We don't want static libraries winding up in our closures.

@matthewbauer
Copy link
Member Author

matthewbauer commented Jun 13, 2018

The advantage of something like this is to avoid some of the package-specific hacks we are now using for static flags:

https://github.com/NixOS/nixpkgs/search?p=1&q=outputs+static&unscoped_q=outputs+static
https://github.com/NixOS/nixpkgs/search?q=--enable-static&unscoped_q=--enable-static

@Ericson2314
Copy link
Member

@matthewbauer sorry I meant your thing is better than those old things :). Let's go all in!

@@ -73,6 +77,7 @@ _multioutConfig() {
--docdir=${!outputDoc}/share/doc/${shareDocName} \
--libdir=${!outputLib}/lib --libexecdir=${!outputLib}/libexec \
--localedir=${!outputLib}/share/locale \
--enable-static \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly this should only be added if there is a static output. Even then, I fear it is to prescriptive but I'd love to be wrong!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If doing this, we should better also remove the --disable-static flag (on line ~963).

@@ -164,6 +169,11 @@ _multioutDevs() {
done
}

_multioutStatic() {
if [ "$outputs" = "out" ] || [ -z "${moveToStatic-1}" ]; then return; fi;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want this to run if there is only [ "out" "lib" ]? If not, try [[ "$outputs" = *static* ]]

@nlewo nlewo mentioned this pull request Jun 14, 2018
8 tasks
@@ -164,6 +169,11 @@ _multioutDevs() {
done
}

_multioutStatic() {
if [ "$outputs" = "out" ] || [ -z "${moveToStatic-1}" ]; then return; fi;
find lib -name "*.a" -exec moveToOutput "{}" "${!outputStatic}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moveToOutput is not a program, it can't be exec'ed.

@orivej
Copy link
Contributor

orivej commented Jun 15, 2018

--enable-static will also undermine the effect of #41819 (once enabled for all Nixpkgs) because it adds e.g. old_library='libxml2.a' to libxml2.la. This can be mitigated by clearing old_library in the original libtool file before moveToOutput static, or by teaching pruneLibtoolFiles to clear it when the static library is not present near the libtool file.

@dezgeg
Copy link
Contributor

dezgeg commented Jun 15, 2018

Another problem is that the static outputs aren't included by default, so anything wanting to build statically needs to add their entire dependency closure manually (see e.g. git grep '\<glibc.static\>' for the gore).

On a side note, in this age where executables are PIE anyway for hardening reasons, what is actually different (with regards to code generation) in shared objects compared to static objects? There's a project (https://github.com/endrazine/wcc) which claims to be able to "[...] to 'unlink' (undo the work of a linker) ELF binaries, either executables or shared libraries, back into relocatable shared objects.". I wonder if that would work as a modern day replacement of getting each and every build system of every single package build both static and dynamic libs correctly?

@vcunat
Copy link
Member

vcunat commented Jun 18, 2018

I had thought about this years ago, but I concluded that it will be better to do these as separate derivations and e.g. standardize something like myLibrary.override { static = true; } (though we'd probably need some kind of "deep" or half-automatic override).

There are a few reasons. One is that IIRC often you need to compile the objects twice anyway, due to using different flags, e.g. if you want static I gather you often don't want position-independent code (to prefer performance over some security advantages) – so you don't really save much CPU time by building them at once. And there are some technical problems, like you need to propagate a different set of dependencies, and also this horrible trouble: #12085

@matthewbauer
Copy link
Member Author

Yeah I am split on whether this is a good idea as well. I like the standardized ".static" output over the usually not standard "enableStatic" vs "useStatic" vs "static", etc. I want to enforce something like this & have it built by Hydra through something like #39580 with some Hydra release magic but that has stalled. I am closing this for now though because @vcunat raises some good points on the usefulness of this.

@vcunat
Copy link
Member

vcunat commented Jun 18, 2018

Well, standardizing some name like useStatic is the easiest part, I think. Other technical details will be much more important to the decision which way to go – I've posted some I've remembered, but I haven't really dug deep into this due to not being motivated enough for static linking.

@matthewbauer
Copy link
Member Author

Yes. Which do you prefer? I think enableStatic is the ideal one because it maps well to the very common --enable-static configure flag. The thing is we probably need a corresponding "enableShared" flag so that we can avoid having both in the closure.

@vcunat
Copy link
Member

vcunat commented Jun 18, 2018

I would expect that by default enabling one would disable the other. enableStatic sounds fine to me, but I don't have a strong preference.

@matthewbauer
Copy link
Member Author

Is that how the configure flag is supposed to work? I guess when I see --enable-static I had always assumed they weren't mutually exclusive - otherwise it would be something like --link-target=static or --link-target=shared but maybe that is the convention.

@vcunat
Copy link
Member

vcunat commented Jun 18, 2018

The configure flags probably aren't exclusive (in standard autotools packaging), but for our flags it seems to make more sense, at least if we go this way of preferring to separate the static and shared builds...

@matthewbauer matthewbauer deleted the always-static branch June 21, 2018 01:20
@nlewo nlewo mentioned this pull request Jun 22, 2018
9 tasks
@orivej orivej mentioned this pull request Jul 4, 2018
9 tasks
@vcunat vcunat mentioned this pull request Dec 14, 2018
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants