fix: pre-built llama.cpp install fails with "Found 0 of 2" on first-run setup#320
Merged
fix: pre-built llama.cpp install fails with "Found 0 of 2" on first-run setup#320
Conversation
…uard Modern llama.cpp GitHub release archives use the structure llama-b<tag>/<filename> rather than any build/bin/ sub-directory. The old filter caused every tar entry to be skipped, leaving extracted_binaries=0 and triggering: Failed to extract all required binaries. Found 0 of 2 Replace the string-contains guard with a path-components length check: keep only entries that are exactly one level deep inside the archive top- level directory. This is robust to future tag-name changes. Fixes #319
…xpected paths All three download_prebuilt_binaries* functions were computing: bin_dir = data_root() / "bin" but llama_server_path() and llama_cli_path() (gglib-core) expect binaries at: resource_root() / ".llama" / "bin" / <binary> Since resource_root() == data_root() for standalone installs, the post- extraction verification step would always fail with "binaries not found". Update all three functions to use .llama/bin as the extraction target, matching the authoritative path definitions in gglib-core. Part of #319
…nfig_path() save_prebuilt_config was writing llama-config.json directly to data_root(), but llama_config_path() (gglib-core) resolves it as: resource_root() / .llama / llama-config.json Align the write path so the config is discoverable after installation. Part of #319
Build a synthetic archive with the llama-b<tag>/<file> structure that GitHub releases actually use, call extract_binaries_tar_gz, and assert both required binaries and a shared library are extracted to bin_dir. This would have caught the build/bin/ filter regression covered in the preceding commits. Part of #319
Previously the cleanup code ran only on the success path: extract_binaries(&zip_path, &bin_dir)?; // early return on error ... let _ = fs::remove_file(&zip_path); // never reached on failure let _ = fs::remove_dir(&download_dir); If extraction (or the optional Windows CUDA download) returned an error the downloaded archive and the downloads/ directory were left permanently on disk. Restructure all three download_prebuilt_binaries* functions to capture the extraction result in an async block, then unconditionally call fs::remove_dir_all(&download_dir) before propagating the error. remove_dir_all replaces the previous remove_file + remove_dir pair, so leftover files from a partial CUDA download don't prevent the directory from being removed. Part of #319
…ll [skip ci] The prebuilt binary installer creates a downloads/ directory at the repo/ data root to stage the archive before extraction, then removes it. Add it to .gitignore so it can never be accidentally committed if cleanup fails. Part of #319
…entries Modern llama.cpp macOS release archives contain versioned dylib symlink entries (e.g. libggml.dylib -> libggml-metal.0.dylib). After entry.unpack() creates the symlink in bin_dir, the chmod block called fs::metadata which follows the symlink (stat, not lstat). If the target had not yet been extracted, stat returned ENOENT -- propagated bare as: Failed to install llama.cpp: No such file or directory (os error 2) Fix both extract_binaries_tar_gz and extract_binaries_zip: - Replace fs::metadata with fs::symlink_metadata (lstat) so symlink targets are never traversed - Skip set_permissions entirely for symlink entries -- chmod on a symlink has no effect on macOS/Linux - Add .with_context() to all previously bare fs:: error paths Also fix a stale doc comment on extract_binaries that still referred to the old build/bin/ archive layout, and add .with_context() to the fs::write in save_prebuilt_config. Fixes #319
…tion test Extend the existing archive layout test to include a symlink entry whose target is not present in the archive (a dangling symlink, as seen in real macOS llama.cpp releases with versioned .dylib aliases). Before the symlink_metadata fix, this case triggered the bare ENOENT that surfaced as "Failed to install llama.cpp: No such file or directory (os error 2)". The test now asserts: - Both required binaries and a shared library are extracted - The symlink entry itself is created in bin_dir (verifiable via symlink_metadata) - The whole operation completes without error despite the dangling target Part of #319
Owner
Author
|
Round 2 fix pushed (commits After the round-1 fixes resolved the Root cause: Modern macOS llama.cpp release archives contain versioned dylib symlink entries (e.g. Fix:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #319
What was broken
When a user ran
make setup(without building llama.cpp from source) and then launched the GUI, the first-run setup wizard would download the pre-built archive but immediately fail with:Three bugs in
crates/gglib-runtime/src/llama/download/mod.rscaused this cascade.Commits
1.
fix(download): replace stale build/bin/ filter with component-count guardThe extraction loop for
.tar.gzarchives filtered entries with:This path never existed in llama.cpp release archives. The actual structure is:
Every entry was rejected →
extracted_binariesstayed0→ the error fired.Fix: replaced with a path-components length check — skip entries that are not exactly one level deep inside the archive's top-level directory. Resilient to future tag name changes.
2.
fix(download): extract pre-built binaries into .llama/bin/ to match expected pathsAll three
download_prebuilt_binaries*functions used:But
llama_server_path()/llama_cli_path()(ingglib-core) expect binaries atresource_root()/.llama/bin/. The binaries would have landed in the wrong directory and the post-install verification step would have failed:Fix: changed to
gglib_dir.join(".llama").join("bin")in all three functions.3.
fix(download): write llama-config.json into .llama/ to match llama_config_path()save_prebuilt_configwrote the config todata_root()/llama-config.jsonbutllama_config_path()expects it atresource_root()/.llama/llama-config.json.Fix: changed the write path accordingly.
4.
test(download): add unit test for modern llama.cpp tar.gz archive layoutAdded a unit test that constructs a synthetic tar.gz archive matching the actual
llama-b<tag>/<filename>release layout, callsextract_binaries_tar_gz, and asserts both required binaries and a shared library are properly extracted. This test would have caught the regression in commit 1.Testing
New test:
llama::download::tests::test_extract_binaries_tar_gz_modern_layout✓