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

fix(download): correctly handle download = false and implementationn = "rust" #1334

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

stefanboca
Copy link
Collaborator

Previously, this set of options would result in the lua implementation being used. The new flow should be more linear and easy to follow, with more descriptive and specialized error messages.

…n = "rust"`

Previously, this set of options would result in the lua implementation
being used. The new flow should be more linear and easy to follow.
@Saghen Saghen merged commit 2c8e4c7 into main Feb 27, 2025
6 checks passed
@Saghen
Copy link
Owner

Saghen commented Feb 27, 2025

Much cleaner, thank you!

@Saghen Saghen deleted the sb/push-ukzmwyquktwv branch February 27, 2025 15:59
@konradmalik
Copy link
Contributor

konradmalik commented Feb 28, 2025

hmm, after this PR I get this warning:

[blink.cmp]: Falling back to Lua implementation due to error while downloading pre-built binary:
...es/start/blink-cmp/lua/blink/cmp/fuzzy/download/init.lua:73: No fuzzy matching library found, but downloading from github is disabled.
Either run `cargo build --release` via your package manager, or set `fuzzy.prebuilt_binaries.download = true` in config.
See the docs for more info.
[blink.cmp]: Set `fuzzy.implementation = 'prefer_rust' | 'lua'` to suppress this warning.

But I'm using nix, and the binary is provided by just using blink-cmp plugin from this repo's flake.
In my settings I have download=false because it won't work on Nix. Also I'm using the version from main, not a tag.

Before lua implementation all worked fine, so I suspect that it still does but now I get this warning.
But then it's not clear how I can ensure that it indeed is using Rust implementation and not Lua if I disable this warning 🤔 I'd also like to know in the future if it breaks.
Any ideas on how to solve this?

PS setting implementation = "rust" results in this error:

...es/start/blink-cmp/lua/blink/cmp/fuzzy/download/init.lua:73: No fuzzy matching library found, but downloading from github is disabled.
Either run `cargo build --release` via your package manager, or set `fuzzy.prebuilt_binaries.download = true` in config.
See the docs for more info.

Saghen added a commit that referenced this pull request Feb 28, 2025
@Saghen
Copy link
Owner

Saghen commented Feb 28, 2025

The latest change should bypass the prebuilt binaries entirely when on nix (no need to set download false). Lmk if that works for you

@debugloop
Copy link

Something is still of with the v0.13 line and also current main. I'm getting

.../nixpkgs/blink-cmp/lua/blink/cmp/fuzzy/download/init.lua:90: No fuzzy matching library found, but can't download from github due to not being on a git tag and no fuzzy.prebuilt_binaries.force_version is set.
Either run cargo build --release via your package manager, switch to a git tag, or set fuzzy.prebuilt_binaries.force_version in config.
See the docs for more info.

I think whatever the issue around downloads was, there's still something off about the path resolution of the .so file, even if it's included with the flake. Any way to provide a path statically? I'll try to find you some more info to work with.

@konradmalik
Copy link
Contributor

yeah, still the same issue, but I'm pretty sure the so file is properly loaded.
I can force the implementation into be rust by a hack (require + set_implementation).
I will have a PR shortly.

@konradmalik
Copy link
Contributor

watch out, this is a big one 😄 #1356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants