openresty / luajit2

OpenResty's Branch of LuaJIT 2
https://luajit.org/luajit.html
Other
1.2k stars 193 forks source link

feature: export the key_sentinel. #154

Closed zhuizhuhaomeng closed 2 years ago

zhuizhuhaomeng commented 2 years ago

fix https://github.com/openresty/luajit2/issues/153

XmiliaH commented 2 years ago

Why not use package.loaded[...] at the top level of resty/core/base.lua to get the sentinel? This would not require so many changes.

zhuizhuhaomeng commented 2 years ago

resty/core/base.lua

I have added the key_sentinel at package.loaded.package.key_sentinel before. but need to change another repo(https://github.com/openresty/luajit2-test-suite).

I think you you are right, I will change back to the original method.

XmiliaH commented 2 years ago

I have added the key_sentinel at package.loaded.package.key_sentinel before.

You do not need to add key_sentinel. When running on the top level of resty/core/base.lua it can be found in package.loaded['resty.core.base']. I used (...) since the first arg of the chunk is its name in the resty/core/base.lua case this would be resty.core.base.

zhuizhuhaomeng commented 2 years ago

@XmiliaH update, please review again

XmiliaH commented 2 years ago

I think the changes to luajit2 are not required. I would replace

local function my_require(name)
    local mod = pkg_loaded[name]
    if mod then
        return mod
    end
    return orig_require(name)
end

with

local key_sentinel = pkg_loaded[...]
local error = error

local function my_require(name)
    local mod = pkg_loaded[name]
    if mod then
        if mod == key_sentinel then
            error("loop or previous error loading module '" .. name .. "'")
        end
        return mod
    end
    return orig_require(name)
end

in resty/core/base.lua. This should fix the problem with less changes.

zhuizhuhaomeng commented 2 years ago

local key_sentinel = pkg_loaded[...] is nil. how would that work?

XmiliaH commented 2 years ago

local key_sentinel = pkg_loaded[...] is nil.

How did you test that? When I do: test.lua

print(package.loaded[...])

and run

luajit -e 'require("test")'

I get the sentinel printed as it is inserted in to package.loaded before the chunk is executed and replaced if the chunk completes normally. So during the execution of the chunk the sentinel can be found in the table.

zhuizhuhaomeng commented 2 years ago

I test with the following code.

luajit -e "print(package.loaded[...])"

but your code works when I replace it with mime and run the test case.

XmiliaH commented 2 years ago

Your test did not work since the package.loaded[...] only works in Lua files which are loaded via require, but this should be the case for resty/core/base.lua.

zhuizhuhaomeng commented 2 years ago

I test with the following code. I will close this PR.