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

[RDY] lua: Add paths from &runtimepath to package.path and package.cpath #6789

Merged
merged 7 commits into from Jun 27, 2017

Conversation

Projects
None yet
4 participants
@ZyX-I
Contributor

ZyX-I commented May 22, 2017

No description provided.

api/vim: Fix nvim_list_runtimepaths
It used to

1. Always omit last component in runtimepath.
2. Always omit trailing empty item and leave uninitialized memory in place of 
   it.
@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

By the way, I found that both in Vim and Neovim lua package.path/package.cpath imports from current directory. I do not think we need this (and AFAIR Python bindings contain code to throw away imports from the current directory). Better not in this PR though.

@ZyX-I ZyX-I force-pushed the ZyX-I:lua-path branch from f201474 to 20452d2 May 22, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

Also existing nvim_list_runtimepaths appeared to both lack any tests and be utterly broken.

@@ -299,7 +299,7 @@ ArrayOf(String) nvim_list_runtime_paths(void)
FUNC_API_SINCE(1)
{
Array rv = ARRAY_DICT_INIT;
uint8_t *rtp = p_rtp;
char_u *rtp = p_rtp;

This comment has been minimized.

@justinmk

justinmk May 22, 2017

Member

what is the purpose of this change? If we plan to eliminate char_u, is uint8_t not the appropriate replacement? Is the concern about systems with char size != 8?

This comment has been minimized.

@ZyX-I

ZyX-I May 22, 2017

Contributor

I was already commenting this. uint8_t is 8-bit unsigned number. char_u is an unsigned character. It is deprecated, but not in a sense “you should replace char_u with uint8_t”, it is to be replaced by “char”, with a few exceptions where a number was actually meant and not a character.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

Travis build halted:

[ RUN      ] jobs can omit exit callback: 7.54 ms OK
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

Seems irrelevant, restarting.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

By the way, it looks like nvim simply busy processing something like “press enter” prompt. Can’t we just kill it? It looks like everything what needed is there, just need to replace session:close() with session:close('kill') in functional helpers.

@justinmk

This comment has been minimized.

Member

justinmk commented May 22, 2017

just need to replace session:close() with session:close('kill') in functional helpers.

Certainly worth a try, but my guess was that it sometimes happens before the end of the test.

With nvim_get_mode there is now a way to detect this state, FWIW.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

@justinmk Test has already reported “OK”, but next test has not yet started. I am not sure whether busted would run our clear() before or after printing test name, but AFAIR it was running before. And certanly this is not something “before the end of the test”, there would not be OK in log otherwise.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 22, 2017

AppVeyor failed due to slashes, that was somewhat expected (I was not sure what lua would put in package.[c]path, so did not adjust anything).

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 23, 2017

Everything successfull, but I forgot to run testlint. Should be ready after CI.

@ZyX-I ZyX-I changed the title from lua: Add paths from &runtimepath to package.path and package.cpath to [RDY] lua: Add paths from &runtimepath to package.path and package.cpath May 23, 2017

local new_paths_str = table.concat(new_paths, ';')
eq(new_paths_str, eval_lua('package.path'):sub(1, #new_paths_str))

set_path('path', as'foo/?.lua;foo/?/init.lua;' .. new_paths_str)

This comment has been minimized.

@justinmk

justinmk May 23, 2017

Member

Why not use parens for function calls? I thought we always did so. as is a misleading name as well, I would suggest sl or slash.

This comment has been minimized.

@ZyX-I

ZyX-I May 23, 2017

Contributor

I figured it would look nicer if I pretended it is something like string/table literal prefix (similar to e.g. L"foo" for const wchar_t*) and not a function call.

Neovim lua interface automatically adjusts `package.path` and `package.cpath`
according to effective &runtimepath value. Adjustment happens after each time
'runtimepath' is changed, `package.path` and `package.cpath` are adjusted by
prepending directories from 'runtimepath' each suffixed by `/lua` and

This comment has been minimized.

@justinmk

justinmk May 23, 2017

Member

@ZyX-I I am trying to understand this explanation. How is /lua suffix involved in the following example? The example mentions /foo in &rtp, and prepends /foo to some parts of package.cpath. But /lua is not involved.

This comment has been minimized.

@ZyX-I

ZyX-I May 23, 2017

Contributor

Example is incorrect, it should be /foo/lua/?.lua, etc.

current values of `package.path` or `package.cpath`. If you happened to delete
some paths from there you need to reset 'runtimepath' to make them readded.

Note 3: paths containing semicolons are ignored.

This comment has been minimized.

@justinmk

justinmk May 23, 2017

Member

Do you mean paths in 'runtimepath' ?

This comment has been minimized.

@ZyX-I

ZyX-I May 23, 2017

Contributor

Such paths could not be present in package.[c]path, so yes.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 23, 2017

I have added example plugin which should make it easier to understand, plugin was tested locally on README.md (checked that with  main() { printf("%.*s", (int)sizeof(blob), blob); } added to the end of the transformed README tcc -run - transforms transformed README back).

`./a?d/j/g.nlua;./?.lua;/bar/?.lua` the resulting `package.path` after
adjustments will look like this: >

/foo/lua/?.lua;/foo/lua/a?d/j/g.nlua;./a?d/j/g.nlua;./?.lua;/bar/?.lua

This comment has been minimized.

@justinmk

justinmk May 23, 2017

Member
  1. Why isn't there also /foo/lua/bar/?.lua ?
  2. Is there any significance to the swapped of the order of /foo/lua/?.lua;/foo/lua/a?d/j/g.nlua; vs the original order, ./a?d/j/g.nlua;./?.lua ? Shouldn't the order be preserved, or it doesn't matter for lua loading rules?

This comment has been minimized.

@ZyX-I

ZyX-I May 23, 2017

Contributor
  1. It must not be there, out of /bar/?.lua it gets /?.lua suffix.
  2. Order is preserved, example is wrong again.

This comment has been minimized.

@ZyX-I

ZyX-I May 23, 2017

Contributor

The 1. point is explained in the first note.

@justinmk

This comment has been minimized.

Member

justinmk commented May 23, 2017

The example plugin is helpful, thanks.

So the TL;DR is that lua/ on 'runtimepath' works like plugin/, ftplugin/, pythonx/, etc.

`./?.lua;./a?d/j/g.nlua;/bar/?.lua` the resulting `package.path` after
adjustments will look like this: >

/foo/lua/?.lua;/foo/lua/a?d/j/g.nlua;./?.lua./a?d/j/g.nlua;;/bar/?.lua

This comment has been minimized.

@justinmk

justinmk May 23, 2017

Member

missed a semicolon :)

@ZyX-I ZyX-I force-pushed the ZyX-I:lua-path branch from b9e9f4f to 4ac0894 May 23, 2017

@justinmk

This comment has been minimized.

Member

justinmk commented May 23, 2017

From comments:

It must not be there, out of /bar/?.lua it gets /?.lua suffix.

From doc:

Note that code have taken everything starting from the last path component from existing paths containing a question mark as a ?-containing suffix, but only applied unique suffixes.

Let me see if I understand.

  • /bar/?.lua, /zub/?.lua, ./?.lua, and /?.lua all resolve to the same "unique suffix". Is that correct?
  • If package.path contains /zim/zam/?.lua, the "unique suffix" is /zam/?.lua. And for the example in the docs, the adjusted package.path would contain /foo/lua/zam/?.lua.
  • Is it correct to conclude that the first segment of each package.path entry is removed? I see that ./ is removed from ./?.lua, and /bar is removed from /bar/?.lua.
@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 23, 2017

Not first segment is removed. It takes first segment which contains question mark and uses it and everything what follows as a prefix. So /?.lua, /foo/?.lua, /foo/bar/?.lua and /foo/bar/baz/?.lua give the same /?.lua suffix.

from existing paths containing a question mark as a `?`-containing suffix, but
only applied unique suffixes.

Note 2: even though adjustments happens automatically Neovim does not track

This comment has been minimized.

@aktau

aktau May 24, 2017

Member

Does this paragraph say that if you adjust package.path and package.cpath from within a Lua script run by Neovim, the result will be unreliable because the next runtimepath change will reset the package path?


Note 2: even though adjustments happens automatically Neovim does not track
current values of `package.path` or `package.cpath`. If you happened to delete
some paths from there you need to reset 'runtimepath' to make them readded.

This comment has been minimized.

@aktau

aktau May 24, 2017

Member

s/make them readed/regenerate the (c)path/.

Also I think it would be nice to illustrate this with some Vim/Lua code for how to properly change the cpath and then regenerate the runtimepath.

/// Calls nlua_init() if needed. Is responsible for pre-lua call initalization
/// like updating `package.[c]path` with directories derived from &runtimepath.
///
/// @return Interprter instance to use. Will either be initialized now or taken

This comment has been minimized.

@aktau

aktau May 24, 2017

Member

Interpreter.

@aktau

This comment has been minimized.

Member

aktau commented May 24, 2017

I can't immediately see how this change solves the problem of package precedence (sorry, didn't look at all the code, just the docs).

The problem is this:

  1. plugin A wants to use its own module bar
  2. plugin B also wants to use its own module bar

This could be done by setting a different (c)path before the invocation of any function in plugin A or B:

package.path for any function in plugin A is: "/path/to/plugin/A/lua/?.lua;" .. package.path. This means that plugin A will always preferentially load bar.lua from its own folder. The advantage of this approach is that plugin A and plugin B could use require("bar") and be happy. However, I consider this hacky (changing the path all the time) and it makes the next use case a bit more annoying:

  1. plugin C wants to use plugin A's bar [1]

In an ideal world, plugin C could just use require("A.bar"). However, I don't think the current (c)path setup allows this. The path for plugin A's bar is: /path/to/plugin/A/lua/bar.lua, which means that the module only has the name require("bar"), and if we decide to also add /path/to/plugin/?.lua [2] to the (c)path, then we could load it as require("A.lua.bar"). Which is doable, but looks a bit ugly.

An alternative strategy may be to tell the user to create their lua files inside of subfolders named after their own plugin, e.g.: /path/to/plugin/A/lua/A/bar.lua [3]. Then, both plugin A would need to require("A.bar") and plugin B would need to require("B.bar"). Plugin C can do just like plugin A: require("A.bar"). This requires no continuous changes to the package path depending on which function is being called. It also shows directly in the code where a function or module is from, which I think is good for readability.

Of course, this would rely on plugin authors doing the right thing, they can name the folders however they want and mess with the loading order if we don't change the package path for every separate plugin.

An automated alternative solution (though magical), is to create a shadow folder for all Lua files under $PLUGIN/lua/*.lua. But that will fail in many unpredictable ways. Some Lua code may rely on its own relative location.

[1] This use case is not too crazy, some plugins require other plugins, re. vim-easytags needing vim-misc.
[2] Deduplicated, because for Bundle/Vundle/Pathogen/... setups this will be the same for every plugin.
[3] I've seen the idea in vim-misc/autoload/xolox/misc.

@justinmk

This comment has been minimized.

Member

justinmk commented May 24, 2017

An alternative strategy may be to tell the user to create their lua files inside of subfolders named after their own plugin, e.g.: /path/to/plugin/A/lua/A/bar.lua [3]. Then, both plugin A would need to require("A.bar") and plugin B would need to require("B.bar"). Plugin C can do just like plugin A: require("A.bar"). This requires no continuous changes to the package path depending on which function is being called. It also shows directly in the code where a function or module is from, which I think is good for readability.

That is analogous to how VimL "namespaces" work, right? Function foo in /path/to/plugin/A/autoload/A/bar.vim is called A#bar#foo().

If it works like that then that is consistent with Vim plugin conventions.

@aktau

This comment has been minimized.

Member

aktau commented May 24, 2017

That is analogous to how VimL "namespaces" work, right? Function foo in /path/to/plugin/A/autoload/A/bar.vim is called A#bar#foo().

If it works like that then that is consistent with Vim plugin conventions.

It would work like that if users followed this setup. It may be helpful to document that this is desirable for plugin interoperability and to avoid collisions. By collisions I mean: if we have a static package.path for our lua_State, then one library path will always have precedence over the other. So if two plugins have bar.lua inside of their lua/ folder, then both plugins will load the same file (one plugin will be loading the other plugins' bar.lua).

Please correct me if I'm wrong, @ZyX-I.

@aktau

This comment has been minimized.

Member

aktau commented May 24, 2017

I think I understand what this PR tries to do better now (from reading the docs and comments), and I think it goes a bit too far with the package path modification (which also makes it difficult to explain). The part I disagree with:

...package.path and package.cpath are adjusted by
prepending directories from 'runtimepath' each suffixed by /lua and
?-containing suffixes from package.path and package.cpath
. I.e. when
'runtimepath' option contains /foo and package.path contains only
./a?d/j/g.nlua;./?.lua;/bar/?.lua the resulting package.path after
adjustments will look like this...

Not only is this hard to parse, but I really don't think this is useful for plugin authors. They don't/can't know what the (c)path setup of the user is ($LUA_PATH and $LUA_CPATH env variable), so they can't rely on this.

It would be far simpler to just:

package.path and package.cpath are adjusted by prepending directories from 'runtimepath' each suffixed by /lua/?.lua. Suppose the default package.path is

./?.lua;./bar/?.lua

After modification, it will look like:

`$PLUGIN_A/lua/?.lua;$PLUGIN_B/lua/?.lua;./?.lua;./bar/?.lua

NOTE: The order in which paths are prepended is not defined, so don't rely on it to break name collisions. Rather, put your Lua files inside of a folder named after your plugin. This minimizes the chance of collisions and allows other plugin authors to rely on your functions. For example:

/path/to/mycoolplugin/lua/**mycoolplugin/mypackage.lua**

Loading this module is done using local mypackage = require("mycoolplugin.mypackage").

Having a single way to resolve packages is good for readability and interoperability.

prepending directories from 'runtimepath' each suffixed by `/lua` and
`?`-containing suffixes from `package.path` and `package.cpath`. I.e. when
'runtimepath' option contains `/foo` and `package.path` contains only
`./?.lua;./a?d/j/g.nlua;/bar/?.lua` the resulting `package.path` after

This comment has been minimized.

@aktau

aktau May 24, 2017

Member

While possible, ./a?d/j/g.nlua is really a very strange example.

@aktau

This comment has been minimized.

Member

aktau commented May 25, 2017

Which explosion? The code does not distinguish non-absolute and absolute paths, it simply takes the first path component containing ? plus path separator (also avoids bothering with path separators) plus everything what follows. It does not care a tiny bit whether it takes data from absolute or relative path. So /foo/bar/?.lua and ./?.lua will both contribute /?.lua suffix, but it is repeated only once.

I felt like I had to read the code now (left some comments too), because I just couldn't figure it out from your wording. The (Vim) docs may need a revision, but I believe it is just hard to explain this. Perhaps a native English speaker like @justinmk (or whoever else is lurking) can try their hand? I think it's important we give prospective Neovim/Lua developers an accurate and easy to understand introduction.

Now that I read the code, I understand what you're doing with this code. I still think it's overengineering things. Making stuff more flexible also has downsides (see below).

One of the other things I'm trying to avoid is some plugin working on the developers' machine, because she has package.path = "./?.lua; ./?/baz.lua", and their plugins' code is at /somemodule/baz.lua. This won't work on most other users' Neovim installations.

Why would she want to have this?

Do you mean: why would she want her Lua file named /somemodule/baz.lua? If that's the case: I don't presume to know what drives users to name their files a certain way. What's worse: it will work on their system but not on other users' systems. Imagine some disto adding ./?/init.lua to the package path, and a developer makes a plugin which works there. This plugin won't work on other distros that do not add this package path. It will be reported as a bug and the plugin author will investigate, bump into the distro's package.path mismatch, and fix their package to follow the nearly ubiquitous ./?.lua standard such that it works for everyone.

Why should everyone go through this lesson, if we can fix it right now?

TL;DR, I'm in favour of converting every rtp path into: rtp .. sep .. 'lua' .. sep .. '?.lua'.

That convoluted method of determining paths is there because I am thinking that assumptions “people do not put non-standard suffixes into package.[c]path without a good reason, and if they did they may want to do the same for every path” and “there will be all needed suffixes in package.[c]path” are true enough to rely on them more then on hardcoding those suffixes in vim.lua code.

In reality, suffixes like /?/init.lua will be present because of omnipresent paths like /usr/local/share/lua/5.2/?/init.lua. It could be that users find this useful, for making an init.lua file which requires all the other files in the directory. Sort of like a global package initializer:

-- init.lua, to require this in your code, call:
-- require("A.lua")
return {
  functional = require("A.lua.functional") -- load functional.lua in the same folder.
  imperative = require("A.lua.imperative") -- load imperative.lua in the same folder.
}

But if so, I propose we add this for everyone, and not at random depending on whether the users' current [c]path already contains such a suffix. I'm telling you, this behaviour will be surprising, we need to standardize it as best we can. And if there is some OS which we currently support that doesn't play nice with whatever "hardcoded" thing we come up with, it's easy to fix.

BTW, these are my paths on macOS:

$ le 'print("LUA MODULES:\n",(package.path:gsub("%;","\n\t")),"\n\nC MODULES:\n",(package.cpath:gsub("%;","\n\t")))
'
LUA MODULES:
	/usr/local/share/lua/5.2/?.lua
	/usr/local/share/lua/5.2/?/init.lua
	/usr/local/Cellar/lua/5.2.4_4/libexec/share/lua/5.2/?.lua
	/usr/local/lib/lua/5.2/?.lua
	/usr/local/lib/lua/5.2/?/init.lua
	./?.lua
	/Users/aktau/.lua/?.lua

C MODULES:
	/usr/local/lib/lua/5.2/?.so
	/usr/local/lib/lua/5.2/loadall.so
	./?.so
	/Users/aktau/.lua/?.so

Also it allows code I put into vim.lua not bother about path separators or file extensions. (And it provides some “automatic” forward compatibility, but I hope we will not actually need this.)

Path separators is a done deal (see comment). File extensions is something else. To be very honest, we control which Lua(JIT) gets released with Neovim, so if there are any compatibility breakages there (e.g.: the .lua extension is deprecated and we should all use .LUNA), we will know. Moreover, if we have an example plugin that itself gets loaded and tested in our testsuite, we will notice automatically.

For cpath, I agree that copying whatever is already there has some advantages versus attempting to hardcode the shared library format for our supported OS'es (Darwin: .dylib, Windows: .dll, UNIX: .so). Though still, I would not be against this hardcode, if it unifies the way in which plugin authors load their modules.

I have 1645 packages in the system (not Vim plugins, regular packages) sharing much less amount of paths like /usr/bin. Most of time there are no collisions, rare collisions which are present are either handled by the maintainers of ebuilds (by setting blocks which prevent installing some packages, or renaming (e.g. rc (Plan 9 shell reimplementation) was renamed to rcsh), or using version suffixes and eselect to manage symlinks (python2.7, python3.4 and python3.5 may be installed at the same time on the same system), etc) or by package manager itself (before installation, last resort).

You're saying that distribution maintainers will detect and take care of this (see below for the consequences)? Most distributions don't even package a small percentage of what's available in the wide world of Vim plugins, by the way. What do regular users do when they find collisions?

What I am talking about: collisions are rare. Most plugin writers know that they should be using plugin-specific prefixes in many places to avoid problems, are used to writing plugins like this and definitely do not want their plugin be confused with some other plugin so check whether name is occupied.

I really do not think they will be rare if everyone puts their .lua files inside of their plugins' /lua folder. I'm trying to not overdo it with plugins yet I'm counting 30 here. They are all managed by vim-plug and I'm betting a decent chunk of our userbase uses such plugin managers as well, instead of their distributions' package manager (which they may not have, if they're on Windows).

By the way, if we rely on the distribution maintainers to solve this problem, they will (1) dislike us and (2) this work will get duplicated and (3) it's not that trival to fix, you need to rename both the Lua file and the require statements. It's not trivially automatable. Furthermore, they may decide to drop plugins for being more trouble than they're worth. Meanwhile, if it's just packaging up a few plain files from a tarball, everything is hunky dory.

It is a non-problem, we do not have a warning about collisions in if_pyth.txt and still everything is fine. And there is absolutely nothing special in Python regarding package naming issues.

I'm guessing that before this conversation is over, I'm going to have to look at how Python does this in order to provide a good (or none) retort. Still though, I'm thinking of how to make Lua better, not how to make it conform to Python, howevermuch (or not) their package management schemes look alike.

There is a difference between what package.[c]path is not allowed to contain in directory names (basically only semicolon) and what ? can expand to. E.g. you can’t have directory named deoplete.nvim because substituting A to deoplete.nvim in require('A.lua.bar') will make lua search for deoplete/nvim/lua/bar.lua. I do not agree this should be banned.

You're completely right, of course, but this will be detected early by the plugin authors. They will name their subfolder something like deoplete (it's also not really vim code, so that makes sense). Example:

path/to/deoplete.vim/lua/deoplete/somefunctions.lua

And note: Vim plugin managers have a habit of putting plugin in a one-plugin-per-directory fashion only because their authors were being lazy and regular people are not supposed to have hundreds of Vim plugins where advantage of using /usr/bin is enough greater then advantage of /usr/bin/packagea:/usr/bin/packageb:/usr/bin/packagec to justify less maintainable directory structure. And also new executable in /usr/bin causes much less slowdown then new .vim file in plugin/ because it is not automatically loaded.

So you're saying that the directory-per-plugin approach is less maintainable? I beg to disagree. The popularity of package managers like pathogen/vim-plug/vundle/... seems to suggest otherwise.

I also don't think the reason they're put into different folders is because of plugin author laziness. Rather, it's to make plugin management more mangeable. Few people care about having a smaller or larger &runtimepath. I'd wager more people care about being able to easily install/deinstall/update packages because they're all in their own paths with their own git configuration.

@aktau

This comment has been minimized.

Member

aktau commented May 25, 2017

We should probably also invite some well-known Vim/Lua plugin developers to the discussion. @Shougo, can you give some input on how Vim plugin developers (would like to) use or organize their Lua plugins?

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 25, 2017

Do you mean: why would she want her Lua file named /somemodule/baz.lua? If that's the case: I don't presume to know what drives users to name their files a certain way. What's worse: it will work on their system but not on other users' systems. Imagine some disto adding ./?/init.lua to the package path, and a developer makes a plugin which works there. This plugin won't work on other distros that do not add this package path. It will be reported as a bug and the plugin author will investigate, bump into the distro's package.path mismatch, and fix their package to follow the nearly ubiquitous ./?.lua standard such that it works for everyone.

I do not think that this is likely to happen (BTW, /?/init.lua is one of the standard suffixes): what benefit will distro have of creating non-standard suffix? And note that they will receive bugs from lua users shortly if they strip out standard once, so since various lua tutorials talk about standard paths this is not likely to cause troubles.

Do you mean: why would she want her Lua file named /somemodule/baz.lua?

No, why would she want to add /?/baz.lua to path. She can have such file and do require('somemodule.baz') in place of require('somemodule') with /?/baz.lua-ending entry.

In reality, suffixes like /?/init.lua will be present because of omnipresent paths like /usr/local/share/lua/5.2/?/init.lua. It could be that users find this useful, for making an init.lua file which requires all the other files in the directory. Sort of like a global package initializer:


But if so, I propose we add this for everyone, and not at random depending on whether the users' current [c]path already contains such a suffix. I'm telling you, this behaviour will be surprising, we need to standardize it as best we can. And if there is some OS which we currently support that doesn't play nice with whatever "hardcoded" thing we come up with, it's easy to fix.

It is not random. If users current package.path does not already contain such a suffix then a bunch of lua modules is broken in the user environment: e.g. busted and luacheck use init.lua. I do not think that relying on user environment not breaking widely used modules means “at random”.

You're saying that distribution maintainers will detect and take care of this (see below for the consequences)? Most distributions don't even package a small percentage of what's available in the wide world of Vim plugins, by the way. What do regular users do when they find collisions?

Go to the plugins issue tracker.

I really do not think they will be rare if everyone puts their .lua files inside of their plugins' /lua folder. I'm trying to not overdo it with plugins yet I'm counting 30 here. They are all managed by vim-plug and I'm betting a decent chunk of our userbase uses such plugin managers as well, instead of their distributions' package manager (which they may not have, if they're on Windows).

They will be rare if plugin authors understand that they need to have plugin-specific name. I did not see plugins failing to understand that for a long period of time (users may report a problem), unless they are using things like :luafile and not proper modules. AFAIK all +python plugin authors who understand the need of using modules also understand the need of naming them properly, do not think lua will be different.

By the way, if we rely on the distribution maintainers to solve this problem, they will (1) dislike us and (2) this work will get duplicated and (3) it's not that trival to fix, you need to rename both the Lua file and the require statements. It's not trivially automatable. Furthermore, they may decide to drop plugins for being more trouble than they're worth. Meanwhile, if it's just packaging up a few plain files from a tarball, everything is hunky dory.

Distribution maintainers normally do not do the renaming. The supposed ways of fixing things is

  1. Reporting the issue upstream.
  2. Temporary set a block (package A blocks installing B and vice versa).
  3. Removing block if issue is fixed upstream.

Dropping plugin is also an option, then whoever was interested in package will report the issue. Actually, I would expect proposed new packages with plugins with name collisions be rejected more then adding blocks for them.

Note: the only thing plugin author needs to do is use plugin-specific name for his files. As long as he does that with proper plugin name choice the name collisions are rare.

So you're saying that the directory-per-plugin approach is less maintainable?

I am saying exactly the opposite: directory-per-plugin is more maintainable, but has some disadvantages. Currently disadvantages are not great enough to justify using less maintainable “everything in one directory” variant, but that is not the case for system directories like /usr/bin.

@aktau

This comment has been minimized.

Member

aktau commented May 25, 2017

I do not think that this is likely to happen (BTW, /?/init.lua is one of the standard suffixes):

It is one of the standard suffixes (though it is not one that's used for relative paths by default).

...what benefit will distro have of creating non-standard suffix?

I don't know, but your current solution decidedly does copy anything non-standard that's in the path right now, whether added by the distribution or the user.

And note that they will receive bugs from lua users shortly if they strip out standard once, so since various lua tutorials talk about standard paths this is not likely to cause troubles.

Yes, I also don't believe that anyone will cut down on the defaults (see below), that would lead to bug reports. That's why I believe we must focus on the defaults, and not propagate weird stuff from the users' path.

Defaults:

luajit-2.1/src » cpp =(echo "#include \"luaconf.h\"\nchar *path = LUA_PATH_DEFAULT;\nchar *cpath = LUA_CPATH_DEFAULT") | ggrep -P '\*c?path'
char *path = "/usr/local/" "share/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.lua;" "/usr/local/" "share/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?/init.lua;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.lua;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?/init.lua;" "./?.lua";
char *cpath = "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.so;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""loadall.so;" "./?.so"

lua-5.1.5/src » cpp =(echo "#include \"luaconf.h\"\nchar *path = LUA_PATH_DEFAULT;\nchar *cpath = LUA_CPATH_DEFAULT") | ggrep -P '\*c?path'
char *path = "/usr/local/" "share/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.lua;" "/usr/local/" "share/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?/init.lua;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.lua;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?/init.lua;" "./?.lua";
char *cpath = "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""?.so;" "/usr/local/" "lib/lua/" LUA_VERSION_MAJOR "." LUA_VERSION_MINOR "/""loadall.so;" "./?.so"

The only relative path is ./?.lua. The suffix .../?/init.lua does occur, though only for absolute paths. I'm not against adding this by default as well: **for every runtime path, add $rtp/lua/?/init.lua;$rtp/lua/?.lua.

The funny thing is that with this setup, if a plugin developer wants to use an init.lua, they will have to create a subfolder, which is exactly what I would want (to minimize collisions).

Anyway, what I'm trying to get at is this: if a user wants to create and publish a Vim plugin, that code is no longer meant just for the users' machine. If we can nudge the user towards.

Tell you what, as soon as people start complaining that their strange path patterns are not being propagated to their Vim/Lua plugin folders. I solemnly swear I'll send a PR to add this functionality. It's easy to add new functionality, it's very hard to take it away.

Do you mean: why would she want her Lua file named /somemodule/baz.lua?

No, why would she want to add /?/baz.lua to path. She can have such file and do require('somemodule.baz') in place of require('somemodule') with /?/baz.lua-ending entry.

Yes! Sometimes I believe we're actually violently agreeing with each other. The developer could do this, as you say. That doesn't mean they will. /?/baz.lua was an example of something the developer has in their package.path for some reason, and because it is there, their plugin which has require('somemodule') works on their computer. I want to eliminate the possibility of this confusion. Reducing the variation in cpath suffixes for Vim plugins has the other good side effect of making Vim/Lua code more readable: if I see an require statement, I know exactly where to find the file it's loading.

I want plugins using Lua to be portable across all distros/OS'es that Neovim support from the start. Therefore I'd like to eliminate variation in path.

It is not random. If users current package.path does not already contain such a suffix then a bunch of lua modules is broken in the user environment: e.g. busted and luacheck use init.lua. I do not think that relying on user environment not breaking widely used modules means “at random”.

The question of breaking widely used modules is really a separate thing from this PR. To actually allow sourcing Lua libraries that have nothing to do with Vim plugins will require more jumbling. I think we can defer this to (a bit) later. That said, init.lua seems ubiquitous enough for both the standard Lua distribution and packages on Luarocks, I'd be OK with adding it.

Go to the plugins issue tracker.

I don't even know what/where that is... Can you perhaps point something out? More importantly: what's the harm in adding a note to the docs telling people the dangers of having their *.lua files in the top-level ./lua path of their plugin (collisions, inability of other plugins to use their code)? Advising them that they should put their files in something like mycoolplugin/lua/mycoolplugin/*.lua (or deeper).

I really do not think they will be rare if everyone puts their .lua files inside of their plugins' /lua folder. I'm trying to not overdo it with plugins yet I'm counting 30 here. They are all managed by vim-plug and I'm betting a decent chunk of our userbase uses such plugin managers as well, instead of their distributions' package manager (which they may not have, if they're on Windows).

They will be rare if plugin authors understand that they need to have plugin-specific name. I did not see plugins failing to understand that for a long period of time (users may report a problem), unless they are using things like :luafile and not proper modules. AFAIK all +python plugin authors who understand the need of using modules also understand the need of naming them properly, do not think lua will be different.

Yes! If they understand. What's so wrong with adding a note to the docs? Is this tiny amount of text going to inconvenience everyone?

Distribution maintainers normally do not do the renaming. The supposed ways of fixing things is

Sounds like a bunch of work that could be avoided with just a little bit of advice.

So you're saying that the directory-per-plugin approach is less maintainable?

I am saying exactly the opposite:

In that case we are in agreement. I think I failed to correctly parse your reply, my apologies.

...directory-per-plugin is more maintainable, but has some disadvantages. Currently disadvantages are not great enough to justify using less maintainable “everything in one directory” variant, but that is not the case for system directories like /usr/bin.

For my curiosity: which disadvantages? And even more importantly: when/how are those disadvantages going to grow large enough?

Also, I never mentioned system directories like /usr/bin. I don't think it's a particularly good comparison (not in the least because people don't drop 10's of files per package in there, usually a single binary which is supposed to be uniquely named because otherwise the user could not invoke it). In fact, the /usr/bin example would be equivalent to Vim plugins being named exactly the same. AFAIK, in vim-plug/vundle/... those would collide, since the filesystem paths do not contain the github/bitbucket/... user name.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 25, 2017

I don't even know what/where that is... Can you perhaps point something out? More importantly: what's the harm in adding a note to the docs telling people the dangers of having their .lua files in the top-level ./lua path of their plugin (collisions, inability of other plugins to use their code)? Advising them that they should put their files in something like mycoolplugin/lua/mycoolplugin/.lua (or deeper).

I see absolutely no reason in using /lua/mycoolplugin/*.lua. If plugin is small enough to have just one file it can just as well have /lua/mycoolplugin.lua. Two levels are not needed for many plugins.

For my curiosity: which disadvantages? And even more importantly: when/how are those disadvantages going to grow large enough?

That was in the previous-previous reply: searching is harder if you have hundreds of directories (e.g. out of my 1645 packages 755 have put something in /usr/bin). And with the current infrastructure it increases memory footprint in many places: e.g. 755 * /usr/bin/{pkgname} means something like 18 KiB in environment, also need to hold all those directories in FS cache and some shells have their own caches as well. Additionally the easiest way to define $PATH for semi-secure environments (where user items are not allowed in $PATH at shell startup) is lost. Many problems like this could be fixed by using different infrastructure, but $PATH searching is one of the simplest methods of finding executables.

They could not possibly grow large enough with (Neo)Vim plugins as long as most big plugins are doing something (i.e. have plugin/ folder or anything else loaded at each startup). I do not think that on modern PCs they would grow large enough up until regular (Neo)Vim user will have about a thousand occasionally used library/syntax/ftplugin plugins (if they are used always user will have other problems with slow startup much earlier then hitting &runtimepath search slowdown).

@aktau

This comment has been minimized.

Member

aktau commented May 25, 2017

I see absolutely no reason in using /lua/mycoolplugin/*.lua. If plugin is small enough to have just one file it can just as well have /lua/mycoolplugin.lua. Two levels are not needed for many plugins.

That's something I could live with, as well. In addition, this approach allows a seamless switchover to an init.lua script that loads another set of files (as in my earlier comments). I still strongly feel we should provide some guidance to the user. What good is it if we're discussing ways to organize plugins that's consistent and unlikely to collide if everyone has to find out about that for themselves? Especially the occasional user if Vim/Lua.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 25, 2017

Yes! If they understand. What's so wrong with adding a note to the docs? Is this tiny amount of text going to inconvenience everyone?

There are no such notes in other places. Also there already is an example of the plugin which is using plugin-specific naming. Also the correct place to put such notes are sections like :h write-plugin and not if_lua.txt.

By the way, :h write-plugin is highly outdated (still suggest 8-character names), shows bad practices (:abbrev in place of :noreabbrev, uses :map (in some of the examples no nore is OK, but not :map, should be something more specific like :nmap)), not talks about VCS (who needs “Last change” in header when there is something as wonderful as mercurial?).

`?`-containing suffixes from `package.path` and `package.cpath`. I.e. when
'runtimepath' option contains `/foo` and `package.path` contains only
`./?.lua;./a?d/j/g.nlua;/bar/?.lua` the resulting `package.path` after
'runtimepath' is changed. `package.path` is adjusted by simply appending

This comment has been minimized.

@aktau

aktau May 25, 2017

Member

s/after//, it feels superfluous.

This comment has been minimized.

@ZyX-I

ZyX-I May 25, 2017

Contributor

But it is more correct. Adjusting is not triggered by changing &runtimepath, it is triggered by running something like :lua after &runtimepath was changed.

`./?.lua;./a?d/j/g.nlua;/bar/?.lua` the resulting `package.path` after
'runtimepath' is changed. `package.path` is adjusted by simply appending
`/lua/?.lua` and `/lua/?/init.lua` to each directory from 'runtimepath' (`/`
is actually a first character from `package.config`).

This comment has been minimized.

@aktau

aktau May 25, 2017

Member

s/a first/the first/
s/from package.config/of package.config/

Or rather:

/ is actually the operating system's native path separator (the first character of package.config).

`package.cpath` is adjusted by prepending directories from 'runtimepath' each
suffixed by `/lua` and `?`-containing suffixes from existing `package.cpath`.
I.e. when 'runtimepath' option contains `/foo` and `package.cpath` contains
only `./?.so;./a?d/j/g.elf;/bar/?.so` the resulting `package.cpath` after

This comment has been minimized.

@aktau

aktau May 25, 2017

Member

I still find this wording hard to parse: is adjusted by prepending ... each suffixed by ... -containing suffixing from. This is work for a native English speaker, but I'll give it a shot:

Similarly to `package.path`, modified directories from `runtimepath` are also added to `package.cpath`. In this case, instead of appending `/lua/?.lua` and `/lua/?/init.lua` to each runtimepath, all unique `?`-containing suffixes of the existing `package.cpath` are used. An example explains it better: 

1. 'runtimepath' option contains `/foo`
2. `package.cpath` is `./?.so;./a?d/j/g.elf;/bar/?.so`, which means the unique `?`-suffixes are `/?.so` (from `./?.so` and `/bar/?.so`) and `/a?d/j/g.elf` (from `./a?d/j/g.elf`). 
3. Thus the adjusted `package.cpath` looks like:  

  /foo/lua/?.so;/foo/lua/a?d/j/g.elf;./?.so;./a?d/j/g.elf;/bar/?.so
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
             runtimepath derived        ^^^^^^^^^^^^^^^^^^^^^^^^^
                                                         original package.cpath

I realize this is also in poor shape. I just believe that though the Lua pattern is easy to grok, any text describing it is not. Which is why emphasizing the example is worth a War and Peace IMHO. Any suggestions, @justinmk?

@@ -31,7 +34,8 @@ current values of `package.path` or `package.cpath`. If you happened to delete
some paths from there you need to reset 'runtimepath' to make them readded.

Note 3: paths from 'runtimepath' which contain semicolons cannot be put into
`package.[c]path` and thus are ignored.
`package.[c]path` for that being a semicolon-separated list and thus are
ignored.

This comment has been minimized.

@aktau

aktau May 25, 2017

Member

Perhaps:

paths from 'runtimepath' which contain semicolons are not added to package.[c]path, because those use the semicolon as a path separator.

@aktau

This comment has been minimized.

Member

aktau commented May 25, 2017

Yes! If they understand. What's so wrong with adding a note to the docs? Is this tiny amount of text going to inconvenience everyone?

There are no such notes in other places.

Correct, though that doesn't mean it's the right course of action. Perhaps it is very clear how to do this with other languages?

...Also there already is an example of the plugin which is using plugin-specific naming.

Yes, I consider the example in the docs you added a very good thing.

Also the correct place to put such notes are sections like :h write-plugin and not if_lua.txt.

Hmmm, maybe. However, that section does not seem to deal with anything except for standard VimL plugins. I'm not sure if a Vim/Lua programmer will be inclined to pore through those pages and decide that they should follow the same methodology (if described there) in their Lua code organization.

Moreover, I had no idea write-plugin even existed, while I did know about if-python and if-lua. This may just be my shortcoming though.

Whether it's correct to place it in if_lua.txt depends entirely on whether Lua is the same as most (all?) other plugin languages for Neovim with respect to module loading. If Lua is special, it deserves to be in if_lua.txt or Lua should be called out especially in write-plugin.

By the way, :h write-plugin is highly outdated (still suggest 8-character names), shows bad practices (:abbrev in place of :noreabbrev, uses :map (in some of the examples no nore is OK, but not :map, should be something more specific like :nmap)), not talks about VCS (who needs “Last change” in header when there is something as wonderful as mercurial?).

Yes, perhaps someone should clean that up. I had never read it though.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 25, 2017

Hmmm, maybe. However, that section does not seem to deal with anything except for standard VimL plugins. I'm not sure if a Vim/Lua programmer will be inclined to pore through those pages and decide that they should follow the same methodology (if described there) in their Lua code organization.

“Like :h write-plugin”, not “in :h write-plugin”. Though I am actually not aware of such sections for other languages and lua is now always present in Neovim making it completely possible to write such documentation exactly in :h write-plugin, and that could contain a note like “when writing your plugin note that 1. place where plugin is put by user may be a place where other plugins are also put, this is a regular behaviour of system plugin managers and 2. place where plugin is put by user may be read-only”. The 1. should then be expanded “this means that all files you create should either start with your plugin’s name or be inside subfolder named like your plugin”.

@aktau

This comment has been minimized.

Member

aktau commented May 26, 2017

  1. should then be expanded “this means that all files you create should either start with your plugin’s name or be inside subfolder named like your plugin”.

Indeed, that would be a very good thing, in my opinion.

As you say, most (experienced) Vim plugin developers will know this. Newer developers may not know this, and we can save them the trouble of the eventual bug reports with just a couple of lines of documentation.

Actually, this "happened" to me. I was trying out making a Lua plugin for Neovim. Naturally, I named the files after what they did:

./lua/refresh.lua (refreshes a ctags file)
./lua/highlight.lua (highlights words based on a ctags file)

These are pretty generic names, and I wouldn't be surprised if some other hapless soul named their own Lua files something similar at some point.

@aktau

aktau approved these changes May 26, 2017

2. It finds `?`-containing suffixes `/?.so`, `/a?d/j/g.elf` and `?.so`, in
order: parts of the path starting from the first path component containing
question mark and preceding path separator.
3. In the example last suffix is not unique so it is out, leaving only `/?.so`

This comment has been minimized.

@aktau

aktau May 26, 2017

Member

I'd change point (3) a little:

  1. The suffix of /def/?.so, namely /?.so is not unique, as it's the same as the suffix of the first path from package.path (i.e.: ./?.so). Which leaves /?.so and /a?d/j/g.elf, in this order.
and `/a?d/j/g.elf` in this order.
4. 'runtimepath' has three paths: `/foo/bar`, `/xxx;yyy/baz` and `/abc`.
Second one contains semicolon which is a paths separator so it is out,
leaving only `/foo/bar` and `/abc`, in order.

This comment has been minimized.

@aktau

aktau May 26, 2017

Member
  1. 'runtimepath' has three paths: /foo/bar, /xxx;yyy/baz and /abc.
    The second path contains a semicolon which is a path separator so it is ignored,
    leaving only /foo/bar and /abc, in order.
- `/foo/bar/lua/a?d/j/g.elf`
- `/abc/lua/?.so`
- `/abc/lua/a?d/j/g.elf`

This comment has been minimized.

@aktau

aktau May 26, 2017

Member
  1. The cartesian product of the paths from (4) and the unique suffixes determined in (3) is taken. The /lua path segment is inserted between that paths and the suffixes. The result:
Note: to keep up with 'runtimepath' changes paths prepended at previous
`package.path` or `package.cpath` update will not be prepended on next thus
allowing to delete directories from `package.path`/`package.cpath` when
deleting directories from 'runtimepath'.

This comment has been minimized.

@aktau

aktau May 26, 2017

Member

I would just say: if paths are removed from runtimepath, they will also be removed from the package.[c]path. Likewise if paths are added.

Note 3: removing paths with semicolons applies both to `package.path` and
`package.cpath`. Given that there is a number of badly written plugins using
shell which will not work with paths with semicolons it is better to not have
them in 'runtimepath' at all.

This comment has been minimized.

@aktau

aktau May 26, 2017

Member

Instead of:

Note 3: removing paths with ...

Perhaps:

Note 3: Skipping paths containing semicolons from &runtimepath...

@ZyX-I ZyX-I force-pushed the ZyX-I:lua-path branch from 9ab9a85 to 9da7571 May 26, 2017

@aktau

This comment has been minimized.

Member

aktau commented May 28, 2017

Not sure how things are done at the moment with the Github built-in LGTM or an actual comment saying LGTM, but: this change LGTM. Thanks @ZyX-I.

@ZyX-I ZyX-I force-pushed the ZyX-I:lua-path branch from 9da7571 to c06197f May 28, 2017

@ZyX-I ZyX-I force-pushed the ZyX-I:lua-path branch from c06197f to cab3a24 May 28, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented May 29, 2017

CI passed.

hkupty added a commit to Vigemus/nvimux that referenced this pull request May 31, 2017

@marvim marvim added the RDY label Jun 18, 2017

@justinmk justinmk merged commit f34befe into neovim:master Jun 27, 2017

4 checks passed

QuickBuild Build pr-6789 finished with status SUCCESSFUL
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 75.866%
Details

@justinmk justinmk removed the RDY label Jun 27, 2017

@ZyX-I ZyX-I deleted the ZyX-I:lua-path branch Jun 27, 2017

@cweagans cweagans referenced this pull request Oct 10, 2017

Merged

[RFC] News #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment