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

AsyncFdSocket.h: fix for macOS < 11 #2124

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

barracuda156
Copy link

Fixes: #2099

Credits to @sryze for pointing out the cause of the breakage in #2031 (comment)
However, the problem is not specific to Catalina, but affects every macOS from 10.15 down to 10.5, and of course the solution is not to hack the SDK, but provide a fix in the code.

@AaronNGray
Copy link

I really need this patch on my MacOS 10.15.x machine, otherwise I have to buy a new machine to develop on :| I am unfamiliar with folly or brew and how to patch brew packages.

Does rpm-build:fedora-rawhide-* build anyway ?

@AaronNGray
Copy link

I think the rpm-build:fedora-rawhide-* issue is nothing to do with with this issue !

Looks like its a failed patch in a test specific to rpm-build:fedora-rawhide !

https://github.com/facebook/folly/blob/main/folly/executors/test/CodelTest.cpp

Building for target aarch64
setting SOURCE_DATE_EPOCH=1705017600
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.QYXdi5
+ umask 022
+ cd /builddir/build/BUILD
+ cd /builddir/build/BUILD
+ rm -rf folly-v2024.01.08.00
+ /usr/lib/rpm/rpmuncompress -x /builddir/build/SOURCES/folly-v2024.01.08.00.tar.gz
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd folly-v2024.01.08.00
+ rm -rf /builddir/build/BUILD/folly-v2024.01.08.00-SPECPARTS
+ /usr/bin/mkdir -p /builddir/build/BUILD/folly-v2024.01.08.00-SPECPARTS
+ /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ echo 'Patch #2 (folly-fix_codel_test.patch):'
Patch #2 (folly-fix_codel_test.patch):
+ /usr/bin/patch --no-backup-if-mismatch -f -p1 --fuzz=0
patching file folly/executors/test/CodelTest.cpp
Hunk #1 FAILED at 41.
1 out of 1 hunk FAILED -- saving rejects to file folly/executors/test/CodelTest.cpp.rej
error: Bad exit status from /var/tmp/rpm-tmp.QYXdi5 (%prep)

@barracuda156
Copy link
Author

I really need this patch on my MacOS 10.15.x machine, otherwise I have to buy a new machine to develop on :| I am unfamiliar with folly or brew and how to patch brew packages.

If Macports is an acceptable solution for you, the patch is already there.

Does rpm-build:fedora-rawhide-* build anyway ?

Sorry, I have no idea, I do not use Linux.

@AaronNGray
Copy link

AaronNGray commented Jan 15, 2024

I really need this patch on my MacOS 10.15.x machine, otherwise I have to buy a new machine to develop on :| I am unfamiliar with folly or brew and how to patch brew packages.

If Macports is an acceptable solution for you, the patch is already there.

@barracuda156 - Sorry for my ignorance I am not a Mac person, how do I about obtaining those ?

Ah okay its an alternative to brew, okay will try, thanks !

Does rpm-build:fedora-rawhide-* build anyway ?

Sorry, I have no idea, I do not use Linux.

I am a seasoned Linux and ex Fedora user, but needs to be attended to by someone who knows the specific area.

@barracuda156
Copy link
Author

I really need this patch on my MacOS 10.15.x machine, otherwise I have to buy a new machine to develop on :| I am unfamiliar with folly or brew and how to patch brew packages.

If Macports is an acceptable solution for you, the patch is already there.

@barracuda156 - Sorry for my ignorance I am not a Mac person, how do I about obtaining those ?

Ah okay its an alternative to brew, okay will try, thanks !

On a macOS, you could install folly after installing Macports via sudo port -v install folly. It currently has v 2024.01.08.00.
Aside of a few very old Intel systems, it builds on every macOS at the moment: https://ports.macports.org/port/folly/details
It should work for 10.15 (without any ridiculous SDK hacking).

@AaronNGray
Copy link

@barracuda156 - Great, I am going to move over to MacPorts most likely. I got watchman installed using it that uses folly. Just need to see what its node.js support is like.

@yfeldblum
Copy link
Contributor

Interesting! I think we can do something like this.

I would like to avoid changing preprocessor symbol definitions not owned by folly. __DARWIN_ALIGN32 is owned by darwin, not by folly, so I would not want to mess with it.

What we can do is #define FOLLY_DETAIL_CMSG_SPACE CMG_SPACE on most platforms and #define it to something else on macos < 11 (or relevant condition on darwin). Then use FOLLY_DETAIL_CMSG_SPACE instead of CMG_SPACE in folly code. It would be in a new file folly/portability/SysSocket.h since CMSG_SPACE is documented on linux to be exported from sys/socket.h (https://linux.die.net/man/3/cmsg_space).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants