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

Dependencies format #19

Open
ii14 opened this issue Feb 11, 2022 · 21 comments
Open

Dependencies format #19

ii14 opened this issue Feb 11, 2022 · 21 comments

Comments

@ii14
Copy link
Member

ii14 commented Feb 11, 2022

The way I see it there are a couple problems with the current dependencies format.

First, special dependencies like neovim. I assume the implementation is supposed to just have special handling for keys like neovim. However, what if a new dependency type like this is introduced in the future? neovim_api for plugins using RPC, treesitter for treesitter version, lua for interpreter version, vim for vanilla vim etc. Sure, you can just bump the spec version when adding something, but what if an implementation wants to add something custom on top of that, that we can't predict? This could potentially result in conflicts with what plugins have declared as dependencies in packspec.lua.

Second, luarocks. I'm not familiar with how it works yet, but how do you detect what is supposed to be a rock? Do you have to clone the repository, check what's inside, and if it's a rock then delete the repository and run luarocks install <rock>? That kinda complicates it.

In my opinion it should be required to somehow specify the type. Either by the type field (but that would make the entire thing kinda verbose) or by grouping them in tables, like this:

dependencies = {}
dependencies.neovim = { version = ">= 0.6.1" }
dependencies.neovim_api = { version = ">= 0.7" }

dependencies.pack = {
  gitsigns = { version = "> 0.3", source = "https://github.com/lewis6991/gitsigns.nvim.git" },
}

dependencies.rock = {
  foo = { ... },
}

-- maybe even move external_dependencies here
dependencies.external = {
  git = { version = ">= 1.6.0" },
}

cc @wbthomason

@lewis6991
Copy link
Member

Let's not assume that treesitter will get special handling within the spec. The same goes for LSP or any built-in feature. We may decide it is out-of-scope. If you disagree, please open a specific issue so we can discuss the requirements the current spec doesn't fulfil.

lua interpreter version should always be LuaJIT, even though Neovim supports being built with other Lua versions, but that's really just an artefact of Neovim's build infrastructure. From my understanding, we don't officially bless non-LuaJIT builds.

but what if an implementation wants to add something custom on top of that, that we can't predict?

That's still the case despite what is proposed here. Implementations have (and should) have a lot of freedom in what they decide to do. If we find any form of fragmentation starts to form, then we improve (and tighten) the spec. We shouldn't over-speculate. And tbh, this point is a little hyperbole, packer.nvim and paq-nvim are currently the only package managers (I know of) that will be reasonably able to support this spec.

Second, luarocks. I'm not familiar with how it works yet, but how do you detect what is supposed to be a rock?

Being registered on https://luarocks.org/. Rocks don't need a URL as it is a centralized source.

We've already agreed on the structure dependencies.<dep>.<field> so if we go in the direction here it should just be a matter of adding a type field. The most complicated plugins have at most 1 or 2 dependencies, so I don't think we should get hung up on verbosity (...yet).

These are just my opinions though. Feel free to push back.

@ii14
Copy link
Member Author

ii14 commented Feb 11, 2022

That's still the case despite what is proposed here.

The point I was making is conflicts between the package names and special dependencies. If packages are kept separate from everything else that should fix the problem. So just as an example, some implementation adds treesitter library version as extension, and some plugin declares nvim-treesitter/nvim-treesitter plugin as treesitter. Or the opposite, plugin assumes you have that extension, while your package manager does not actually support that. If they're separate, the plugin manager that supports the bare minimum of the spec can just ignore unknown special dependencies like that, instead of pointlessly trying to figure out is this a package or rock.

We have no idea what will happen, like maybe someone will one day fork neovim and add some stuff to it that plugins might like to check. If we can future-proof the specification against things like that and make it solid, why shouldn't we?

@lewis6991
Copy link
Member

lewis6991 commented Feb 11, 2022

Other stuff aside, you do raise an important point about conflicting package names.

Since packspec aims to support a decentralized model, we need to be able to handle conflicting names. With current systems, plugins are discriminated using the github project name, which can't conflict, so it hasn't been a problem. However since packspec has a package field for the package name, the lack of any centralization means we can't prevent these names from conflicting. Discriminating by type won't help us. This will be especially important for handling forks of packages.

Not really sure how we handle this, but I think we definitely need to.

@ii14
Copy link
Member Author

ii14 commented Feb 11, 2022

About that, I think the package field and keys for dependencies are going to be basically unused in actual implementations, unless there is something that I don't understand about it. There is no requirement for keys in dependencies to match the package name, right? The only conflict there seems to be what directory the package repo should be cloned to -- think of as field in plugin managers -- and I'm pretty sure it can be solved by doing the same thing, automatically picking another name for it.

@ii14
Copy link
Member Author

ii14 commented Feb 12, 2022

Actually wait one second, you said rocks don't need URL? So it's going to be based on what key it has? How is that supposed to work, if there is a URL it's a package for sure, but if not then check if it happens to be in the luarocks repository? If so then I don't like this idea. If anything, it should probably have a luarocks://<package> URL or something similar in my opinion.

@lewis6991
Copy link
Member

Maybe keep rocks out of dependencies then? Few packages will use rocks (none use them currently), so will be ok in a separate rocks table.

@ii14
Copy link
Member Author

ii14 commented Feb 12, 2022

Anything is fine by me, as long as there is no ambiguity in what the dependency is actually supposed to be. It can be a type field that's required by the spec, it can be a nested table in dependencies or a separate table all together. But keeping everything together is just asking for problems in my opinion, and makes the otherwise straightforward implementation unnecessarily harder.

@muniter
Copy link
Member

muniter commented Feb 12, 2022

I've been thinking a bit about the dependencies and the names colliding. I think an alternative is to think of the url of the dependency as the actual unique ID. And the name provided to be like what dns is to ip addresses (if that makes sense? : 😊)

And then implementations can support something like git rewriting rules:

[url "ssh://[email protected]/"]
	insteadOf = https://github.com/

@lewis6991
Copy link
Member

Hmm that might work. Something like base_url and url becomes base_url/name.

@mjlbach
Copy link
Contributor

mjlbach commented Feb 13, 2022

Can we summarize the options and come to a decision?

  • Nested dependencies table by type
dependencies = {}
dependencies.neovim = { version = ">= 0.6.1" }

dependencies.pack = {
  gitsigns = { version = "> 0.3", source = "https://github.com/lewis6991/gitsigns.nvim.git" },
}

dependencies.rock = {
  foo = { ... },
}

-- maybe even move external_dependencies here
dependencies.external = {
  git = { version = ">= 1.6.0" },
}
  • Dependencies type key (seems a bit redundant IMO)
dependencies = {
  gitsigns = { version = "> 0.3", source = "https://github.com/lewis6991/gitsigns.nvim.git" , type="git"},
  foo = { type = "luarocks" },
}

external_dependencies = {
    neovim = { version = ">= 0.7" }
}
  • Dependencies URI only
dependencies = {
  gitsigns = { version = "> 0.3", source = "git://github.com/lewis6991/gitsigns.nvim.git"},
  foo = { source = "luarocks://foo"},
}

external_dependencies = {
    neovim = { version = ">= 0.7" }
}

Let's separate out the neovim API version validation question, and where to store the neovim dependency itself. I don't think in practice name collisions will be an issue, the name as it stands does not matter, it's mostly organization of the source keys. I think it would look much worse if the dependency table keys were the source URIs.

@lewis6991
Copy link
Member

I've thought about this some and here's my rough idea.

  • Add a package field in each dependency, which defaults to the dependency key name if not provided.
  • Default source to github.
  • Disallow / in package names. May need to introduce a vague notion of source dependent namespaces.

E.g.

dependencies = {
  ['lewis6991/gitsigns.nvim'] = { version = "> 0.3"}
}

canonicalizes too

dependencies = {
  ['lewis6991/gitsigns.nvim'] = { 
    package = 'gitsigns.nvim',
    version = "> 0.3", 
    source = "https://github.com/lewis6991/gitsigns.nvim.git" ,
    type = "git"
  },
}

However, this would still be valid:

dependencies = {
  my_gitsigns_dep = { 
    package = 'gitsigns.nvim',
    version = "> 0.3", 
    source = "https://github.com/lewis6991/gitsigns.nvim.git" ,
    type = "git"
  },
  my_gitsigns_dep2 = { 
    package = 'gitsigns.nvim',
    version = "> 0.3", 
    source = "https://github.com/lewis6991/gitsigns.nvim.git" ,
    type = "git"
  },
  my_gitsigns_dep3 = { 
    package = 'gitsigns.nvim',
    version = "> 0.3", 
    source = "https://github.com/lewis6991/gitsigns.nvim.git" ,
    type = "git"
  },
}

Note: having duplicate deps like this isn't actually possible in Neovim currently, but that shouldn't concern the spec. We just need to have some format to allow it.

This allows us to avoid conflicts in the unlikely case they happen, since the dependency key name isn't strictly the package name.

We may also want to enumerate github (and eventually gitlab) as its own type, similar to git, but better instructs how to canonicalize shortforms.

