Parameter extra_volumes for mounting additional volumes on the container

[B3RR10]

As proposed by @WhyNotHugo in the issue lspcontainers/dockerfiles#55, it could be useful to be able to mount additional directories to the containers.

This could be a start on solving the "customization" on the go-lsp case, and support global caches, which is very common in several languages such as rust, C# (omnisharp/nuget), ruby (solargraph/bundle), etc.

[WhyNotHugo]

Haven't had a chance to test this, but the changes LGTM.

[erikreinert]

This looks good - can you possibly resolve the conflicts on this and I'll merge it? Thank you!

[B3RR10]

@erikreinert Sorry that it took so long... But the conflicts were resolved

[jgero]

Would it be a good idea to maybe add named volumes as default values for this? I am doing this with my own setup at the moment. This enables persisting those directories between containers and even between projects even. I think this makes sense because these files are of no use mounted on the host anyways because the compilers and language servers that would use those are not installed on the host anyways when you run these tools in a container.

[WhyNotHugo]

An attribute for each LSP with the volume mappings as strings makes sense.

[WhyNotHugo]

I've been using bubblewrap to sandbox some locally-installed containers, and I'm defining per-LSP config with this format:

      network = true,
      volumes = {
        {
          host = "/home/hugo/.local/state/rustup",
          lsp = "/home/hugo/.local/state/rustup",
          readonly = false
        },
      },

This works well because it's easy to convert this into either docker or bubblewrap arguments.

[WhyNotHugo]

I should caution that mounting things like go's cache or rust's cache has some risks tho. In particular, a compromised LSP could pollute the shared cache, and this would be picked up by the compiler when building some other project on the host.

[jgero]

I've been using bubblewrap to sandbox some locally-installed containers, and I'm defining per-LSP config with this format:

      network = true,
      volumes = {
        {
          host = "/home/hugo/.local/state/rustup",
          lsp = "/home/hugo/.local/state/rustup",
          readonly = false
        },
      },

This works well because it's easy to convert this into either docker or bubblewrap arguments.

Are you using bubblewrap as a runtime substitution for docker or podman? How does this work, are the CLIs compatible? Otherwise I think this is out of scope for this plugin. If you really want to use that configuration style you could just include the logic to convert from this style to a volume string in your config. For the configuration value I strongly prefer a string type instead of a table. With the table style the user would be unable to add SE Linux labels (the :z at the end) or use named volumes, which I think is a big limitation for no real benefit imo.

[WhyNotHugo]

Are you using bubblewrap as a runtime substitution for docker or podman?

Essentially yes.

How does this work, are the CLIs compatible?

Not at all. But a lot of definitions are. Like volume mappings (+readonly), network (enabled/disabled), etc. Just looking to avoid duplicating all that TBH.

If you really want to use that configuration style you could just include the logic to convert from this style to a volume string in your config.

The configuration style I'm suggesting is rather backend-agnostic, and that's the whole point...

With the table style the user would be unable to add SE Linux labels (the :z at the end)

Appending this to the string is part of generating the cli, and not something that users need to provide IMHO.

This is already the case now too, btw.

[jgero]

Okay I'm sorry I was wrong then. How would a named volume setup look with the table style configuration?

lua

network = true,
      volumes = {
        {
          host = "named_volume",
          lsp = "/home/hugo/.local/state/rustup",
          readonly = false
        },
      },

Something like this?

[jgero]

Would allowing functions as parameters for the extra volumes make sense? I just thought of that because functions would then enable something like this:

image = "docker.io/lspcontainers/gopls",
extra_volumes = {
  function ()
    local env = vim.api.nvim_eval('environ()')
    local gopath = env.GOPATH or env.HOME.."/go"
    return gopath..":"..gopath..":z"
  end
}

It is definitely not necessary but I think some people would like and use this. It would also be useful for setting default values in the plugin itself.

[WhyNotHugo]

@jgero Your function returns the same value each time it's run -- you could just execute that code once and pass its output to extra_volumes. There's no need to regenerate the volumes each time. I guess a function could help if there are scenarios where the volumes vary depending on some runtime condition that can change... but I can't think of any real examples.