Skip to content
Merged
Changes from 1 commit
Commits
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
12 changes: 9 additions & 3 deletions unsloth/save.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@
"q3_k_xs": "3-bit extra small quantization",
}

def has_curl():
return shutil.which("curl") is not None

CURL_FLAG = "-DLLAMA_CURL=ON" if has_curl() else "-DLLAMA_CURL=OFF"

def print_quantization_methods():
for key, value in ALLOWED_QUANTS.items():
Expand Down Expand Up @@ -879,8 +883,9 @@ def install_llama_cpp_make_non_blocking():
# Uses new CMAKE
n_jobs = max(int(psutil.cpu_count()), 1) # Use less CPUs since 1.5x faster
check = os.system(
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF -DLLAMA_CURL=ON"
f"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}"
)

if check != 0:
raise RuntimeError(
f"*** Unsloth: Failed compiling llama.cpp using os.system(...) with error {check}. Please report this ASAP!"
Expand Down Expand Up @@ -991,11 +996,12 @@ def install_llama_cpp_old(version = -10):
if try_execute(commands) == "CMAKE":
# Instead use CMAKE
commands = [
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF -DLLAMA_CURL=ON",
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

This string should be an f-string to correctly interpolate the CURL_FLAG variable. As it is, the literal string {CURL_FLAG} will be passed to the cmake command, which will cause a build failure.

Suggested change
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",
f"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Format CURL_FLAG before passing to cmake

When the make build fails and try_execute falls back to CMake, this command passes the literal string {CURL_FLAG} to cmake because it isn’t an f-string. That means CMake receives an unrecognized argument instead of -DLLAMA_CURL=ON/OFF, so the fallback build can fail regardless of curl availability. This regression only shows up in the CMake fallback path.

Useful? React with 👍 / 👎.

f"cmake --build llama.cpp/build --config Release -j{psutil.cpu_count()*2} --clean-first --target {' '.join(LLAMA_CPP_TARGETS)}",
"cp llama.cpp/build/bin/llama-* llama.cpp",
"rm -rf llama.cpp/build",
]

try_execute(commands)

# Check if successful
Expand Down Expand Up @@ -1037,7 +1043,7 @@ def install_llama_cpp_blocking(use_cuda = False):
if try_execute(commands) == "CMAKE":
# Instead use CMAKE
commands = [
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF -DLLAMA_CURL=ON",
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Similar to a previous part of the code, this string needs to be an f-string to interpolate the CURL_FLAG variable. Without the f prefix, the command will receive the literal string {CURL_FLAG}, causing the build to fail.

Suggested change
"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",
f"cmake llama.cpp -B llama.cpp/build -DBUILD_SHARED_LIBS=OFF -DGGML_CUDA=OFF {CURL_FLAG}",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Format CURL_FLAG before passing to cmake

In the blocking install path’s CMake fallback, {CURL_FLAG} is not interpolated (string is missing the f prefix). As a result, the literal placeholder is passed to CMake and the fallback build can fail even when it’s supposed to disable curl. This only affects environments where the make build fails and the CMake path is used.

Useful? React with 👍 / 👎.

f"cmake --build llama.cpp/build --config Release -j{psutil.cpu_count()*2} --clean-first --target {' '.join(LLAMA_CPP_TARGETS)}",
"cp llama.cpp/build/bin/llama-* llama.cpp",
"rm -rf llama.cpp/build",
Expand Down