Skip to content

Nix portability improvements #13005

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+34 −5
Open

Nix portability improvements #13005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8e426df
build(nix): Use a split derivation
hacker1024 Apr 18, 2025
b507bb4
build(nix): Move shaderc to nativeBuildInputs
hacker1024 Apr 18, 2025
bf1f1b3
build(nix): Propagate BLAS appropriately
hacker1024 Apr 18, 2025
b1f01c7
build(nix): Allow using OpenMP
hacker1024 Apr 18, 2025
60092b4
cmake: Only link stdc++fs on GCC
hacker1024 Apr 18, 2025
70a689e
build(nix): Add hacker1024 as a Nix package maintainer
hacker1024 Apr 18, 2025
4bffb15
build(nix): Allow disabling common, tests, examples, server
hacker1024 Apr 18, 2025
ff1ac5c
build(nix): Don't set CMAKE_SKIP_BUILD_RPATH
hacker1024 Apr 18, 2025
4a4a753
build(nix): Enable tests
hacker1024 Apr 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions .devops/nix/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
cudaPackages,
autoAddDriverRunpath,
darwin,
llvmPackages,
rocmPackages,
vulkan-headers,
vulkan-loader,
Expand All @@ -34,6 +35,10 @@
rocmGpuTargets ? builtins.concatStringsSep ";" rocmPackages.clr.gpuTargets,
enableCurl ? true,
useVulkan ? false,
useOpenMP ? false,
buildCommon ? true,
buildExamples ? buildCommon,
buildServer ? buildExamples,
llamaVersion ? "0.0.0", # Arbitrary version, substituted by the flake

# It's necessary to consistently use backendStdenv when building with CUDA support,
Expand All @@ -49,6 +54,7 @@ let
cmakeFeature
optionals
strings
assertMsg
;

stdenv = throw "Use effectiveStdenv instead";
Expand Down Expand Up @@ -96,17 +102,25 @@ let
rocblas
];

vulkanNativeBuildInputs = [
shaderc
];

vulkanBuildInputs = [
vulkan-headers
vulkan-loader
shaderc
];
in

assert assertMsg (buildExamples -> buildCommon) "buildCommon must be true when buildExamples is true";
assert assertMsg (buildServer -> buildExamples) "buildExamples must be true when buildServer is true";

effectiveStdenv.mkDerivation (finalAttrs: {
pname = "llama-cpp${pnameSuffix}";
version = llamaVersion;

outputs = [ "out" "lib" "dev" ];

# Note: none of the files discarded here are visible in the sandbox or
# affect the output hash. This also means they can be modified without
# triggering a rebuild.
Expand Down Expand Up @@ -145,14 +159,14 @@ effectiveStdenv.mkDerivation (finalAttrs: {
[
cmake
ninja
pkg-config
git
]
++ optionals useCuda [
cudaPackages.cuda_nvcc

autoAddDriverRunpath
]
++ optionals useVulkan vulkanNativeBuildInputs
++ optionals (effectiveStdenv.hostPlatform.isGnu && enableStatic) [ glibc.static ]
++ optionals (effectiveStdenv.isDarwin && useMetalKit && precompileMetalShaders) [ xcrunHost ];

Expand All @@ -165,19 +179,30 @@ effectiveStdenv.mkDerivation (finalAttrs: {
++ optionals useVulkan vulkanBuildInputs
++ optionals enableCurl [ curl ];

propagatedNativeBuildInputs = [ pkg-config ];

propagatedBuildInputs =
optionals useBlas [ blas ]
++ optionals useOpenMP [ llvmPackages.openmp ];

cmakeFlags =
[
(cmakeBool "LLAMA_BUILD_SERVER" true)
(cmakeBool "BUILD_SHARED_LIBS" (!enableStatic))
(cmakeBool "CMAKE_SKIP_BUILD_RPATH" true)
(cmakeBool "LLAMA_CURL" enableCurl)
(cmakeBool "GGML_NATIVE" false)
(cmakeBool "GGML_BLAS" useBlas)
(cmakeBool "GGML_CUDA" useCuda)
(cmakeBool "GGML_HIP" useRocm)
(cmakeBool "GGML_METAL" useMetalKit)
(cmakeBool "GGML_VULKAN" useVulkan)
(cmakeBool "GGML_OPENMP" useOpenMP)
(cmakeBool "GGML_STATIC" enableStatic)
(cmakeBool "LLAMA_BUILD_COMMON" (buildCommon || finalAttrs.doCheck or false))
(cmakeBool "LLAMA_BUILD_TESTS" finalAttrs.doCheck or false)
(cmakeBool "LLAMA_BUILD_EXAMPLES" buildExamples)
(cmakeBool "LLAMA_BUILD_SERVER" buildServer)
(cmakeBool "BLA_PREFER_PKGCONFIG" true)
(cmakeFeature "CMAKE_CTEST_ARGUMENTS" "--exclude-regex;test-eval-callback") # Uses Internet
]
++ optionals useCuda [
(
Expand Down Expand Up @@ -209,6 +234,8 @@ effectiveStdenv.mkDerivation (finalAttrs: {
cp $src/include/llama.h $out/include/
'';

doCheck = true;

meta = {
# Configurations we don't want even the CI to evaluate. Results in the
# "unsupported platform" messages. This is mostly a no-op, because
Expand Down Expand Up @@ -239,6 +266,7 @@ effectiveStdenv.mkDerivation (finalAttrs: {
maintainers = with lib.maintainers; [
philiptaron
SomeoneSerge
hacker1024
];

# Extend `badPlatforms` instead
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ poetry.toml

# Nix
/result
/result-*

# Test binaries
/tests/test-backend-ops
Expand Down
2 changes: 1 addition & 1 deletion ggml/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ add_library(ggml

target_link_libraries(ggml PUBLIC ggml-base)

if (CMAKE_SYSTEM_NAME MATCHES "Linux")
Copy link
Collaborator

@SomeoneSerge SomeoneSerge Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should there be a special branch for https://releases.llvm.org/10.0.0/projects/libcxx/docs/UsingLibcxx.html#id2? Note that clang does not necessarily mean libc++, and gcc does not necessarily mean libstdc++ (e.g. in Nixpkgs all stdenvs use the same stdlib determined by the platform)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also idk about libdl handling

Copy link
Author

Choose a reason for hiding this comment

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

Potentially. I guess it depends on how many systems we actually want to support - the original change was required for GCC on RHEL 8, so there's a reason for this branch for that use case.

I'm not sure if it's worth preemptively handling ancient LLVM versions as well unless either someone explicitly requires it or we're actively testing it...

Copy link
Collaborator

Choose a reason for hiding this comment

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

GCC on RHEL 8,

Let's then add a comment explaining the "why"s

if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
target_link_libraries(ggml PRIVATE dl stdc++fs)
endif()

Expand Down
Loading