starwing / lua-protobuf

A Lua module to work with Google protobuf
MIT License
1.71k stars 388 forks source link

unknown hook works only when attached to module #231

Closed alonbg closed 1 year ago

alonbg commented 1 year ago

Problem: loading this envoy proto

With the following environment & prerequisites: LuaJIT 2.1.0-beta3 -- Copyright (C) 2005-2022 Mike Pall. https://luajit.org/ lua-protobuf - latest commit as of this writing

PROTO_PREFIX =  "/etc/proto/"

local protoc_import_paths = {
    ".",
    PROTO_PREFIX .. "envoy/api", -- https://github.com/envoyproxy/envoy
    PROTO_PREFIX .. "xds", -- https://github.com/cncf/xds.git
    PROTO_PREFIX .. "protoc-gen-validate",  -- https://github.com/bufbuild/protoc-gen-validate
    "/usr/local/include" -- https://github.com/google/protobuf
}

fails on

`envoy/config/core/v3/base.proto:98:3: unknown type 'type.v3.SemanticVersion'` 
`envoy/config/core/v3/base.proto:273:3: unknown type 'type.v3.Percent'`
`envoy/config/core/v3/base.proto:496:3: unknown type 'type.v3.FractionalPercent'`
`/etc/proto/envoy/api/envoy/service/ext_proc/v3/external_processor.proto:183:3: unknown type 'config.core.v3.HeaderMap'`
`/etc/proto/envoy/api/envoy/service/ext_proc/v3/external_processor.proto:288:3: unknown type 'type.v3.HttpStatus'`

I realized that this is solved by prefixing these types with .envoy. Thus I created this hook

local unknown_type = function(self, name)    
        if name == "type.v3.SemanticVersion"
           or name == "type.v3.Percent"
           or name == "type.v3.FractionalPercent"
           or name == "config.core.v3.HeaderMap"
           or "type.v3.HttpStatus" then
            return ".envoy." .. name, "message"
        end
    end

proto_inst.unknown_type = unknown_type

However, it still errored. It workes only when hook is attached to the module itself (i.e. protoc )

local protoc = require("protoc")
protoc.unknown_type = unknown_type

Any ideas ?

starwing commented 1 year ago

I have made a new commit for better unknown handling in proton.Lua, please try it out to see whether it solve your issue.

alonbg commented 1 year ago

@starwing Issue is indeed resolved workaround I used still works btw (since Parser is the metatable fallback, I presume ) and this is fine by me :) the following test failes:

pb predefined types: 33
......E.............
Failed tests:
-------------
1) test_extend
./protoc.lua:337: attempt to call local 'import_fallback' (a boolean value)
stack traceback:
        ./protoc.lua:511: in function 'top_parser'
        ./protoc.lua:836: in function 'f'
        ./protoc.lua:1152: in function 'do_compile'
        ./protoc.lua:1157: in function 'compile'
        ./protoc.lua:1169: in function 'load'
        test.lua:199: in function 'test_extend'

Ran 20 tests in 0.012 seconds, 19 successes, 1 error

replacing protoc.lua:337:

if import_fallback then
    info = import_fallback(self, name)
end

with

if import_fallback then
    info = import_fallback == true or import_fallback(self, name)
end

solves the issue. seems right ?

starwing commented 1 year ago

okay, thanks for the catch! Now the tests are passed.