fix(runtime): don't use regexes inside lua require'mod'

bfredl · bfredl · commit ea2023f689ad · 2021-10-17T16:21:42.000+02:00
Fixes #15147 and fixes #15497. Also sketch "subdir" caching. Currently this only caches whether an rtp entry has a "lua/" subdir but we could consider cache other subdirs potentially or even "lua/mybigplugin/" possibly. Note: the async_leftpad test doesn't actually fail on master, at least not deterministically (even when disabling the fast_breakcheck throttling). It's still useful as a regression test for further changes and included as such.
diff --git a/src/nvim/api/keysets.lua b/src/nvim/api/keysets.lua
@@ -48,5 +48,8 @@ return {
     "style";
     "noautocmd";
   };
+  runtime = {
+    "is_lua";
+  };
 }
 
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
@@ -756,6 +756,11 @@ ArrayOf(String) nvim_list_runtime_paths(Error *err)
   return nvim_get_runtime_file(NULL_STRING, true, err);
 }
 
+Array nvim__runtime_inspect(void)
+{
+  return runtime_inspect();
+}
+
 /// Find files in runtime directories
 ///
 /// 'name' can contain wildcards. For example
@@ -794,6 +799,25 @@ String nvim__get_lib_dir(void)
   return cstr_as_string(get_lib_dir());
 }
 
+/// Find files in runtime directories
+///
+/// @param pat pattern of files to search for
+/// @param all whether to return all matches or only the first
+/// @param options
+///          is_lua: only search lua subdirs
+/// @return list of absolute paths to the found files
+ArrayOf(String) nvim__get_runtime(Array pat, Boolean all, Dict(runtime) *opts, Error *err)
+  FUNC_API_SINCE(8)
+  FUNC_API_FAST
+{
+  bool is_lua = api_object_to_bool(opts->is_lua, "is_lua", false, err);
+  if (ERROR_SET(err)) {
+    return (Array)ARRAY_DICT_INIT;
+  }
+  return runtime_get_named(is_lua, pat, all);
+}
+
+
 /// Changes the global working directory.
 ///
 /// @param dir      Directory path
