go: fix cgo bugs preventing kafka module from working with cgo #17592
Comments
|
What version of Pants are you using? You will need to upgrade to a version of Pants with cgo support. |
|
Probably the latest 2.15.0 alpha release. I do not believe the cgo support landed on |
|
Going to close as completed. @clsx524 please let us know if you have a chance to try out the new Cgo support in 2.15 and if you have any questions! |
|
my use case is related to this package and it has a header file to select rdkafka. It provides a local version of it under the same directory or I can install it separately. When I used it with pants 2.15.0a1, it complained Can you provide some suggestions on how to use it with pants? |
|
I reproduced the failure locally. There are some bugs in the Pants cgo support to fix related to what files are inferred to be in a package and how references to "SRCDIR" in include and link flags in the kafka source code are resolved by Pants. |
tdyas
changed the title
|
#17728 fixes part of this issue by making all files in a module's sources available to package compilation as packages are permitted to expect. I still need to solve how |
## Problem As reported in #17592, third-party Go modules are allowed to assume that all files packaged with the module will be visible during compilation. In that particular issue, the [Confluent Kafka client library](https://github.com/confluentinc/confluent-kafka-go) uses Cgo and expected a subdirectory with files used during C compilation will be visible to the package using Cgo. Pants, however, subsets the modules sources digest to just the files in the exact subdirectory for each of the module's packages. This means that the [subdirectory files](https://github.com/confluentinc/confluent-kafka-go/tree/master/kafka/librdkafka_vendor) were not visible when compiling [the package in confluent-kafka-go in the immediate parent directory](https://github.com/confluentinc/confluent-kafka-go/tree/master/kafka/). ## Solution Since Pants cannot predict how a third party package will use the files in the modules sources, Pants cannot reasonably subset the modules source digest. Solve the issue by not subsetting the module sources digest to produce a package-specific digest. This is a partial fix for #17592.
The `go-export-cgo-codegen` debug goal was unnecessarily limited to exporting just Cgo codegen for first-party `go_package` targets. As part of fixing #17592, it is useful to also be able to dump Cgo codegen for any third-party packages using Cgo. This PR teaches `go-export-cgo-codegen` how to do that.
|
I was able to get a simple Kafka client example to link successfully in this branch when run with |
|
Split out the removal of the |
Remove the notion of a separate `__obj__` directory (`obj_dir_path` in the code) for compiling object files (e.g., Cgo and assembly) and just generate and capture object files in the same directory as the sources they are built from.
The issue is that the user can configure references to the source directory in `-I` and other compiler/assembler options using the `${SRCDIR}` syntax. With a separate object directory, the rules would need additional code to either copy files around or duplicate `-I` and other options to account for both the original sources directory and the objects directory if code wanted to be agnostic to the location of files that it expected to be in the package directory.
The idea for an objects tmp directory originally came from the `go` toolchain. `go` copies source files into the objects directory to avoid the duplication problem mentioned above. It makes sense for the Go toolchain to do this because the original sources are the user's actual source repository. Writing temporary object files into the original repository would make for a bad user experience.
Pants, on the other hand, uses an execution sandbox and does not have the same issue about modifying the original repository during a build. Thus, it makes sense in Pants to avoid copying files and just put object files next to the sources in the execution sandbox they are built from. This means the Cgo rules can avoid having to parse and duplicate options in which `${SRCDIR}` is used.
This is a partial fix for #17592. It fixes errors due to the mishandling of `${SRCDIR}` by the Cgo rules in not duplicating options that had `${SRCDIR}` in them to account for both the original sources directory and the objects directory.
|
The final fix is in #17780. |
#17780) Some Go modules, e.g. https://github.com/confluentinc/confluent-kafka-go, try to link a static archive embedded in the module into the package archive using Cgo. If so, mark this package so that the module sources are included in the output digest for the build of this package. The need for this inclusion is assumed if the Cgo rules observe the `${SRCDIR}` replacement in any linker option. The `${SRCDIR}` string will be replaced with the string `__PANTS_SANDBOX_ROOT__` which will then be replaced during linking with the actual sandbox path. This is accomplished by overriding the "external linker" binary used by `go tool link`. As an additional benefit, this also allows Pants to directly control which external linker binary is actually used so the link `Process` can be properly invalidated if the external linker changes. Fixes #17592. With this, a simple client binary using https://github.com/confluentinc/confluent-kafka-go links and can be executed.
|
Thanks. I will have a try once there is a release. |
|
I verified it works on 2.16.0.dev3. Great work! |
Is your feature request related to a problem? Please describe.
currently pants doesn't support .h files in the repo and compilation would result in errors
for example, it would encounter this error when depending on this error. https://github.com/confluentinc/confluent-kafka-go/blob/master/kafka/glue_rdkafka.h
Describe the solution you'd like
support .h file
Describe alternatives you've considered
NA
Additional context
NA
The text was updated successfully, but these errors were encountered: