mujx / nheko

No longer maintained - Desktop client for the Matrix protocol (active fork https://github.com/Nheko-Reborn)
GNU General Public License v3.0
394 stars 65 forks source link

Nix build fails: `error: could not find git for clone of MatrixStructs` #271

Closed Slabity closed 6 years ago

Slabity commented 6 years ago

The program fails to build with the provided Nix instructions.

I believe the issue is caused by CMake trying to use git during the build process, which should be done before the build process begins.

Build Trace

$ nix-build
these derivations will be built:
  /nix/store/hxlybmq6nxqyr06aj8mn7mg7x9xbx96h-nheko-0.2.0.drv
building '/nix/store/hxlybmq6nxqyr06aj8mn7mg7x9xbx96h-nheko-0.2.0.drv'...
unpacking sources
unpacking source archive /nix/store/qbws8p7idcx9zmz8l7ghw4lvj8x5dm2m-nheko
source root is nheko
patching sources
configuring
fixing cmake files...
cmake flags: -DCMAKE_BUILD_TYPE=Release -DCMAKE_SKIP_BUILD_RPATH=ON -DCMAKE_INSTALL_INCLUDEDIR=/nix/store/wyqhhayagqzc2xgcz3hkyyh1ldkf9f3f-nheko-0.2.0/include -DCMAKE_INSTALL_LIBDIR=/nix/store/wyqhhayagqzc2xgcz3hkyyh1ldkf9f3f-nheko-0.2.0/lib -DCMAKE_INSTALL_NAME_DIR=/nix/store/wyqhhayagqzc2xgcz3hkyyh1ldkf9f3f-nheko-0.2.0/lib -DCMAKE_INSTALL_PREFIX=/nix/store/wyqhhayagqzc2xgcz3hkyyh1ldkf9f3f-nheko-0.2.0
-- The C compiler identification is GNU 7.3.0
-- The CXX compiler identification is GNU 7.3.0
-- Check for working C compiler: /nix/store/gqg2vrcq7krqi9rrl6pphvsg81sb8pjw-gcc-wrapper-7.3.0/bin/gcc
-- Check for working C compiler: /nix/store/gqg2vrcq7krqi9rrl6pphvsg81sb8pjw-gcc-wrapper-7.3.0/bin/gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /nix/store/gqg2vrcq7krqi9rrl6pphvsg81sb8pjw-gcc-wrapper-7.3.0/bin/g++
-- Check for working CXX compiler: /nix/store/gqg2vrcq7krqi9rrl6pphvsg81sb8pjw-gcc-wrapper-7.3.0/bin/g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found LMDB: /nix/store/ywa43pg1ad62wd1hpqdg0j5iin693j5l-lmdb-0.9.21-dev/include
Build type set to 'Release'
-- Version: 0.1.0
CMake Error at /nix/store/9bhialxbn35a1vlgjbvn6nwjgs54cl19-cmake-3.10.2/share/cmake-3.10/Modules/ExternalProject.cmake:2275 (message):
  error: could not find git for clone of MatrixStructs
Call Stack (most recent call first):
  /nix/store/9bhialxbn35a1vlgjbvn6nwjgs54cl19-cmake-3.10.2/share/cmake-3.10/Modules/ExternalProject.cmake:3029 (_ep_add_download_command)
  cmake/MatrixStructs.cmake:22 (ExternalProject_Add)
  CMakeLists.txt:217 (include)
mujx commented 6 years ago

I'm not really familiar with nix. CMake calls git clone to download some third party library. Why would that be a problem?

Should we add git as a dependency?

Slabity commented 6 years ago

One of the underlying strengths of Nix is that all packages are pure functions. This basically means they have no side effects; only inputs and an output.

Calling git clone (or accessing any resource not passed as a function argument) is a side effect, and results in the function being impure. This is forbidden by Nix, and enforced by preventing the build environment from accessing any resources not explicitly passed as an argument to the function.

Basically, you can't access the internet or anything outside of the build environment during a build.

mujx commented 6 years ago

Is there a way for nix to download the sources then? Cmake puts the cloned repo in .third-party/matrix-structs but even then I think cmake will try to check if the repo is synced with the upstream (using git).

Slabity commented 6 years ago

Is there a way for nix to download the sources then?

Yes. You can make a derivation for your matrix-structs library and then pass it into this one. I can try making a derivation of both and getting them into the nixpkgs repo.

I think cmake will try to check if the repo is synced with the upstream (using git).

Automatically updating sources is typically not recommended for release builds of most projects, even in other build systems. However, if these two repos are closely tied together, then it may be worth using git submodules and removing that logic from CMake completely.

mujx commented 6 years ago

Automatically updating sources is typically not recommended for release builds of most projects

By 'synced with upstream' I meant to check that the local clone is checked out to the commit defined in cmake/MatrixStructs.cmake. So it's not fetching the latest master every time.

Slabity commented 6 years ago

@mujx

I'm not quite sure how cmake works, but that seems a bit unorthodox. I'd like to be able to build each dependency separately.

I've made a derivation that builds matrix-structs. Is there a place I can put the output libmatrix_structs.a file to stop nheko from trying to build it itself?

mujx commented 6 years ago

nheko/.third-party/matrix_structs/lib/libmatrix_structs.a is the output path.

Slabity commented 6 years ago

I've finished the Nix derivation. It requires a small patch that stops CMake from trying to check upstream for matrix-structs and lmdbxx repos, but it works pretty well outside of that:

{ stdenv, lib, fetchFromGitHub, qtbase, qttranslations, qtmultimedia, lmdb, cmake, git }:

let
  matrix-structs = fetchFromGitHub {
    owner  = "mujx";
    repo   = "matrix-structs";
    rev    = "850100c0ac2b5a04720b2a1f09270749bf99f7dd";
    sha256 = "1r9n1744zbyqhqs0i9jzsi9pl922vl9a33r5ighpkmhz1dm0ryph";
    fetchSubmodules = true;
  };

  lmdbxx = fetchFromGitHub {
    owner = "bendiken";
    repo = "lmdbxx";
    rev = "0b43ca87d8cfabba392dfe884eb1edb83874de02";
    sha256 = "1whsc5cybf9rmgyaj6qjji03fv5jbgcgygp956s3835b9f9cjg1n";
    fetchSubmodules = true;
  };
in
stdenv.mkDerivation rec {
  name = "nheko-${version}";
  version = "0.2.1";

  src = fetchFromGitHub {
    owner  = "mujx";
    repo   = "nheko";
    rev    = "v${version}";
    sha256 = "18xbmg9xmx2jsz7q5gqlh6qwd9fnw4zmsyimmf8rf40z6vm5fqb0";
    fetchSubmodules = true;
  };

  buildInputs = [ qtbase qttranslations qtmultimedia lmdb matrix-structs ];

  nativeBuildInputs = [ cmake git ];

  enableParallelBuilding = true;

  patches = [ ./no-download.patch ];

  preConfigure = ''
    mkdir -p .third-party
    cp -r ${matrix-structs} .third-party/matrix_structs
    cp -r ${lmdbxx} .third-party/lmdbxx
    chmod -R +w .third-party/matrix_structs
    chmod -R +w .third-party/lmdbxx
  '';

  installPhase = "install -Dm755 nheko $out/bin/nheko";

  meta = with lib; {
    description = "Desktop client for the Matrix protocol";
    homepage    = https://github.com/mujx/nheko;
    license     = licenses.gpl3;
    inherit (qtbase.meta) platforms;
    inherit version;
  };
}

And here is the no-download.patch, which just tells CMake to run the true program as its download step:

diff --git a/cmake/LMDBXX.cmake b/cmake/LMDBXX.cmake
index 502d6b6..7fcdd74 100644
--- a/cmake/LMDBXX.cmake
+++ b/cmake/LMDBXX.cmake
@@ -12,8 +12,7 @@ set(LMDBXX_INCLUDE_DIRS ${LMDBXX_ROOT})
 ExternalProject_Add(
   lmdbxx

-  GIT_REPOSITORY https://github.com/bendiken/lmdbxx
-  GIT_TAG 0b43ca87d8cfabba392dfe884eb1edb83874de02
+  DOWNLOAD_COMMAND true

   BUILD_IN_SOURCE 1
   SOURCE_DIR ${LMDBXX_ROOT}
diff --git a/cmake/MatrixStructs.cmake b/cmake/MatrixStructs.cmake
index 61cf619..37379e1 100644
--- a/cmake/MatrixStructs.cmake
+++ b/cmake/MatrixStructs.cmake
@@ -22,8 +22,7 @@ endif()
 ExternalProject_Add(
   MatrixStructs

-  GIT_REPOSITORY https://github.com/mujx/matrix-structs
-  GIT_TAG 850100c0ac2b5a04720b2a1f09270749bf99f7dd
+  DOWNLOAD_COMMAND true

   BUILD_IN_SOURCE 1
   SOURCE_DIR ${MATRIX_STRUCTS_ROOT}

I'll see about getting this into the nixpkgs repo.

mujx commented 6 years ago

@Slabity Thanks for working on it! I have updated the repo to make download optional. So you'll only have to provide the location of the matrix_structs.{so, a} library and the include directory.

-DMATRIX_STRUCTS_ROOT # where the matrix_structs library is
-DMATRIX_STRUCTS_INCLUDE_DIR
-DLMDBXX_INCLUDE_DIR
-DTWEENY_INCLUDE_DIR
Slabity commented 6 years ago

@mujx Awesome. Is it possible to do the same thing for lmdbxx?

mujx commented 6 years ago

It's already done. You just need to use -DLMDBXX_INCLUDE_DIR to point cmake to the directory that contains the header.

Cogitri commented 6 years ago

I guess alternatively you could use git submodules to solve this a little more elegantly (IMO)

mujx commented 6 years ago

@Slabity Is this still relevant or I should close the issue? Nix has been removed from the repo because it was unmaintained.

Slabity commented 6 years ago

It should be fine to be closed.