diff --git a/src/nvim/generators/gen_api_dispatch.lua b/src/nvim/generators/gen_api_dispatch.lua
@@ -419,7 +419,7 @@ local function process_function(fn)
 
   if not fn.fast then
     write_shifted_output(output, string.format([[
-    if (!nlua_is_deferred_safe(lstate)) {
+    if (!nlua_is_deferred_safe()) {
       return luaL_error(lstate, e_luv_api_disabled, "%s");
     }
     ]], fn.name))
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
@@ -790,7 +790,7 @@ int nlua_call(lua_State *lstate)
   Error err = ERROR_INIT;
   size_t name_len;
   const char_u *name = (const char_u *)luaL_checklstring(lstate, 1, &name_len);
-  if (!nlua_is_deferred_safe(lstate)) {
+  if (!nlua_is_deferred_safe()) {
     return luaL_error(lstate, e_luv_api_disabled, "vimL function");
   }
 
@@ -846,7 +846,7 @@ int nlua_call(lua_State *lstate)
 
 static int nlua_rpcrequest(lua_State *lstate)
 {
-  if (!nlua_is_deferred_safe(lstate)) {
+  if (!nlua_is_deferred_safe()) {
     return luaL_error(lstate, e_luv_api_disabled, "rpcrequest");
   }
   return nlua_rpc(lstate, true);
@@ -1316,7 +1316,7 @@ Object nlua_call_ref(LuaRef ref, const char *name, Array args, bool retval, Erro
 
 /// check if the current execution context is safe for calling deferred API
 /// methods. Luv callbacks are unsafe as they are called inside the uv loop.
-bool nlua_is_deferred_safe(lua_State *lstate)
+bool nlua_is_deferred_safe(void)
 {
   return in_fast_callback == 0;
 }
diff --git a/src/nvim/lua/vim.lua b/src/nvim/lua/vim.lua
@@ -57,28 +57,29 @@ end
 function vim._load_package(name)
   local basename = name:gsub('%.', '/')
   local paths = {"lua/"..basename..".lua", "lua/"..basename.."/init.lua"}
-  for _,path in ipairs(paths) do
-    local found = vim.api.nvim_get_runtime_file(path, false)
-    if #found > 0 then
-      local f, err = loadfile(found[1])
-      return f or error(err)
-    end
+  local found = vim.api.nvim__get_runtime(paths, false, {is_lua=true})
+  if #found > 0 then
+    local f, err = loadfile(found[1])
+    return f or error(err)
   end
 
+  local so_paths = {}
   for _,trail in ipairs(vim._so_trails) do
     local path = "lua"..trail:gsub('?', basename) -- so_trails contains a leading slash
-    local found = vim.api.nvim_get_runtime_file(path, false)
-    if #found > 0 then
-      -- Making function name in Lua 5.1 (see src/loadlib.c:mkfuncname) is
-      -- a) strip prefix up to and including the first dash, if any
-      -- b) replace all dots by underscores
-      -- c) prepend "luaopen_"
-      -- So "foo-bar.baz" should result in "luaopen_bar_baz"
-      local dash = name:find("-", 1, true)
-      local modname = dash and name:sub(dash + 1) or name
-      local f, err = package.loadlib(found[1], "luaopen_"..modname:gsub("%.", "_"))
-      return f or error(err)
-    end
+    table.insert(so_paths, path)
+  end
+
+  found = vim.api.nvim__get_runtime(so_paths, false, {is_lua=true})
+  if #found > 0 then
+    -- Making function name in Lua 5.1 (see src/loadlib.c:mkfuncname) is
+    -- a) strip prefix up to and including the first dash, if any
+    -- b) replace all dots by underscores
+    -- c) prepend "luaopen_"
+    -- So "foo-bar.baz" should result in "luaopen_bar_baz"
+    local dash = name:find("-", 1, true)
+    local modname = dash and name:sub(dash + 1) or name
+    local f, err = package.loadlib(found[1], "luaopen_"..modname:gsub("%.", "_"))
+    return f or error(err)
   end
   return nil
 end
diff --git a/src/nvim/runtime.c b/src/nvim/runtime.c
@@ -11,6 +11,7 @@
 #include "nvim/eval.h"
 #include "nvim/ex_cmds.h"
 #include "nvim/ex_cmds2.h"
+#include "nvim/lua/executor.h"
 #include "nvim/misc1.h"
 #include "nvim/option.h"
 #include "nvim/os/os.h"
@@ -157,6 +158,34 @@ int do_in_path(char_u *path, char *name, int flags, DoInRuntimepathCB callback,
   return did_one ? OK : FAIL;
 }
 
+RuntimeSearchPath runtime_search_path_get_cached(int *ref)
+  FUNC_ATTR_NONNULL_ALL
+{
+  runtime_search_path_validate();
+
+  *ref = 0;
+  if (runtime_search_path_ref == NULL) {
+    // cached path was unreferenced. keep a ref to
+    // prevent runtime_search_path() to freeing it too early
+    (*ref)++;
+    runtime_search_path_ref = ref;
+  }
+  return runtime_search_path;
+}
+
+void runtime_search_path_unref(RuntimeSearchPath path, int *ref)
+  FUNC_ATTR_NONNULL_ALL
+{
+  if (*ref) {
+    if (runtime_search_path_ref == ref) {
+      runtime_search_path_ref = NULL;
+    } else {
+      runtime_search_path_free(path);
+    }
+  }
+}
+
+
 /// Find the file "name" in all directories in "path" and invoke
 /// "callback(fname, cookie)".
 /// "name" can contain wildcards.
@@ -167,7 +196,6 @@ int do_in_path(char_u *path, char *name, int flags, DoInRuntimepathCB callback,
 /// return FAIL when no file could be sourced, OK otherwise.
 int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void *cookie)
 {
-  runtime_search_path_validate();
   char_u *tail;
   int num_files;
   char_u **files;
@@ -182,14 +210,8 @@ int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void
     verbose_leave();
   }
 
-  RuntimeSearchPath path = runtime_search_path;
-  int ref = 0;
-  if (runtime_search_path_ref == NULL) {
-    // cached path was unreferenced. keep a ref to
-    // prevent runtime_search_path() to freeing it too early
-    ref++;
-    runtime_search_path_ref = &ref;
-  }
+  int ref;
+  RuntimeSearchPath path = runtime_search_path_get_cached(&ref);
 
   // Loop over all entries in cached path
   for (size_t j = 0; j < kv_size(path); j++) {
@@ -206,7 +228,6 @@ int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void
 
     if (name == NULL) {
       (*callback)((char_u *)item.path, cookie);
-      did_one = true;
     } else if (buflen + STRLEN(name) + 2 < MAXPATHL) {
       STRCPY(buf, item.path);
       add_pathsep((char *)buf);
@@ -255,17 +276,70 @@ int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void
     }
   }
 