@mjlbach
Copy link
Contributor

mjlbach commented Feb 13, 2022

Is it worth having a type field if we are already distinguishing source by URI prefix? Or should we remove the URI prefixes?

@lewis6991
Copy link
Member

I think the URL is redundant if there's a given type field, but URL on its own is enough.

For most cases we should aim to build the URL from package+type+namespace. This is at least true for GitHub but other source types will be different.

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

What is the purpose of package name? Where clone the repository to, ie. git clone <url> <directory>?

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

I don't think git is a good name for the type. It shouldn't be distinguished by where the dependency comes from, but what the package manager should do with it. So the default type (pack, package, plugin?) should mean that after cloning the directory, the package manager should add it to vim's runtimepath.

@lewis6991
Copy link
Member

What is the purpose of package name? Where clone the repository to, ie. git clone ?

The match against the deps package name in package.lua/json. Without it, both fields are pointless.

I don't think git is a good name for the type. It shouldn't be distinguished by where the dependency comes from, but what the package manager should do with it. So the default type (pack, package, plugin?) should mean that after cloning the directory, the package manager should add it to vim's runtimepath.

I think what you're really saying is that type is a bad name for this field. We can have source_type instead.

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

The match against the deps package name in package.lua/json. Without it, both fields are pointless.

So the package manager should assert that dependency name matches the actual package name, do I understand it correctly? Is it really necessary though? Or could it be used for package repositories in the future?

I think what you're really saying is that type is a bad name for this field. We can have source_type instead.

Isn't that what the URI scheme is for?

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

Okay, sorry, I can see the point now. In my opinion dependencies should always have the full URI.

@lewis6991
Copy link
Member

If they should always have the full URI, then dependencies only needs to be a list of URI's. However, I disagree with this and users should be able to specify dependencies in a more abstract way, this will allow implementations more freedom in how they handle certain sources. E.g. for git do you use ssh or https or just the Github's gh CLI? User would never care, but implementations can make good decisions on the best way to handle specific source types.

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

That's how I imagined it, that it's essentially just a list of URIs with some metadata, just like most vim plugin managers work. For git I figured you'd just use the git:// URI scheme and let the plugin manager clone the repository however it wants. At least in the sense of picking between ssh and https, I don't know about gh. And if you need to do something on top of that, like use the Github or Gitlab API, it could be probably figured out from the URI. I get the point, but I don't know, I need to think about it.

@rktjmp
Copy link

rktjmp commented Dec 6, 2022

What is the purpose of package name? Where clone the repository to, ie. git clone ?

I don't think the spec should have any impact on where the manger puts things. Don't think you're implicitly suggesting that though.

don't think git is a good name for the type. It shouldn't be distinguished by where the dependency comes from, but what the package manager should do with it. So the default type (pack, package, plugin?) should mean that after cloning the directory, the package manager should add it to vim's runtimepath.

I agree with this.

I like this method to define the neovim requirement

dependencies = {}
dependencies.neovim = { version = ">= 0.6.1" }

I prefer this version as

  • Neovim is so intertwined with the idea that listing it as an "external dep" seems obtuse. It seems very unlikely that a packspec plugin might want to exist without Neovim, and couldn't also use another additional spec format for whatever that might mean.
  • You can explicitly and clearly enforce, "dependencies must define a deps.neovim key with at least a version key" in the spec.
  • It also makes it very painless to check if a plugin is compatible vs scanning ext-deps, looking for "neovim" (which may or may not be present??)
  • I definitely find listing neovim with a url and version in the plain dependencies table (as on the readme example) pretty distasteful - as I think has been covered in other issues

Personally I would promote the key up out of the deps list as I see it as a runtime requirement, not a dependency to "be managed in relation to the plugin", but if you have separate deps.pack deps.rocks, deps.external keys then it also fits in there neat enough.

dependencies.external = {
  git = { version = ">= 1.6.0" },
}

What is the proviso for a tool that has no version, or no semver version?

dependencies.external = {
  -- true -> just must be present?  
  git = true
  git = { version = "any" }, -- 🤮
  -- placeholder for `git` exists and don't check otherwise?
  -- valid constraint but the manager can take liberty to
  -- shortcut it to a presence check?
  git = { version = ">=  0.0.0" },
}

Default source to github.

I'd really prefer to not see this. Specs are written a few times and (programatically) read often, I don't think it hurts too much to define urls explicitly and I don't think it does the collective us any favours by tying a schema explicitly to a single forge.

You could define some namespace shortcut format as say, github@lewis6991/gitsigns.nvim, or github/... (anything before the first /)?

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

No branches or pull requests

5 participants