dynamic modules: add abi.h to go mod vendor tree#44395
dynamic modules: add abi.h to go mod vendor tree#44395agrawroh merged 2 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Adam Anderson <6754028+AdamEAnderson@users.noreply.github.com>
AdamEAnderson
requested review from
agrawroh,
mathetake,
mattklein123 and
wbpcode
as code owners
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a placeholder Go file, abi.go, to the source/extensions/dynamic_modules/abi/ directory to ensure that the directory and its associated C ABI header are included during Go vendoring. A review comment suggests renaming this file to doc.go to better align with Go's idiomatic practices for package documentation files.
| // Package abi contains the C ABI header for Envoy dynamic modules. | ||
| // | ||
| // This file exists so that `go mod vendor` includes this directory. | ||
| // Without a .go file, the Go toolchain skips this directory during | ||
| // vendoring, which causes the abi.h header (needed by the Go SDK's | ||
| // cgo include) to be missing from the vendor tree. | ||
| package abi |
There was a problem hiding this comment.
To align with Go's idiomatic practices, it is recommended to name files that solely contain package documentation and a package clause as doc.go. This convention makes the file's purpose—to ensure the directory is treated as a Go package—immediately clear to other developers.
Consider renaming abi.go to doc.go.
agrawroh
left a comment
agrawroh
left a comment
There was a problem hiding this comment.
@AdamEAnderson Could you please address the comment on renaming the file to doc.go. I think it make sense as we only have the Dummy ABI file.
addressed, thanks for the review |
Commit Message: add abi.h to go mod vendor tree Additional Description: go mod vendor is a common way in Golang to import modules/packages. It works by downloading and storing the locked version of a dependency in a vendor directory within the module. Vendoring skips downloading and storing all directories that do not have Go source files. This is a problem because the Go dynamic modules SDK uses a CGO preamble with an import of this header file, but this header is missing from the vendor download. This change adds a dummy Go file that will make Go treat this directory as a valid Go package and thus let it be imported so the header will exist properly. Risk Level: none Testing: n/a Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a --------- Signed-off-by: Adam Anderson <6754028+AdamEAnderson@users.noreply.github.com>
2 participants
Commit Message: add abi.h to go mod vendor tree
Additional Description: go mod vendor is a common way in Golang to import modules/packages. It works by downloading and storing the locked version of a dependency in a vendor directory within the module. Vendoring skips downloading and storing all directories that do not have Go source files. This is a problem because the Go dynamic modules SDK uses a CGO preamble with an import of this header file, but this header is missing from the vendor download. This change adds a dummy Go file that will make Go treat this directory as a valid Go package and thus let it be imported so the header will exist properly.
Risk Level: none
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a