Conversation
|
@DolceTriade @scheremisin @dgoel Could you test this? |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes rpath generation for CGO libraries by implementing comprehensive coverage of different executable and library location scenarios based on Bazel's C++ rules logic. The fix addresses issue #4451 by ensuring that rpaths are correctly generated for various deployment patterns including cross-repository dependencies.
- Updated rpath generation logic to handle all cases from Bazel's C++ rules
- Added comprehensive test coverage for CGO libraries with shared dependencies
- Removed custom
_rlocation_pathfunction in favor of directshort_pathusage
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go/private/rpath.bzl | Updated rpath generation logic to cover all cases based on rules_cc implementation |
| tests/core/cgo/wrapped_cgo_test.go | Added new test file for wrapped CGO functionality with shared libraries |
| tests/core/cgo/BUILD.bazel | Added test target for wrapped CGO test with platform compatibility |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thanks!! I removed our local hack and replaced it with this patch and our CI passed! Thanks for the fix, it looks like the logic for rpaths is quite involved 😄 |
This is based on the corresponding logic in rules_cc.
Commented out macOS compatibility for wrapped_cgo_test.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
This is based on the corresponding logic in rules_cc.
Which issues(s) does this PR fix?
Fixes #4451
Other notes for review