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.

Sign up for GitHub

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

Jump to bottom

build: make dynamic modules work on macos #38437

Merged
merged 17 commits into from
Feb 16, 2025
Merged

build: make dynamic modules work on macos #38437

merged 17 commits into from
Feb 16, 2025

Conversation

mathetake
Copy link
Member

@mathetake mathetake commented Feb 12, 2025 •
edited
Loading

Commit Message: build: make dynamic modules work on macos
Additional Description:

This tweaks a build setting on macos especially around symbol exports to enable dynamic modules work on macos. Notably, this adds the macos compatible symbols list which has a different format than linux platform.

Risk Level: low
Testing: existing ones
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

@mathetake
build: run dynamic modules test on macos
56f094f 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@repokitteh-read-only RepoKitteh - Read Only
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: #38437 was opened by mathetake.

see: more, trace.

@mathetake
try
5422aaf 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
try
e1cba96 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more
5b1cb77 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
make it work
93cc91e 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more
a9597d3 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more
7ff4900 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake mentioned this pull request Feb 12, 2025
Open
7 tasks
@mathetake
try
cff2413 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Member Author 

                _$LT$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilterImpl$u20$as$u20$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilter$GT$::set_request_header::h347b3f3891966fb5 in libenvoy_proxy_dynamic_modules_rust_sdk-1759820727.rlib[10](envoy_proxy_dynamic_modules_rust_sdk-1759820727.envoy_proxy_dynamic_modules_rust_sdk.81a14b5c08b35574-cgu.07.rcgu.o)
            "_envoy_dynamic_module_callback_http_set_request_trailer", referenced from:
                _$LT$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilterImpl$u20$as$u20$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilter$GT$::set_request_trailer::h0fad4285779b0197 in libenvoy_proxy_dynamic_modules_rust_sdk-1759820727.rlib[10](envoy_proxy_dynamic_modules_rust_sdk-1759820727.envoy_proxy_dynamic_modules_rust_sdk.81a14b5c08b35574-cgu.07.rcgu.o)
            "_envoy_dynamic_module_callback_http_set_response_header", referenced from:
                _$LT$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilterImpl$u20$as$u20$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilter$GT$::set_response_header::h47dd3a79d0503159 in libenvoy_proxy_dynamic_modules_rust_sdk-1759820727.rlib[10](envoy_proxy_dynamic_modules_rust_sdk-1759820727.envoy_proxy_dynamic_modules_rust_sdk.81a14b5c08b35574-cgu.07.rcgu.o)
            "_envoy_dynamic_module_callback_http_set_response_trailer", referenced from:
                _$LT$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilterImpl$u20$as$u20$envoy_proxy_dynamic_modules_rust_sdk..EnvoyHttpFilter$GT$::set_response_trailer::h13ece9e443583df5 in libenvoy_proxy_dynamic_modules_rust_sdk-1759820727.rlib[10](envoy_proxy_dynamic_modules_rust_sdk-1759820727.envoy_proxy_dynamic_modules_rust_sdk.81a14b5c08b35574-cgu.07.rcgu.o)
          ld: symbol(s) not found for architecture arm64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

the error seems to have originated from the compilation of dynamic modules with rust toolchain and this issue must be something to do with this rust-lang/rust#62874

I tried adding the flag build.rs but no luck.

I think i will give up for now - in case someone interested cc @zirain

@zirain
Copy link
Member

zirain commented Feb 13, 2025

maybe we can highlight somewhere that it's not available on mac.

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

ok almost there..

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

finally only problem is symbol resolution at runtime

C++ exception with description "Bad StatusOr access: INVALID_ARGUMENT: Failed to load dynamic module: /private/var/tmp/_bazel_runner/24e7a101d26e6b05dd6bbfabf7fb2d6f/sandbox/darwin-sandbox/7233/execroot/envoy/bazel-out/darwin_arm64-fastbuild/bin/test/extensions/dynamic_modules/http/filter_test.runfiles/envoy/test/extensions/dynamic_modules/test_data/rust/libhttp.so : dlopen(/private/var/tmp/_bazel_runner/24e7a101d26e6b05dd6bbfabf7fb2d6f/sandbox/darwin-sandbox/7233/execroot/envoy/bazel-out/darwin_arm64-fastbuild/bin/test/extensions/dynamic_modules/http/filter_test.runfiles/envoy/test/extensions/dynamic_modules/test_data/rust/libhttp.so, 0x0005): symbol not found in flat namespace '_envoy_dynamic_module_callback_http_get_request_header'" thrown in the test body.

it seems there's some weird "_" prefix

@mathetake
Merge remote-tracking branch 'origin/main' into macosfixbazeldynamicm…
619a67d 
…odules
@mathetake
more
4b50b64 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more
16c0cf0 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
fix
693345c 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more
41e7483 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
more consistency
fe984cd 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake changed the title build: run dynamic modules test on macos build: make dynamic modules work on macos Feb 16, 2025
mathetake
mathetake commented Feb 16, 2025
View reviewed changes
source/extensions/dynamic_modules/BUILD
@@ -15,45 +15,6 @@ envoy_cc_library(
"abi.h",
"dynamic_modules.h",
],
linkopts = select({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: we no longer need to manually list callback symbols here!

mathetake
mathetake commented Feb 16, 2025
View reviewed changes
ci/mac_ci_steps.sh
@@ -32,6 +32,7 @@ DEFAULT_TEST_TARGETS=(
"//test/integration:protocol_integration_test"
"//test/integration:tcp_proxy_integration_test"
"//test/integration:extension_discovery_integration_test"
"//test/extensions/dynamic_modules/http:filter_test"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: this is the minimal test that ensures the dynamic loading/symbol resolution working

@mathetake
Copy link
Member Author

working!!!!!!!!

@mathetake mathetake marked this pull request as ready for review February 16, 2025 20:03
@mathetake
Copy link
Member Author

/assign @mattklein123

@mattklein123 mattklein123 merged commit 36c1891 into envoyproxy:main Feb 16, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

Assignees

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mathetake @zirain @mattklein123