Skip to content

Commit

Permalink
snappy: build shared library
Browse files Browse the repository at this point in the history 
See f689a6d

Closes #32880
  • Loading branch information
@orivej
orivej committed Dec 20, 2017
1 parent 3445138 commit febcb66
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
13 changes: 6 additions & 7 deletions pkgs/development/libraries/snappy/default.nix
View file
Expand Up @@ -11,18 +11,17 @@ stdenv.mkDerivation rec {
sha256 = "1x7r8sjmdqlqjz0xfiwdyrqpgaj5yrvrgb28ivgpvnxgar5qv6m2";
};

patches = [ ./disable-benchmark.patch ];

outputs = [ "out" "dev" ];

nativeBuildInputs = [ cmake ];

# -DNDEBUG for speed
configureFlags = [ "CXXFLAGS=-DNDEBUG" ];
cmakeFlags = [ "-DBUILD_SHARED_LIBS=ON" "-DCMAKE_SKIP_BUILD_RPATH=OFF" ];

This comment has been minimized.

Sign in to view

Copy link
@dezgeg

dezgeg Dec 20, 2017

Contributor

Is this a common CMake flag or something specific to snappy? Sounds like it should be always on.

This comment has been minimized.

Sign in to view

Copy link
@orivej

orivej Dec 20, 2017

Author Contributor

BUILD_SHARED_LIBS is a common flag. However, it affects all ADD_LIBRARY calls without explicit STATIC or SHARED qualifier, including internal project libraries that make part of installed programs and shared libraries but are not normally installed themselves. I don't know if this is harmful in any way, it may be worth investigating. Nixpkgs enable it for quiet a few projects. It may break projects that are not regularly tested with it: http://llvm.1065342.n5.nabble.com/llvm-dev-Proposal-Enable-BUILD-SHARED-LIBS-ON-by-default-for-debug-build-tp90368p90376.html

This comment has been minimized.

Sign in to view

Copy link
@orivej

orivej Dec 20, 2017

Author Contributor

BUILD_STATIC_LIBS may be the best practice among projects supporting simultaneous build of shared and static libraries but it has no special meaning for CMake, unlike BUILD_SHARED_LIBS. The latter defaults to OFF, and @dezgeg was asking if we should enable it by default. Since it affects project-internal libraries, I expect that many projects won't build on macOS because it checks that shared libraries have sufficient dependencies (equivalent to -Wl,--no-undefined on Linux), and in my experience internal libraries do not specify all of them unless they are tested in such configuration; and they also might not get installed.

This comment has been minimized.

Sign in to view

Copy link
@dezgeg

dezgeg Dec 20, 2017

Contributor

Ok, thanks for the fixes and explanations. As I said in #32880 (comment), I don't personally need the static library.

checkTarget = "test";

# SIGILL on darwin
doCheck = !stdenv.isDarwin;
checkPhase = ''
(cd .. && ./build/snappy_unittest)
'';
doCheck = true;

meta = with stdenv.lib; {
homepage = https://google.github.io/snappy/;
Expand Down
5 changes: 5 additions & 0 deletions pkgs/development/libraries/snappy/disable-benchmark.patch
View file
@@ -0,0 +1,5 @@
--- a/snappy-test.cc

This comment has been minimized.

Sign in to view

Copy link
@dezgeg

dezgeg Dec 20, 2017

Contributor

Why this change?

This comment has been minimized.

Sign in to view

Copy link
@orivej

orivej Dec 20, 2017

Author Contributor

See https://github.com/google/snappy#tests-and-benchmarks

Benchmarks can't be disabled from the command line without linking to gflags.
+++ b/snappy-test.cc
@@ -46 +46 @@
-DEFINE_bool(run_microbenchmarks, true,
+DEFINE_bool(run_microbenchmarks, false,

0 comments on commit febcb66

Please sign in to comment.