Some luvit-loader fixes/improvements #191
Conversation
- Windows root is 2 characters, so the #dir < 2 check would fail over and over (pathJoin("C:", "..") = "C:")
- Mimics default Lua behavior (require('module-name') would make module-name.lua print 'module-name' if its contents were `print(...)`)
e.g. pcall(require, "module-name") will now behave the same as require('module-name')
| @@ -183,7 +183,7 @@ local function loader(dir, path, bundleOnly) | |||
| try(pathJoin(dir, "libs", path)) then | |||
| break | |||
| end | |||
| if #dir < 2 then | |||
| if dir == pathJoin(dir, "..") then | |||
There was a problem hiding this comment.
Interesting, what issue is this working around or fixing?
There was a problem hiding this comment.
See this commit message:
Fix infinite loop when a module is not found on Windows
- Windows root is 2 characters, so the #dir < 2 check would fail over and over (pathJoin("C:", "..") = "C:")
|
Another thing I just realized: require automatically adds a key to package.loaded using the module name that is passed in, which can cause some weirdness. For example: -- /usr/main.lua
local r = require("./relative.lua")
-- luvit-loader will set package.loaded['/usr/relative.lua'] and
-- require will set package.loaded['./relative.lua'] to the return value of the loaded file
assert(package.loaded['./relative.lua'] == package.loaded['/usr/relative.lua'])
require("sub.test")-- /usr/sub/test.lua
local r = require("./relative.lua")
-- r will be the cached value of package.loaded['./relative.lua'],
-- not the return value of loading /usr/sub/relative.lua
assert(package.loaded['./relative.lua'] == package.loaded['/usr/relative.lua'])
assert(package.loaded['/usr/sub/relative.lua'] == nil) |
|
Yes, the package.loaded insertion weirdness is why I insert this loader at the front of the list, even before the one that looks in package.loaded. |
|
As far as I can tell, that lookup is done in require itself, not in a loader, see https://github.com/lua/lua/blob/5.1.x/src/loadlib.c#L454-L461 |
|
That's what I thought as well, but when I was testing it, I did find that by inserting my loader first, it somehow got around the problem. It's been a while so I may be remembering something wrong. |
|
Did you still want to merge this in. Sorry I got distracted with travels. |
|
Yeah, I think this is good to go. The '@' prefix when loading bundled dependencies would require a larger rewrite and potentially require changes to luvi (since it also uses the 'bundle:' prefix). |
Fixes for some some issues I ran into while making luver.
Let me know if you'd like any more information about any of the changes or if you see a better way to handle any of these issues.