openpty/forkpty build failure using nix package manager macOS environment

[LnL7]

BPO	34027
Nosy	@ronaldoussoren, @orivej, @ned-deily, @LnL7
PRs	bpo-34027: Only use util.h on macOS, fixes build errors for openpty/forkpty #8056
Files	darwin-libutil.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2018-07-02.21:31:08.119>
labels = ['OS-mac', 'build', '3.9', '3.10', '3.11']
title = 'openpty/forkpty build failure using nix package manager macOS environment'
updated_at = <Date 2022-01-17.22:57:04.283>
user = 'https://github.com/LnL7'

bugs.python.org fields:

activity = <Date 2022-01-17.22:57:04.283>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Build', 'macOS']
creation = <Date 2018-07-02.21:31:08.119>
creator = 'LnL7'
dependencies = []
files = ['47666']
hgrepos = []
issue_num = 34027
keywords = ['patch']
message_count = 14.0
messages = ['320913', '320914', '320915', '320918', '320920', '320921', '320925', '320930', '320931', '320933', '320936', '320940', '320992', '392618']
nosy_count = 5.0
nosy_names = ['ronaldoussoren', 'orivej', 'ned.deily', 'LnL7', 'lukegb']
pr_nums = ['8056']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue34027'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[LnL7]

I can't really figure out why, but the <util.h> import in posixmodule.c seems to get skipped since python 3.7. The condition doesn't look entirely correct in case libutil.h is also available on macOS, however this has not changed since 3.6 and it worked fine there while both where available.

Part of the configure log:

checking pty.h usability... no
checking pty.h presence... no
checking for pty.h... no
checking libutil.h usability... yes
checking libutil.h presence... yes
checking for libutil.h... yes
...
checking util.h usability... yes
checking util.h presence... yes
checking for util.h... yes

Build error:

./Modules/posixmodule.c:5924:9: error: implicit declaration of function 'openpty' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL) != 0)
        ^
./Modules/posixmodule.c:5924:9: note: did you mean 'openat'?
/nix/store/q819d3vjz7vswpvkrfa9gck3ys8rmvcj-Libsystem-osx-10.11.6/include/sys/fcntl.h:480:5: note: 'openat' declared here
int     openat(int, const char *, int, ...) __DARWIN_NOCANCEL(openat) __OSX_AVAILABLE_STARTING(__MAC_10_10, __IPHONE_8_0);
        ^
./Modules/posixmodule.c:5924:9: warning: this function declaration is not a prototype [-Wstrict-prototypes]
    if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL) != 0)
        ^
./Modules/posixmodule.c:6018:11: error: implicit declaration of function 'forkpty' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    pid = forkpty(&master_fd, NULL, NULL, NULL);
          ^
./Modules/posixmodule.c:6018:11: warning: this function declaration is not a prototype [-Wstrict-prototypes]
2 warnings and 2 errors generated.
make: *** [Makefile:1793: Modules/posixmodule.o] Error 1

[ned-deily]

I don't see this build failure. Can you say where you find libutil.h installed on macOS and what version of macOS this is? It doesn't seem to be part of the standard Apple-supplied /usr/include header files installed by xcode-select --install for macOS 10.13 at least?

[LnL7]

This is a build using the nix package manager, we use all of the opensource components provided by apple instead of relying on xcode. The libutil.h that's found in this case is the one from the 10.11.6 sources.

https://opensource.apple.com/source/libutil/libutil-43/

[ned-deily]

Even on macOS 10.11.6, libutil.h is not installed and posixmodule.c builds just fine. So I don't think that the presence of something in opensource.apple.com necessarily means it was installed in place in a standard system. I'm not saying yet that we shouldn't change the tests along the line you suggest in the PR. But I really would like to understand first why we should make a change when as far as I recall this issue has never come up before.

[LnL7]

The headers end up in the build environment because of Libsystem which is part of our base environment. But yes, this is also the case with 3.6 and that builds just fine so I'm not sure why it's failing now.

These are the logs from our build service for reference:
python37.x86_64-darwin https://hydra.nixos.org/build/76780195/nixlog/2
python36.x86_64-darwin https://hydra.nixos.org/build/76018915/nixlog/1

[ned-deily]

It's failing in 3.7 because of the change introduced in bpo-27659 to prohibit implicit C function declarations by adding the -Werror=implicit-function-declaration to compiles.

[LnL7]

Oh interesting, so it only used to worked by accident. I should have looked at the 3.6 log more closely.

[orivej]

The explicit conditional on __APPLE__ can be avoided with the attached patch. (Tested on NixOS and macOS with Nixpkgs.)

[ned-deily]

Thanks for the suggested patch but I am not persuaded that there is a problem needing fixing on the Python side. As I see it, Python's posixmodule builds fine in the environments we claim to support, including macOS, and has for a long time. It appears that the build failure reported here is because Nix is not properly emulating a macOS build environment and that builds under Nix have been producing warnings prior to 3.7 so it's not a new issue. Rather than trying to patch around the Nix problem on the Python side, and potentially introducing problems on platforms that Python "officially" supports, it seems to me the better options are for either (1) Nix to better emulate a real macOS build environment by supplying a more correct set of header files (which also might help other third-party packages trying to use Nix for macOS builds); or, if that is not possible, (2) to carry a local patch for Python when building under Nix. I'm open to persuasion but it would have to be a very compelling argument. We have never claimed to support macOS builds on anything other than Apple-supplied macOS development environments running on macOS and I don't see that changing.

[orivej]

I understand the desirability of avoiding potential problems on supported platforms (given that you can not test building Python on all of them) and recognize that both options you have given are reasonable, but as I see it posixmodule.c incorrectly uses autoconf checks for the presence of header files to infer the platform: it assumes that libutil.h can not exist on macOS. It should either include all found relevant headers, as proposed in my patch or done e.g. in dropbear [1] and gnulib [2], or it should dispatch on the platform directly as LnL7 has proposed (although in this case it should no longer use the autoconf check on macOS).

[1] https://secure.ucc.asn.au/hg/dropbear/file/DROPBEAR_2018.76/includes.h
[2] http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/pty.in.h?id=281b825eba78dc403b9bf34979923bc587bc31af

[LnL7]

Even tho it's headers are not /usr/lib/libutil.dylib is available on every macOS system. Unlike on bsd this doesn't include the relevant symbols for openpty/forkpty, that's why I used an __APPLE__ condition but including both also works fine. Assuming it's headers are not available doesn't seem like a good assumption, even tho they won't be in most cases.

[ronaldoussoren]

I agree with Ned that we shouldn't add code to deal with non-standard compiler setups, correctly supporting macOS is hard enough as it is.

That said, the attached patch (but not PR8056) looks fine to me, it includes all possibly relevant headers files (as mentioned in msg320933) and that should be ok. The PR is not acceptable because it unnecessarily contains macOS specific code.

[LnL7]

Either patch looks fine to me, should I close the pull request?

[lukegb]

Still seems to be a problem with everything up to Py3.11.

[Bonifacio2]

Yesterday I tried building the HEAD using MacOS 13.2.1 (Ventura) and everything went fine. I thought we could just identify the commit that incidentally fixed this issue and backport it. To identify the commit, I went to main and ran a git bisect using HEAD and HEAD~5506*. The results of my search pointed to this commit 04f6733275. I tried to cherry-pick it to 3.11, but there are too many conflicts in files I don't understand. Personally, I don't think backporting this is worth the effort.

*I ended up with 5506 because it was the sum of commits ahead and behind main comparing it to 3.11.

[reckenrode]

This can probably be closed. As of NixOS/nixpkgs#346043, macOS support in nixpkgs looks more like a native toolchain (while still building from a bootstrap tarball). Consequently, the libutil.h header is not included in the default build environment anymore. Users who do need it can opt into using it with the darwin.libutil package in nixpkgs.

[emilazy]

Yes, was just coming here to say that :) As of the just‐merged NixOS/nixpkgs#353873 we no longer patch Python for this, so unless this affects any non‐Nixpkgs environments, I think this can safely be closed.

[erlend-aasland]

Thanks for chiming in Randy and Emily; closing as outdated.