fix(stdlib): convert dots to path separators in require()#242
Merged
Conversation
Lua 5.3 §6.3 requires that the module-name dots be replaced with the
directory separator before substitution into each `?` slot of a
`package.path` template. The new VM was substituting `modname` verbatim,
so `require("foo.bar")` looked for `foo.bar.lua` instead of `foo/bar.lua`.
This broke downstream consumers that chain dotted requires (e.g. the
`luassert` bootstrap: `luassert/init.lua` → `require("luassert.assert")`).
The fix hoists a `String.replace(modname, ".", "/")` above the pattern
loop. The default `package.path` already hardcodes `/` as the separator,
so no new configuration surface is introduced.
Adds a regression test exercising the disk-load path with a dotted name;
prior tests only used single-segment module names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
require("foo.bar")was resolving to<package.path>/foo.bar.luainstead of<package.path>/foo/bar.lua. Lua 5.3 §6.3 (package.searchpath) requires that every.in the module name be replaced with the directory separator before substitution into the?slot of eachpackage.pathtemplate. The new VM was skipping that step, so any disk-loaded dotted require would look in the wrong place.find_module_file/2: hoistString.replace(modname, ".", "/")above the pattern loop and substitute the result. The defaultpackage.pathalready hardcodes/as the separator (lib/lua/vm/stdlib.ex:597), so this introduces no new configuration surface.package_test.exsonly used single-segment names, so the substitution path was unexercised.Downstream impact
Caught by a downstream
sidecarconsumer bumping to1.0.0-rc.0. Its bootstrap goesrequire("luassert")→luassert/init.lua→require("luassert.assert"), which is exactly the broken path. With this fix and a1.0.0-rc.1release, that consumer can bump.Out of scope (intentionally)
The investigation surfaced broader package-compliance gaps that are not part of this PR — keeping the fix minimal. Track separately if/when needed:
package.config(the 5-line config string) is not exposed.package.searchpath/2is not implemented (would share the same.→/logic; worth extracting a helper at that point).package.cpath/ C-loader semantics.lua53_tests/attrib.luasuite remains skipped — it depends onpackage.searchpathand C submodules in addition to this fix.Test plan
mix test test/lua/vm/stdlib/package_test.exs— new dotted-require test passesmix test— full suite: 1,848 passed, 30 skipped (no new skips, no regressions){:lua, github: "tv-labs/lua", branch: "fix/require-dotted-module-name"}) and confirm itsluassert-chained requires resolve