Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(proto): shim to upstream for py_proto_library #2581

Closed
wants to merge 9 commits into from

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jan 25, 2025

With this PR we attempt to remove our implementation of py_proto_library
and we just forward the calls to the upstream.

Hopefully this fixes #2543.
Work towards #2173.

With this PR we attempt to remove our implementation of py_proto_library
and we just forward the calls to the upstream.

Hopefully this fixes bazelbuild#2543.
@aignas
Copy link
Collaborator Author

aignas commented Jan 25, 2025

@tpudlik, did you have a chance to also submit the fix for #2515 to the upstream, i.e. the protocolbuffers org?

@aignas aignas marked this pull request as ready for review January 28, 2025 04:16
@aignas aignas requested a review from rickeylev as a code owner January 28, 2025 04:16
@rickeylev
Copy link
Collaborator

I noticed the proto version was updated -- does this mean 29.3 has all the parts we need, so we can put this shim in now?

@aignas
Copy link
Collaborator Author

aignas commented Jan 29, 2025

I think we could do it with the old version, but the fix that @tpudlik put in is not in the upstream, so I am wondering what we should do...

@aignas
Copy link
Collaborator Author

aignas commented Jan 29, 2025

Maybe we can provide users with a small patch before it gets picked up by the protocolbuffers team. I think it would be the most we could do from our side.

@tpudlik
Copy link
Contributor

tpudlik commented Jan 29, 2025

Yes, sorry, I've not contributed it yet! Sorry, it slipped my mind. I'll send a CL (PR) this week, but it will take a little while to make it into a released version, of course.

@rickeylev rickeylev added the need: upstream support An issue that needs changes in upstream code label Jan 30, 2025
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this pull request Feb 1, 2025
Previously, the import path within the runfiles was only correct for the case `--legacy_external_runfiles=True` (which copied the runfiles into `$RUNFILES/<main repo>/external/<external repo>/<path>` in addition to `$RUNFILES/<external repo>/<path>`. This flag was flipped to False in Bazel 8.0.0.

This is identical to the change made to rules_python in bazelbuild/rules_python#2516.

Work towards bazelbuild/rules_python#2581.

PiperOrigin-RevId: 721941173
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this pull request Feb 1, 2025
Previously, the import path within the runfiles was only correct for the case `--legacy_external_runfiles=True` (which copied the runfiles into `$RUNFILES/<main repo>/external/<external repo>/<path>` in addition to `$RUNFILES/<external repo>/<path>`. This flag was flipped to False in Bazel 8.0.0.

This is identical to the change made to rules_python in bazelbuild/rules_python#2516.

Work towards bazelbuild/rules_python#2581.

PiperOrigin-RevId: 721941173
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this pull request Feb 1, 2025
Previously, the import path within the runfiles was only correct for the case `--legacy_external_runfiles=True` (which copied the runfiles into `$RUNFILES/<main repo>/external/<external repo>/<path>` in addition to `$RUNFILES/<external repo>/<path>`. This flag was flipped to False in Bazel 8.0.0.

This is identical to the change made to rules_python in bazelbuild/rules_python#2516.

Work towards bazelbuild/rules_python#2581.

PiperOrigin-RevId: 721941173
copybara-service bot pushed a commit to protocolbuffers/protobuf that referenced this pull request Feb 3, 2025
Previously, the import path within the runfiles was only correct for the case `--legacy_external_runfiles=True` (which copied the runfiles into `$RUNFILES/<main repo>/external/<external repo>/<path>` in addition to `$RUNFILES/<external repo>/<path>`. This flag was flipped to False in Bazel 8.0.0.

This is identical to the change made to rules_python in bazelbuild/rules_python#2516.

Work towards bazelbuild/rules_python#2581.

PiperOrigin-RevId: 722690855
@aignas
Copy link
Collaborator Author

aignas commented Feb 8, 2025

Closing in favour of #2604

@aignas aignas closed this Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need: upstream support An issue that needs changes in upstream code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid downloading protobuf dependancy when not needed
3 participants