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

dynamic_modules: switch to crate_universe to remove manual bindgen #36240

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

mathetake
Copy link
Member

@mathetake mathetake commented Sep 20, 2024

Commit Message: dynamic_modules: switch to crate_universe to remove manual bindgen
Additional Description:
This is a follow up on #35914 which ended up requiring a manual
step to generate the binding for the ABI headers.
This switches to use crate_universe and use the bindgen as a dev-dependency.
As a result, we can adopt the build.rs script to generate the C bindings at build
time instead of the manual bindgen step.

This also allows us to freely add a crate dependency when it becomes necessary.

Risk Level: N/A
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #36240 was opened by mathetake.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 20, 2024
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #36240 was opened by mathetake.

see: more, trace.

@mathetake
Copy link
Member Author

cc @phlax - i am trying to enable crate_universe here which I think doesn't work for gcc and compiletime_options jobs. let's see

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Member Author

so crate_universe itself seems to be working. The next is to see whether the bindgen actually works or not

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake changed the title try using crate_universe dynamic_modules: switch to crate_universe to remove manual bindgen step Sep 20, 2024
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake changed the title dynamic_modules: switch to crate_universe to remove manual bindgen step dynamic_modules: switch to crate_universe to remove manual bindgen Sep 20, 2024
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Member Author

i think it's all good now 🤞

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake marked this pull request as ready for review September 20, 2024 12:31
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants