-
Notifications
You must be signed in to change notification settings - Fork 336
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
auto/otel: Fix breakage on non-Linux platforms #1520
base: master
Are you sure you want to change the base?
Conversation
ac000
commented
Dec 20, 2024
When building with --otel on macOS for example I was seeing compile failures with the cpu_set_t stuff which should only be used under Linux. It turned out that despite checking for Linux sched_getaffinity() ... not found we were getting #ifndef NXT_HAVE_LINUX_SCHED_GETAFFINITY #define NXT_HAVE_LINUX_SCHED_GETAFFINITY 1 #endif in build/include/nxt_auto_config.h It seems this was due to the . auto/feature in auto/otel, this check happens right after the above. Without having nxt_feature_name=NXT_HAVE_OTEL set. Instead we were adding the define for that manually. Doing auto/feature without having a nxt_feature_name must have used the last set one and enabled it. Set nxt_feature_name and remove the manual editing of nxt_auto_config.h Signed-off-by: Andrew Clayton <[email protected]>
This patch seems correct to address this particular issue. My fix for macOS build uses a similar trick, but we also need to link with additional frameworks on that OS: thresheek@660e853 Also, another fix is required to be able to build on macOS: thresheek@18381b1 We probably want to incorporate those changes in this PR for the sake of completeness. |
It would affect any system that didn't have a Linux compatible sched_getaffinity(2), not that I was aware of any that did, but it seems at least FreeBSD does, so I'll need to tweak the commit subject...
Weird, I wonder what is doing that... it certainly uses regular OpenSSL for general TLS support...
This one makes sense.
But then there is some other breakage also with (in auto/make) 87 ifeq (\$D,1)
88 NXT_OTEL_LIB_LOC = $NXT_OTEL_LIB_DIR/target/debug/libotel.a
89 else
90 NXT_OTEL_LIB_LOC = $NXT_OTEL_LIB_DIR/target/release/libotel.a
91 endif Prevents the use of 44 if [ -n "$NXT_GNU_MAKE" ] || [ $NXT_OS = "SunOS" ]; then And then above that we have 64 ifeq (\$D,1)
65 CFLAGS += -O0
66 RUST_FLAGS += --debug
67 else
68 RUST_FLAGS += --release
69 endif However that is guarded by the 44 if [ -n "$NXT_GNU_MAKE" ] || [ $NXT_OS = "SunOS" ]; then So on the likes of FreeBSD RUST_FLAGS isn't set all... I don't mind making GNU make a build dependency... however at the moment it seems --otel is fubar on anything non-Linux... (between needing sched_getaffinity(2) and GNU make being the default make(1)) |
Some of this would be simpler if we could just tell
and then just do debug builds by default and enable release builds by setting a cargo environment variable, but that seems to be a stumbling block as unless I missed it, that seems to be the one thing you can't set... |
While we're at it could we
|