Skip to content

Update the strip prefix logic for python libraries#361

Merged
pcj merged 2 commits into
stackb:masterfrom
vihangm:strip_prefix_py_import
Feb 14, 2024
Merged

Update the strip prefix logic for python libraries#361
pcj merged 2 commits into
stackb:masterfrom
vihangm:strip_prefix_py_import

Conversation

@vihangm
Copy link
Copy Markdown
Contributor

@vihangm vihangm commented Feb 14, 2024

The strip prefix logic to generate the imports for py_library rules was inaccurate.

Lets say you have a proto file at a/b/c/d/e/f.proto.
If stripPrefix is a/b/c then imports should be ../.. since that will
ascend the import dir from the current dir e two levels to c which will then
make from d.e import f_py_pb2 work.

This updates the broken logic to implement this correct logic and adds a test to assert the same.

Fixes #362

Signed-off-by: Vihang Mehta <vihang@gimletlabs.ai>
Signed-off-by: Vihang Mehta <vihang@gimletlabs.ai>
@vihangm vihangm force-pushed the strip_prefix_py_import branch from 62833c0 to cd3eff5 Compare February 14, 2024 06:43
@vihangm
Copy link
Copy Markdown
Contributor Author

vihangm commented Feb 14, 2024

@pcj Thanks for adding support for strip_import_prefix and python libraries in the latest release. I upgraded and tried to use the feature but noticed a bug with how the ImportAttr was being calculated. This PR fixes the same.

@pcj
Copy link
Copy Markdown
Member

pcj commented Feb 14, 2024

@vihangm Looks good. I don't personally use strip_import_prefix so the support has always been long delayed or incomplete. Appreciate your contribution.

@pcj pcj merged commit 4da55f5 into stackb:master Feb 14, 2024
@vihangm vihangm deleted the strip_prefix_py_import branch February 14, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

imports calculation for proto_py_library is broken

2 participants