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

clickhouse: 20.5.2.7 -> 20.11.4.13 #105224

Closed
wants to merge 1 commit into from

Conversation

schneefux
Copy link
Contributor

Motivation for this change

Updated clickhouse to the latest version.

Building clickhouse takes multiple hours, my machine is not powerful enough so I could not test this change ☹️
Can someone help here?

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 nixpkgs-review --run "nixpkgs-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.

@RaghavSood
Copy link
Member

It does fail on OfBorg, so likely it would fail when I build it too.

I'll see if I can update it in the coming days, I recall it being somewhat painful to update the last time as well, but I do have some beefy machines that can at least build it, so should be possible

@lukegb
Copy link
Contributor

lukegb commented Nov 28, 2020

We should probably switch clickhouse to using UNBUNDLED mode instead so we don't have to rebuild e.g. LLVM as part of the clickhouse build...

@lukegb
Copy link
Contributor

lukegb commented Nov 28, 2020

I hit

../utils/db-generator/query_db_generator.cpp: In function 'int main(int, const char**)':
../utils/db-generator/query_db_generator.cpp:1295:20: error: ignoring return value of 'FILE* freopen(const char*, const char*, FILE*)', declared with attribute warn_unused_result [-Werror=unused-result]
 1295 |             freopen(vm["input"].as<std::string>().c_str(), "r", stdin);
      |             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../utils/db-generator/query_db_generator.cpp:1297:20: error: ignoring return value of 'FILE* freopen(const char*, const char*, FILE*)', declared with attribute warn_unused_result [-Werror=unused-result]
 1297 |             freopen(vm["output"].as<std::string>().c_str(), "w", stdout);
      |             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

while building this.

@lukegb
Copy link
Contributor

lukegb commented Nov 28, 2020

#105286 makes Clickhouse use the system LLVM instead of compiling its own, which at least seemed to make it evaluate on ofBorg.

@RaghavSood
Copy link
Member

There is a patch for the build issue in: ClickHouse/ClickHouse#16859

Until there is a new upstream version, we could apply that patch ourselves

@lukegb
Copy link
Contributor

lukegb commented Nov 29, 2020

With

diff --git a/pkgs/servers/clickhouse/default.nix b/pkgs/servers/clickhouse/default.nix
index b519d1b4315..2cad2150153 100644
--- a/pkgs/servers/clickhouse/default.nix
+++ b/pkgs/servers/clickhouse/default.nix
@@ -1,4 +1,4 @@
-{ stdenv, fetchFromGitHub, cmake, libtool, lldClang, ninja
+{ stdenv, fetchFromGitHub, fetchpatch, cmake, libtool, lldClang, ninja
 , boost, brotli, capnproto, cctz, clang-unwrapped, double-conversion
 , icu, jemalloc, libcpuid, libxml2, lld, llvm, lz4, libmysqlclient, openssl, perl
 , poco, protobuf, python3, rapidjson, re2, rdkafka, readline, sparsehash, unixODBC
@@ -17,6 +17,13 @@ stdenv.mkDerivation rec {
     sha256 = "0c87k0xqwj9sc3xy2f3ngfszgjiz4rzd787bdg6fxp94w1adjhny";
   };
 
+  patches = [
+    (fetchpatch {
+      url = "https://github.com/ClickHouse/ClickHouse/commit/e31753b4db7aa0a72a85757dc11fc403962e30db.patch";
+      sha256 = "12ax02dh9y9k8smkj6v50yfr46iprscbrvd4bb9vfbx8xqgw7grb";
+    })
+  ];
+
   nativeBuildInputs = [ cmake libtool lldClang.bintools ninja ];
   buildInputs = [
     boost brotli capnproto cctz clang-unwrapped double-conversion

This builds for me:

[lukegb@whitby:~/nixpkgs-clickhouse]$ ./result/bin/clickhouse server --version
ClickHouse server version 20.11.4.13.

[lukegb@whitby:~/nixpkgs-clickhouse]$ ./result/bin/clickhouse client --version
ClickHouse client version 20.11.4.13.

@lukegb
Copy link
Contributor

lukegb commented Nov 29, 2020

@schneefux You should be able to git fetch https://github.com/NixOS/nixpkgs.git eb960b541dfc9a18a4bb4fd7b8a09757e97d14fe && git cherry-pick eb960b541dfc9a18a4bb4fd7b8a09757e97d14fe.

You'll also want to rebase on master, since it now has my change to use system LLVM instead of the bundled one, which should make it build (much) faster. I'll rebuild once you've pushed up a new branch :)

@lukegb
Copy link
Contributor

lukegb commented Dec 1, 2020

Result of nixpkgs-review pr 105224 run on x86_64-linux 1

1 package failed to build:
  • clickhouse

@schneefux
Copy link
Contributor Author

It still does not build within an hour on my server, so I'm afraid I have to give up and hand it over to @RaghavSood 🙈
Thank you for your help!

@lukegb
Copy link
Contributor

lukegb commented Dec 2, 2020

Result of nixpkgs-review pr 105224 run on x86_64-linux 1

1 package failed to build:
  • clickhouse

@lukegb
Copy link
Contributor

lukegb commented Dec 2, 2020

  
  CMake Error at cmake/find/llvm.cmake:50 (message):
    Unsupported LLVM configuration, cannot enable LLVM
  Call Stack (most recent call first):
    CMakeLists.txt:440 (include)

hrm. Maybe I broke it.

@lukegb
Copy link
Contributor

lukegb commented Dec 2, 2020

Clickhouse now needs LLVM10: you'll need to update pkgs/top-level/all-packages.nix to change the clickhouse customization to use llvmPackages10 instead of llvmPackages9, but that should be sufficient to get it to build - see the OfBorg failure: https://logs.nix.ci/?key=nixos/nixpkgs.105224&attempt_id=8b71ccee-d166-4ed4-a69c-cdb990c038f1

@RaghavSood
Copy link
Member

Continuing in #105968

@RaghavSood RaghavSood closed this Dec 5, 2020
@schneefux schneefux deleted the pkg/clickhouse branch December 6, 2020 15:39
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

3 participants