Skip to content

SDL2: Keep .a files on dontDisableStatic; don't move them to $dev; prune .la#72736

Merged
cpages merged 1 commit intoNixOS:stagingfrom
nh2:nix-sdl2-static-fixes
Nov 10, 2019
Merged

SDL2: Keep .a files on dontDisableStatic; don't move them to $dev; prune .la#72736
cpages merged 1 commit intoNixOS:stagingfrom
nh2:nix-sdl2-static-fixes

Conversation

@nh2
Copy link
Contributor

@nh2 nh2 commented Nov 3, 2019

Motivation for this change

Most other packages don't move .a files to "$dev", and that is because
it makes the pkg-config .pc file wrong (the libdir is the non-dev one).

Keeping them in the main output makes static linking of SDL2 work.

See added comment about pruning of .la files.

Tested in nh2/static-haskell-nix#65.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @cpages

FYI @matthewbauer

@nh2 nh2 requested review from cpages and viric November 3, 2019 19:06
@nh2
Copy link
Contributor Author

nh2 commented Nov 3, 2019

Relevant history of the SDL2 package and the moveToOutput in postInstall:

commit b81caba5fbb49012a5cabefb672f3b83bd524d1e
Author: Carles Pagès <page@cubata.homelinux.net>
Date:   Wed Nov 6 23:17:46 2013 +0100

   SDL2: some improvements to the expression.

commit da70d3da0f11b22eac77756b39b349215e06b2e3
Author: Linus Heckemann <git@sphalerite.org>
Date:   Sun Dec 25 01:11:07 2016 +0000

   SDL2: split derivation

commit 0c42efd9d70774eafb47212967caada630335731
Author: Lluís Batlle i Rossell <viric@viric.name>
Date:   Thu Feb 16 22:24:40 2017 +0100

   SDL2: fix creation of libSDL2main.a
   
   It's required by a trigger rally update I will commit next.
   And other games use that too.

commit 1e7da9ec78d11f586fc74747d77b17f91969a7ed
Author: Lluís Batlle i Rossell <viric@viric.name>
Date:   Thu Feb 16 22:25:23 2017 +0100

   trigger: update to 0.6.5

What the libtool problem looks like when static libs are enabled (this PR fixes that):

libtool: link: gcc -shared  .libs/SDL_ttf.o   -Wl,-rpath -Wl,/nix/store/ggafj30pr2sv0hj7jgfywvqv1yz88mws-freetype-2.10.1/lib -Wl,-rpath -Wl,/nix/store/mdxphvay0gaxrrm7hpanfc76j198bbvj-bzip2-1.0.6.0.1/lib -Wl,-rpath -Wl,/nix/store/q33gb957w94mqdqkq4pwil8hix34krwv-libpng-apng-1.6.37/lib -Wl,-rpath -Wl,/nix/store/x3szqg2c1rav8ja944583r1a1m6vmq86-SDL2-2.0.10/lib -Wl,-rpath -Wl,/nix/store/ggafj30pr2sv0hj7jgfywvqv1yz88mws-freetype-2.10.1/lib -Wl,-rpath -Wl,/nix/store/mdxphvay0gaxrrm7hpanfc76j198bbvj-bzip2-1.0.6.0.1/lib -Wl,-rpath -Wl,/nix/store/q33gb957w94mqdqkq4pwil8hix34krwv-libpng-apng-1.6.37/lib -Wl,-rpath -Wl,/nix/store/x3szqg2c1rav8ja944583r1a1m6vmq86-SDL2-2.0.10/lib -L/nix/store/ggafj30pr2sv0hj7jgfywvqv1yz88mws-freetype-2.10.1/lib /nix/store/ggafj30pr2sv0hj7jgfywvqv1yz88mws-freetype-2.10.1/lib/libfreetype.so -L/nix/store/2fd7k3j5vmmkzw14ywwjykw8sa4h0vdd-zlib-1.2.11/lib -L/nix/store/mdxphvay0gaxrrm7hpanfc76j198bbvj-bzip2-1.0.6.0.1/lib -L/nix/store/q33gb957w94mqdqkq4pwil8hix34krwv-libpng-apng-1.6.37/lib /nix/store/mdxphvay0gaxrrm7hpanfc76j198bbvj-bzip2-1.0.6.0.1/lib/libbz2.so /nix/store/q33gb957w94mqdqkq4pwil8hix34krwv-libpng-apng-1.6.37/lib/libpng16.so -lz -L/nix/store/x3szqg2c1rav8ja944583r1a1m6vmq86-SDL2-2.0.10/lib /nix/store/x3szqg2c1rav8ja944583r1a1m6vmq86-SDL2-2.0.10/lib/libSDL2.so -lm -lX11 -lXext -lXcursor -lXinerama -lXi -lXrandr -lXss -lXxf86vm -lpthread -lrt  -Wl,-rpath -Wl,/nix/store/x3szqg2c1rav8ja944583r1a1m6vmq86-SDL2-2.0.10/lib -Wl,--enable-new-dtags   -Wl,-soname -Wl,libSDL2_ttf-2.0.so.0 -o .libs/libSDL2_ttf-2.0.so.0.14.1
checking if g++ static flag -static works... yes
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXext
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXcursor
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXinerama
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXi
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXrandr
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXss
/nix/store/f1v9hcm7pxdrgmmarf5ldss9xc480l0n-binutils-2.31.1/bin/ld: cannot find -lXxf86vm
collect2: error: ld returned 1 exit status
make: *** [Makefile:517: libSDL2_ttf.la] Error 1

builder for '/nix/store/x1gslck2ab1r09by2jgpm6qzcn24d90a-SDL2_ttf-2.0.15.drv' failed with exit code 2

@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. labels Nov 3, 2019
@nh2
Copy link
Contributor Author

nh2 commented Nov 3, 2019

10.rebuild-linux: 501-1000 -- I'll switch it to staging instead.

@nh2 nh2 changed the base branch from master to staging November 3, 2019 20:44
@nh2 nh2 force-pushed the nix-sdl2-static-fixes branch from ffe81da to f08b711 Compare November 3, 2019 20:45
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this file (and other .a) still be removed when dontDisableStatic is true? Otherwise we get .a files in the SDL closure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right.

It seems SDL2 does not quite respect --disable-static; it still generates libSDL2main.a and libSDL2_test.a.

I've force-pushed an else branch that deletes them.

It also generates libSDL2main.la and libSDL2_test.la which are prett useless given that the .a files they mention are nonexistent. Should I delete them, or do we not care?

Also, with what comand can I rebuild everything that depends on SDL2, so that I can check if anything used the libSDL2main.a that before was in the $dev output?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the 'else' branch to keep removing the .a libs. Your force push failed? Otherwise looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your force push failed?

Oops, you are right. Fixed now, sorry.

@nh2
Copy link
Contributor Author

nh2 commented Nov 4, 2019

@GrahamcOfBorg eval

@nh2
Copy link
Contributor Author

nh2 commented Nov 7, 2019

@matthewbauer Good to go with the incorporated feedback?

…prune .la.

Most other packages don't move `.a` files to "$dev", and that is because
it makes the pkg-config `.pc` file wrong (the `libdir` is the non-dev one).

Keeping them in the main output makes static linking of SDL2 work.

See added comment about pruning of `.la` files.
@nh2 nh2 force-pushed the nix-sdl2-static-fixes branch from f08b711 to 57908c1 Compare November 8, 2019 22:13
@nh2
Copy link
Contributor Author

nh2 commented Nov 9, 2019

The ofborg build times out on building GCC 8 after 1 hour.

@cpages cpages merged commit 539e940 into NixOS:staging Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants