Skip to content

build: make dynamic modules work on macos#38437

Merged
mattklein123 merged 17 commits intoenvoyproxy:mainfrom
mathetake:macosfixbazeldynamicmodules
Feb 16, 2025
Merged

build: make dynamic modules work on macos#38437
mattklein123 merged 17 commits intoenvoyproxy:mainfrom
mathetake:macosfixbazeldynamicmodules

Conversation

@mathetake
Copy link
Copy Markdown
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
Copy link
Copy Markdown

repokitteh-read-only bot commented Feb 12, 2025

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
Closed
11 tasks
@mathetake
try
cff2413 
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Copy Markdown
Member Author

mathetake commented Feb 12, 2025 

                _$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
Copy Markdown
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
Copy Markdown
Member Author

mathetake commented Feb 13, 2025

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
Copy Markdown
Member Author

mathetake commented Feb 13, 2025

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
"abi.h",
"dynamic_modules.h",
],
linkopts = select({
Copy link
Copy Markdown
Member Author

@mathetake mathetake Feb 16, 2025

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
"//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
Copy Markdown
Member Author

@mathetake mathetake Feb 16, 2025

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
Copy Markdown
Member Author

mathetake commented Feb 16, 2025

working!!!!!!!!

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

mathetake commented Feb 16, 2025

/assign @mattklein123

@mattklein123 mattklein123 merged commit 36c1891 into envoyproxy:main Feb 16, 2025
24 checks passed
agrawroh pushed a commit to agrawroh/envoy that referenced this pull request Apr 9, 2025
@mathetake @agrawroh
build: make dynamic modules work on macos (envoyproxy#38437)
a0fe522 
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

---------

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

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