Skip to content

Conversation

@linusg
Copy link
Member

@linusg linusg commented Dec 6, 2025

No description provided.

@linusg linusg requested a review from timschumi as a code owner December 6, 2025 17:28
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Dec 6, 2025
)
configopts=(
'--disable-cgi'
'--disable-opcache'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#ifndef ZEND_WIN32
# include <sys/types.h>
# include <sys/wait.h>
-# include <sys/ipc.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add an empty sys/ipc.h instead of this patch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(to be clear,.this is not blocking or anything)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in favor of that personally. In this specific case it seems like the include is unused (as per https://pubs.opengroup.org/onlinepubs/007904975/basedefs/sys/ipc.h.html it defines ftok() and a handful of IPC_ constants, none of which seem to be used here) so we got lucky. If they were used we'd have to patch out more things, so an empty header is unlikely to be generally useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the include upstream then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That I can do: php/php-src#20662

linusg added a commit to linusg/php-src that referenced this pull request Dec 6, 2025
As far as I can tell that's been there since the initial open source
release (commit 528006a). The header defines ftok() and a handful of
IPC_* constants, none of which are used here.

System V IPC is increasingly being replaced by POSIX IPC and may
therefore not be implemented on new and/or hobbyist operating systems
such as SerenityOS[1]. In the past that wasn't an issue as the OPCache
could be disabled, which is no longer possible as of PHP 8.5[2].

I was able to build with the include patched out, but we would prefer
this to be addressed upstream.

1: SerenityOS/serenity#26465
2: php#18961
@nico nico merged commit 10ecbf8 into SerenityOS:master Dec 6, 2025
19 checks passed
@linusg linusg deleted the moar-ports branch December 6, 2025 19:15
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Dec 6, 2025
ndossche pushed a commit to php/php-src that referenced this pull request Dec 7, 2025
As far as I can tell that's been there since the initial open source
release (commit 528006a). The header defines ftok() and a handful of
IPC_* constants, none of which are used here.

System V IPC is increasingly being replaced by POSIX IPC and may
therefore not be implemented on new and/or hobbyist operating systems
such as SerenityOS[1]. In the past that wasn't an issue as the OPCache
could be disabled, which is no longer possible as of PHP 8.5[2].

I was able to build with the include patched out, but we would prefer
this to be addressed upstream.

1: SerenityOS/serenity#26465
2: #18961
ndossche pushed a commit to php/php-src that referenced this pull request Dec 8, 2025
As far as I can tell that's been there since the initial open source
release (commit 528006a). The header defines ftok() and a handful of
IPC_* constants, none of which are used here.

System V IPC is increasingly being replaced by POSIX IPC and may
therefore not be implemented on new and/or hobbyist operating systems
such as SerenityOS[1]. In the past that wasn't an issue as the OPCache
could be disabled, which is no longer possible as of PHP 8.5[2].

I was able to build with the include patched out, but we would prefer
this to be addressed upstream.

1: SerenityOS/serenity#26465
2: #18961
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants