lua-nginx-module icon indicating copy to clipboard operation
lua-nginx-module copied to clipboard

Add append_lua_package_path & append_lua_package_cpath directives

Open hongliang5316 opened this issue 8 years ago • 11 comments

Sometimes we need to add a new program (server), but we don't want to change the content of lua_package_path and lua_package_cpath. Because those may be maintained by other programs, so we need more independent directives to configure lua_path and lua_cpath. This can be used by this way:

http {
    append_lua_package_path "/opt/program1/?.lua;";
    append_lua_package_cpath "/opt/program1/?.so;";
    server {
        #...
    }
    append_lua_package_path "/opt/program2/?.lua;";
    append_lua_package_cpath "/opt/program2/?.so;";
    server {
        #...
    }
}

I hereby granted the copyright of the changes in this pull request to the authors of this lua-nginx-module project.

hongliang5316 avatar May 24 '17 15:05 hongliang5316

@hongliang5316 This is an interesting feature. Will you also add some corresponding tests to the existing test suite? Many thanks!

agentzh avatar May 24 '17 20:05 agentzh

@agentzh All fixed. And also add some test cases.

hongliang5316 avatar May 26 '17 07:05 hongliang5316

@thibaultcha @doujiang24 @bungle What do you think of this feature?

agentzh avatar May 29 '17 22:05 agentzh

I am not so sure about the use case behind this. Considering the possibilities provided by export LUA_PATH and ;;, we can:

Extend the LUA_PATH via the appropriate environment variable:

export LUA_PATH=./resty/?.lua;

Override package.path via the appropriate ngx_lua directive (what I think this PR tries to solve):

lua_package_path './lib/?.lua;';

Or, combine the OpenResty defaults and the env variable with ;; (what I think this PR overlooked?):

http {
    lua_package_path './lib.lua;;';

    init_by_lua_block {
        print(package.path)
    }
}

The latest will do what this PR does, and also include the LUA_PATH env variable for even more flexibility (see beginning and end of log):

2017/05/29 17:18:33 [notice] 3925#0: [lua] init_by_lua:2: ./lib.lua;/usr/local/Cellar/openresty/1.11.2.2/site/lualib/?.lua;/usr/local/Cellar/openresty/1.11.2.2/site/lualib/?/init.lua;/usr/local/Cellar/openresty/1.11.2.2/lualib/?.lua;/usr/local/Cellar/openresty/1.11.2.2/lualib/?/init.lua;./resty/?.lua;

And for the cases needing even more flexibility, we can also do this in the Lua land (which I don't think is even necessary for the use case proposed here):

package.path = "/opt/mylib/" .. package.path

Or did I misunderstand something that this PR is trying to solve?

thibaultcha avatar May 30 '17 00:05 thibaultcha

@thibaultcha Your comments are very good. But it doesn't apply to us. For multiple apps, which have their only lua_package_path, the lua_package_path directive and LUA_PATH environment variable can be used only once. That's mean every app will share this. For us, i think it's not independent enough. Suppose we have one app to maintain lua_package_path directive in nginx.conf, But when we add a new app, this must be changed (we will upgrade the app to be compatible with the new app). It seems like we can modify the package.path in the Lua land, but compared with this PR, i think it's not intuitive enough.

hongliang5316 avatar May 30 '17 11:05 hongliang5316

@hongliang5316 Sorry for my rude interruption... I am afraid that modifying the package.path in Nginx configuration is less intuitive than in the Lua land. Take an example like this:

http {
    append_lua_package_path "/opt/program1/?.lua;";
    server {
        # server A
    }
    append_lua_package_path "/opt/program2/?.lua;";
    server {
        # server B
    }
}

For guys not famous with Nginx configuration, server A and server B seem to use different lua_package_path. However, both server A and server B share the same lua_package_path.

Moreover, if we add a new server, called server C, with its own append_lua_package_path:

append_lua_package_path "/opt/program3/?.lua;";
server {
     # server C
}

Note that server A,B,C share the same lua_package_path. So the append_lua_package_path invited by server C not only changes its own lua_package_path, but also changes other servers' lua_package_path.

Assumed we only allow one app to maintain lua_package_path directive(or LUA_PATH environment variable), we are able to do some conflict detection(or hacks around package.loaded) in the entrance.

spacewander avatar May 30 '17 13:05 spacewander

@thibaultcha @spacewander Thank you for your feedback! Yeah, it seems to me now that it would be more elegant and more flexible to just manipulate the search paths on the Lua land for each OR application running in the same OR instance.

agentzh avatar May 30 '17 18:05 agentzh

@hongliang5316 Maybe we should just allow multiple init_by_lua* directive instances?

agentzh avatar May 30 '17 18:05 agentzh

@spacewander Your analysis is very good. This can lead to a conflict among apps. @agentzh Yeah, allowing multiple init_by_lua* directive instances is more versatile and flexible, i will work for this.

hongliang5316 avatar Jun 01 '17 05:06 hongliang5316

@hongliang5316 Great. Thanks! You can follow a similar style as this PR: #935.

agentzh avatar Jun 01 '17 23:06 agentzh

This pull request is now in conflict :(

mergify[bot] avatar Jun 26 '20 00:06 mergify[bot]