search

eclipse-cyclonedds / cyclonedds

Eclipse Cyclone DDS project
https://projects.eclipse.org/projects/iot.cyclonedds
Other
854 stars 352 forks source link

Fix build for earlier macOS versions #2020

Closed barracuda156 closed 3 months ago

barracuda156 commented 3 months ago

@eboasson These are necessary, but insufficient. At the moment 0.10.5 still fails further at linking, while the master fails earlier: https://github.com/eclipse-cyclonedds/cyclonedds/issues/2019

MAXTHREADNAMESIZE is defined in Libc internal header. Since the code uses it unconditionally, it should be defined to something. For w/e reason the value from Libc is inaccessible, and it also fails on CI on x86_64 up to 10.10: https://build.macports.org/builders/ports-10.10_x86_64-builder/builds/271089/steps/install-port/logs/stdio

Then up to 10.12 there is a failure due to wrong number of arguments in memcpy: https://build.macports.org/builders/ports-10.12_x86_64-builder/builds/275235/steps/install-port/logs/stdio (This is a known issue, Apple changed the declaration for memcpy.)

Unfortunately, I have no idea at the moment why linking fails with undefined _dds_stream_extract_keyBE_from_key.

barracuda156 commented 3 months ago

Well, looks like I cannot sign the ECA, since log in is unavailable to non-members. Feel free to cherry-pick from here and then close the PR.

barracuda156 commented 3 months ago

Given that Debian faced the same issue, apparently, it is a genuine bug: https://sources.debian.org/patches/cyclonedds/0.10.5-1/0005-Fix-unresolved-symbol-on-big-endian-architectures.patch

barracuda156 commented 3 months ago

@eboasson These 4 commits plus a patch from Debian fix 0.10.5 for me.

eboasson commented 3 months ago

Well, looks like I cannot sign the ECA, since log in is unavailable to non-members. Feel free to cherry-pick from here and then close the PR.

Maybe they changed something and now require you to register? Whatever, thank you for trying ☺️ Cherry-picking is a nice pragmatic solution for this particular PR. I'll try to deal with the problem the CI ran into as well.

Thanks!

barracuda156 commented 3 months ago

LGTM, except that the gcc-13 on macOS run has a big problem that I suspect to be triggered by including the AvailabilityMacros.h and to have more to do with gcc/homebrew/macOS then with your proposed changes. I'm sure that'll be easy to work around, it just needs to be sorted out before this can be merged.

Since I do not use Homebrew, I have no idea what could be the issue. AvailabilityMacros.h is a standard header, and it certainly works with gcc13, so perhaps the bug is on Homebrew side, if something fails there.

P. S. gcc13 did have an issue with libc++ headers (fixed in the master after 13.2.0), but a) hardly anyone gonna even need to use this combo and b) AFAIK, there is no relation of that bug to AvailabilityMacros.h.