-
-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
freeswitch: fix build error for undeclared identifier 'NSIG' #77004
Conversation
ARM says: ==> Installing freeswitch dependency: libevent
Error: SystemCommand::ProcessTerminatedInterrupt
/opt/homebrew/Library/Homebrew/system_command.rb:211:in `block in each_output_line' I'll just re-run it |
May need someone to take a deeper look into why the existing Formula ends up failing in #76780 (and #75728, which is for next version, but is probably due to same reason). The condition that is causing the failure is also the same condition that is used to define This PR essentially forcefully set the value Related headers... /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/signal.h ...
#include <sys/signal.h>
#include <sys/_pthread/_pthread_types.h>
#include <sys/_pthread/_pthread_t.h>
#if !defined(_ANSI_SOURCE) && (!defined(_POSIX_C_SOURCE) || defined(_DARWIN_C_SOURCE))
extern __const char *__const sys_signame[NSIG];
extern __const char *__const sys_siglist[NSIG];
#endif /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/signal.h ...
#define __DARWIN_NSIG 32 /* counting 0; could be 33 (mask is 1-32) */
#if !defined(_ANSI_SOURCE) && (!defined(_POSIX_C_SOURCE) || defined(_DARWIN_C_SOURCE))
#define NSIG __DARWIN_NSIG
#endif |
@@ -151,6 +151,10 @@ class Freeswitch < Formula | |||
end | |||
|
|||
def install | |||
on_macos do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a comment here explaining what this is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would something like the headline message along with related upstream issue (signalwire/freeswitch#1145) be sufficient?
I'm actually not sure of exact reason why this is required yet.
If I had to guess, I think it is due to _XOPEN_SOURCE
being set by freeswitch
in certain places, which changes value of _POSIX_C_SOURCE
.
Perhaps due to order of include
s, we hit the odd behavior where the same condition to set NSIG
and access NSIG
are not equal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, macro voodoo. very nice. I guess at minimum a comment should give some indication of when a workaround can be removed, so a link to the upstream issue would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay for the comment.
For actual fix, another alternative may be to set _XOPEN_SOURCE
, which would do the opposite of _DARWIN_C_SOURCE
, i.e. darwin macro would probably allow platform-specific behavior while open-source macro would use portable functions instead.
d57f8b6
to
f54214e
Compare
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingbrew install <formula>
)?Try to fix the
freeswitch
error seen in PR #76780.I confirmed error on local
--build-from-source
:The condition to define
NSIG
is: