feat: better handling of server installation path#528
feat: better handling of server installation path#528acristoffers wants to merge 6 commits intoharrisoncramer:developfrom
Conversation
acristoffers
force-pushed
the
feature/nix-and-configurable-server-path
branch
from
ae32ab8 to
8756cd4
Compare
acristoffers
force-pushed
the
feature/nix-and-configurable-server-path
branch
2 times, most recently
from
4cad31e to
bbcb640
Compare
|
I forgot to add a shell to the nix flake, so I added it and force pushed. It allows developers to have a shell with all dependencies installed with one command. To test the environment I tried running the tests and run into a problem: the test expects a date string back in English, but my locale is set to Portuguese, so the strings didn't match. I modified the |
acristoffers
force-pushed
the
feature/nix-and-configurable-server-path
branch
from
bbcb640 to
829cefd
Compare
flake.nix
Outdated
| pname = "gitlab.nvim-server"; | ||
| version = "git"; | ||
| src = ./.; | ||
| vendorHash = "sha256-OLAKTdzqynBDHqWV5RzIpfc3xZDm6uYyLD4rxbh0DMg="; |
There was a problem hiding this comment.
Claude came up with this:
2. Hardcoded vendorHash (line 15)
- The hash sha256-OLAKTdzqynBDHqWV5RzIpfc3xZDm6uYyLD4rxbh0DMg= will break when Go
dependencies change
Does that mean that whenever the Go dependencies change this flake.nix file will have to be updated to keep working? How is the vendorHash created? Who will be responsible for updating it?
There was a problem hiding this comment.
Yes, unfortunately. But only when there are dependencies changes, not code changes, so not so often. To generate it, I edit the nix file and remove the hash (it becomes an empty string), then I run nix build and it will fail and say what hash is, which I then copy into the file.
I can keep it up-to-date, as I'll quickly see the problem (this is in my NeoVim flake, and I rebuild my flake every day).
There was a problem hiding this comment.
I personally prefer to update the hashes manually, but there is a solution that allows a GitHub action to do it. Since I guess that you as project would prefer it to be automated, I pushed another fixup! commit that makes use of that. It introduces a TOML file that holds dependencies hashes and a GitHub action that updates this file and opens a PR with the changes.
There was a problem hiding this comment.
I leave this up to Harrison, if he wants to have this in the plugin.
| local Path = require("plenary.path") | ||
| local src = Path:new(state.settings.root_path .. u.path_separator .. "cmd" .. u.path_separator .. "config") | ||
| local dest = Path:new(bin_folder .. u.path_separator .. "config") | ||
| src:copy({ destination = dest, recursive = true, override = true }) |
There was a problem hiding this comment.
This is copied because of the code in cmd/app/emoji.go, right? As far as I can see, this code does nothing. The emoji map is only needed for the lua code, so unless @harrisoncramer decides otherwise, the attachEmojis function and it's usage can be deleted. Then we won't need to copy the config folder.
Even if you don't delete attachEmojis function, an alternative to copying the config would be to pass the path to emojis.json as part of the PluginOptions when the server is started.
There was a problem hiding this comment.
I don't want to make design decisions, so I'll wait for @harrisoncramer to decide.
|
I pushed changes as a |
|
@acristoffers could you please rebase? The develop branch has been updated. |
The plugin should not assume its folder is writable. Instead, try to search for a writable folder by checking the data folder and the runtime paths. Also allows the user to give the path to the server's executable intead of always compiling it. All of this makes the plugin friendlier to restricted/read-only environments, like nix.
acristoffers
force-pushed
the
feature/nix-and-configurable-server-path
branch
from
a1168ed to
63e5b95
Compare
|
Hi @acristoffers. I've just realized, you also should update the new structure of the settings in annotations.lua. |
Done |
3 participants
Closes #493
I also use Nix to configure my NeoVim installation. The modifications in this MR make it possible to use the plugin this way, or with another form of distribution that does not provide git or has a read-only plugin folder.
This is achieved by:
vim.fn.stdpath("data"). This is the correct place to put it in. But anyway, don't assume it's writable, and try it and all folders in the runtime to find one that we can write to, and put it there. The data folder is most probably going to be writable, but this way we can at least have more options before failing. This is already done by NeoVim itself when you try to install a spell dictionary, for example, or a tree-sitter grammar, so it is a safe way to handle this that aligns with existing practices.To this end I changed the configuration structure to have a
serverkey with abinarysub-key, and moved theportoption inside it, for coherence.The init script was changed to set the configuration before building, as we now need the configuration during the build.
The build function was, of course, the most affected by the changes.
A
flake.nixis provided that make it easy to use the plugin with nix. It offers the plugin and server, so the user can decide if they want to let the plugin build the server as usual or if they want to let nix manage the build and just point to it in their configuration.The server assumes the path of the emoji configuration file. I don't understand why the file is provided like that, could it not be embedded in the binary? Maybe its path could be passed as configuration by Lua when launching it? I decided to not change it though, and just make sure the config folder is where the server expects it to be.