-  if (ref) {
-    if (runtime_search_path_ref == &ref) {
-      runtime_search_path_ref = NULL;
-    } else {
-      runtime_search_path_free(path);
+  runtime_search_path_unref(path, &ref);
+
+  return did_one ? OK : FAIL;
+}
+
+Array runtime_inspect(void)
+{
+  RuntimeSearchPath path = runtime_search_path;
+  Array rv = ARRAY_DICT_INIT;
+
+  for (size_t i = 0; i < kv_size(path); i++) {
+    SearchPathItem *item = &kv_A(path, i);
+    Array entry = ARRAY_DICT_INIT;
+    ADD(entry, STRING_OBJ(cstr_to_string(item->path)));
+    ADD(entry, BOOLEAN_OBJ(item->after));
+    if (item->has_lua != kNone) {
+      ADD(entry, BOOLEAN_OBJ(item->has_lua == kTrue));
     }
+    ADD(rv, ARRAY_OBJ(entry));
   }
+  return rv;
+}
 
+ArrayOf(String) runtime_get_named(bool lua, Array pat, bool all)
+{
+  int ref;
+  RuntimeSearchPath path = runtime_search_path_get_cached(&ref);
+  ArrayOf(String) rv = ARRAY_DICT_INIT;
+  static char buf[MAXPATHL];
 
-  return did_one ? OK : FAIL;
+  for (size_t i = 0; i < kv_size(path); i++) {
+    SearchPathItem *item = &kv_A(path, i);
+    if (lua) {
+      if (item->has_lua == kNone) {
+        size_t size = (size_t)snprintf(buf, sizeof buf, "%s/lua/", item->path);
+        item->has_lua = (size < sizeof buf && os_isdir((char_u *)buf)) ? kTrue : kFalse;
+      }
+      if (item->has_lua == kFalse) {
+        continue;
+      }
+    }
+
+    for (size_t j = 0; j < pat.size; j++) {
+      Object pat_item = pat.items[j];
+      if (pat_item.type == kObjectTypeString) {
+        size_t size = (size_t)snprintf(buf, sizeof buf, "%s/%s",
+                                       item->path, pat_item.data.string.data);
+        if (size < sizeof buf) {
+          if (os_file_is_readable(buf)) {
+            ADD(rv, STRING_OBJ(cstr_to_string(buf)));
+            if (!all) {
+              goto done;
+            }
+          }
+        }
+      }
+    }
+  }
+
+done:
+  runtime_search_path_unref(path, &ref);
+  return rv;
 }
+
 /// Find "name" in "path".  When found, invoke the callback function for
 /// it: callback(fname, "cookie")
 /// When "flags" has DIP_ALL repeat for all matches, otherwise only the first
@@ -338,7 +412,7 @@ static void push_path(RuntimeSearchPath *search_path, Map(String, handle_T) *rtp
   if (h == 0) {
     char *allocated = xstrdup(entry);
     map_put(String, handle_T)(rtp_used, cstr_as_string(allocated), 1);
-    kv_push(*search_path, ((SearchPathItem){ allocated, after }));
+    kv_push(*search_path, ((SearchPathItem){ allocated, after, kNone }));
   }
 }
 
@@ -481,6 +555,13 @@ void runtime_search_path_free(RuntimeSearchPath path)
 
 void runtime_search_path_validate(void)
 {
+  if (!nlua_is_deferred_safe()) {
+    // Cannot rebuild search path in an async context. As a plugin will invoke
+    // itself asynchronously from sync code in the same plugin, the sought
+    // after lua/autoload module will most likely already be in the cached path.
+    // Thus prefer using the stale cache over erroring out in this situation.
+    return;
+  }
   if (!runtime_search_path_valid) {
     if (!runtime_search_path_ref) {
       runtime_search_path_free(runtime_search_path);
diff --git a/src/nvim/runtime.h b/src/nvim/runtime.h
@@ -10,6 +10,7 @@ typedef void (*DoInRuntimepathCB)(char_u *, void *);
 typedef struct {
   char *path;
   bool after;
+  TriState has_lua;
 } SearchPathItem;
 
 typedef kvec_t(SearchPathItem) RuntimeSearchPath;
diff --git a/test/functional/core/startup_spec.lua b/test/functional/core/startup_spec.lua
@@ -328,6 +328,15 @@ describe('startup', function()
     eq({9003, '\thowdy'}, exec_lua [[ return { _G.y, _G.z } ]])
   end)
 
+  it("handles require from &packpath in an async handler", function()
+      -- NO! you cannot just speed things up by calling async functions during startup!
+      -- It doesn't make anything actually faster! NOOOO!
+    pack_clear [[ lua require'async_leftpad'('brrrr', 'async_res') ]]
+
+    -- haha, async leftpad go brrrrr
+    eq('\tbrrrr', exec_lua [[ return _G.async_res ]])
+  end)
+
   it("handles :packadd during startup", function()
     -- control group: opt/bonus is not availabe by default
     pack_clear [[
diff --git a/test/functional/fixtures/pack/foo/start/fancyplugin/after/lua/fancy_y.lua b/test/functional/fixtures/pack/foo/start/fancyplugin/after/lua/fancy_y.lua
@@ -0,0 +1 @@
+return "I am fancy_y.lua"
diff --git a/test/functional/fixtures/pack/foo/start/fancyplugin/after/lua/fancy_z.lua b/test/functional/fixtures/pack/foo/start/fancyplugin/after/lua/fancy_z.lua
@@ -0,0 +1 @@
+return "I am fancy_z.lua"
diff --git a/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_x.lua b/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_x.lua
@@ -0,0 +1 @@
+return "I am fancy_x.lua"
diff --git a/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_x/init.lua b/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_x/init.lua
@@ -0,0 +1 @@
+return "I am init.lua of fancy_x!"
diff --git a/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_y/init.lua b/test/functional/fixtures/pack/foo/start/fancyplugin/lua/fancy_y/init.lua
@@ -0,0 +1,2 @@
+
+return "I am init.lua of fancy_y!"
diff --git a/test/functional/fixtures/start/nvim-leftpad/lua/async_leftpad.lua b/test/functional/fixtures/start/nvim-leftpad/lua/async_leftpad.lua
@@ -0,0 +1,3 @@
+return function (val, res)
+  vim.loop.new_async(function() _G[res] = require'leftpad'(val) end):send()
+end
diff --git a/test/functional/lua/vim_spec.lua b/test/functional/lua/vim_spec.lua
@@ -2255,7 +2255,7 @@ end)
 
 describe('lua: require("mod") from packages', function()
   before_each(function()
-    command('set rtp+=test/functional/fixtures')
+    command('set rtp+=test/functional/fixtures pp+=test/functional/fixtures')
   end)
 
   it('propagates syntax error', function()
@@ -2266,4 +2266,13 @@ describe('lua: require("mod") from packages', function()
 
     matches("unexpected symbol", syntax_error_msg)
   end)
+
+  it('uses the right order of mod.lua vs mod/init.lua', function()
+    -- lua/fancy_x.lua takes precedence over lua/fancy_x/init.lua
+    eq('I am fancy_x.lua', exec_lua [[ return require'fancy_x' ]])
+    -- but lua/fancy_y/init.lua takes precedence over after/lua/fancy_y.lua
+    eq('I am init.lua of fancy_y!', exec_lua [[ return require'fancy_y' ]])
+    -- safety check: after/lua/fancy_z.lua is still loaded
+    eq('I am fancy_z.lua', exec_lua [[ return require'fancy_z' ]])
+  end)
 end)

Original file line number	Diff line number	Diff line change
`@@ -48,5 +48,8 @@ return {`
`48`	`48`	`"style";`
`49`	`49`	`"noautocmd";`
`50`	`50`	`};`
	`51`	`+ runtime = {`
	`52`	`+ "is_lua";`
	`53`	`+ };`
`51`	`54`	`}`
`52`	`55`
Original file line number	Diff line number	Diff line change
`@@ -419,7 +419,7 @@ local function process_function(fn)`
`419`	`419`
`420`	`420`	`if not fn.fast then`
`421`	`421`	`write_shifted_output(output, string.format([[`
`422`		`- if (!nlua_is_deferred_safe(lstate)) {`
	`422`	`+ if (!nlua_is_deferred_safe()) {`
`423`	`423`	`return luaL_error(lstate, e_luv_api_disabled, "%s");`
`424`	`424`	`}`
`425`	`425`	`]], fn.name))`
Original file line number	Diff line number	Diff line change
`@@ -790,7 +790,7 @@ int nlua_call(lua_State *lstate)`
`790`	`790`	`Error err = ERROR_INIT;`
`791`	`791`	`size_t name_len;`
`792`	`792`	`const char_u name = (const char_u )luaL_checklstring(lstate, 1, &name_len);`
`793`		`- if (!nlua_is_deferred_safe(lstate)) {`
	`793`	`+ if (!nlua_is_deferred_safe()) {`
`794`	`794`	`return luaL_error(lstate, e_luv_api_disabled, "vimL function");`
`795`	`795`	`}`
`796`	`796`
`@@ -846,7 +846,7 @@ int nlua_call(lua_State *lstate)`
`846`	`846`
`847`	`847`	`static int nlua_rpcrequest(lua_State *lstate)`
`848`	`848`	`{`
`849`		`- if (!nlua_is_deferred_safe(lstate)) {`
	`849`	`+ if (!nlua_is_deferred_safe()) {`
`850`	`850`	`return luaL_error(lstate, e_luv_api_disabled, "rpcrequest");`
`851`	`851`	`}`
`852`	`852`	`return nlua_rpc(lstate, true);`
`@@ -1316,7 +1316,7 @@ Object nlua_call_ref(LuaRef ref, const char *name, Array args, bool retval, Erro`
`1316`	`1316`
`1317`	`1317`	`/// check if the current execution context is safe for calling deferred API`
`1318`	`1318`	`/// methods. Luv callbacks are unsafe as they are called inside the uv loop.`
`1319`		`-bool nlua_is_deferred_safe(lua_State *lstate)`
	`1319`	`+bool nlua_is_deferred_safe(void)`
`1320`	`1320`	`{`
`1321`	`1321`	`return in_fast_callback == 0;`
`1322`	`1322`	`}`