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.

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

nix: static build #5814

Merged
merged 11 commits into from
Mar 5, 2024
Merged

nix: static build #5814

Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b4ac6f0
merge
hutli Feb 15, 2024
51ec91b
using default LLAMA_STATIC cmake
hutli Feb 23, 2024
498e998
buildStatic variable to toggle static builds
hutli Mar 1, 2024
bf11052
buildStatic variable to toggle static builds
hutli Mar 1, 2024
db22ce3
using host platform isStatic as default value for "enableStatic"
hutli Mar 2, 2024
48830d7
"buildStatic" -> "enableStatic"
hutli Mar 2, 2024
3cfb0f0
"buildStatic" -> "enableStatic"
hutli Mar 2, 2024
d12f239
using enableStatic to determine glibc dependency
hutli Mar 4, 2024
bc51e28
using correct syntax for effectiveStdenv default
hutli Mar 4, 2024
7a2ca8b
removed dupulicate declaration of effectiveStdenv
hutli Mar 4, 2024
4aa00f4
nix: update comment on effectiveStdenv
philiptaron Mar 4, 2024
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
9 changes: 8 additions & 1 deletion .devops/nix/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
lib,
glibc,
config,
stdenv,
mkShell,
Expand Down Expand Up @@ -30,6 +31,8 @@
useRocm ? config.rocmSupport,
useVulkan ? false,
llamaVersion ? "0.0.0", # Arbitrary version, substituted by the flake
effectiveStdenv = if useCuda then cudaPackages.backendStdenv else stdenv,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment from line 47 needs to move up here, and the effectiveStdenv below needs to be removed, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be implemented and working now 👍

enableStatic ? effectiveStdenv.hostPlatform.isStatic
}@inputs:

let
Expand Down Expand Up @@ -167,6 +170,9 @@ effectiveStdenv.mkDerivation (
# TODO: Replace with autoAddDriverRunpath
# once https://github.com/NixOS/nixpkgs/pull/275241 has been merged
cudaPackages.autoAddOpenGLRunpathHook
]
++ optionals buildStatic [
glibc.static
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
++ optionals buildStatic [
glibc.static
++ optionals (stdenv.hostPlatform.isGnu && stdenv.hostPlatform.isStatic) [
glibc.static

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't buildStatic (static if renamed) take precedence?

Copy link
Contributor Author

@hutli hutli Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think @SomeoneSerge is correct - or more specifically enableStatic going with @philiptaron suggestion. I have no idea how to do this in this smart GitHub interface though.

Copy link
Collaborator

@philiptaron philiptaron Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The thing is that there are other static builds using other libc implementations -- musl being a great example -- that don't require adding glibc static edition. I agree that the test should include enableStatic rather than my proposal of stdenv.hostPlatform.isStatic.

The other approach, since you're hosting the stdenv, is that you add in glib.static in your hosting flake. That would work too, I think, and likely be closer to "right all the time."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. I agree that adding glib.static to the hosting flake would work but the problem I guess is then that anybody wanting a static build would have to know that they need to do so. It feels a bit more explicit to have it here too. I've implemented these changes in d12f239 and fixed some small issues so the branch now passes all tests on my end.

];

buildInputs =
Expand All @@ -181,7 +187,7 @@ effectiveStdenv.mkDerivation (
[
(cmakeBool "LLAMA_NATIVE" false)
(cmakeBool "LLAMA_BUILD_SERVER" true)
(cmakeBool "BUILD_SHARED_LIBS" true)
(cmakeBool "BUILD_SHARED_LIBS" (!enableStatic))
(cmakeBool "CMAKE_SKIP_BUILD_RPATH" true)
(cmakeBool "LLAMA_BLAS" useBlas)
(cmakeBool "LLAMA_CLBLAST" useOpenCL)
Expand All @@ -190,6 +196,7 @@ effectiveStdenv.mkDerivation (
(cmakeBool "LLAMA_METAL" useMetalKit)
(cmakeBool "LLAMA_MPI" useMpi)
(cmakeBool "LLAMA_VULKAN" useVulkan)
(cmakeBool "LLAMA_STATIC" enableStatic)
]
++ optionals useCuda [
(
Expand Down