Repository: python/cpython
Pull request: gh-114875: Add `getgrent` as a prerequisite for building the `grp` module
This allows the module to be correctly skipped on older versions of Android which don't have `getgrent` and related functions:
```
checking for getgrent... no
checking for getgrgid... yes
checking for getgrgid_r... no
[...]
checking for stdlib extension module grp... missing
```

<!-- gh-issue-number: gh-114875 -->
* Fixes: gh-114875
<!-- /gh-issue-number -->


Problem statement:
`grp` module attempts to build even if `getgrent` family of functions are unavailable
# Bug report

### Bug description:

In the configure script, the only prerequisite for the `grp` module is currently the `getgrgid` function. Older versions of Android have this function, but they don't have the `getgrent` function, which the `grp` module uses unconditionally. This causes the build to fail as follows:

```
/Users/msmith/Library/Android/sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/darwin-x86_64/bin/x86_64-linux-android21-clang  -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include     -c ./Modules/grpmodule.c -o Modules/grpmodule.o
./Modules/grpmodule.c:127:24: warning: unused variable 'buf2' [-Wunused-variable]
    char *buf = NULL, *buf2 = NULL;
                       ^
./Modules/grpmodule.c:205:24: warning: unused variable 'buf2' [-Wunused-variable]
    char *buf = NULL, *buf2 = NULL, *name_chars;
                       ^
./Modules/grpmodule.c:287:5: error: implicit declaration of function 'setgrent' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    setgrent();
    ^
./Modules/grpmodule.c:288:17: error: implicit declaration of function 'getgrent' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    while ((p = getgrent()) != NULL) {
                ^
```

### CPython versions tested on:

CPython main branch

### Operating systems tested on:

Other

<!-- gh-linked-prs -->
### Linked PRs
* gh-114876
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: Make `grp` module conditional on `getgrent` availability

Patch to review:
diff --git a/Misc/NEWS.d/next/Build/2024-02-01-20-08-11.gh-issue-114875.x_2iZ9.rst b/Misc/NEWS.d/next/Build/2024-02-01-20-08-11.gh-issue-114875.x_2iZ9.rst
new file mode 100644
index 00000000000000..7abdc42401b405
--- /dev/null
+++ b/Misc/NEWS.d/next/Build/2024-02-01-20-08-11.gh-issue-114875.x_2iZ9.rst
@@ -0,0 +1 @@
+Add ``getgrent`` as a prerequisite for building the :mod:`grp` module.
diff --git a/configure b/configure
index b1153df4d7ec52..e700ca1cae89b4 100755
--- a/configure
+++ b/configure
@@ -17444,6 +17444,12 @@ if test "x$ac_cv_func_getgid" = xyes
 then :
   printf "%s\n" "#define HAVE_GETGID 1" >>confdefs.h
 
+fi
+ac_fn_c_check_func "$LINENO" "getgrent" "ac_cv_func_getgrent"
+if test "x$ac_cv_func_getgrent" = xyes
+then :
+  printf "%s\n" "#define HAVE_GETGRENT 1" >>confdefs.h
+
 fi
 ac_fn_c_check_func "$LINENO" "getgrgid" "ac_cv_func_getgrgid"
 if test "x$ac_cv_func_getgrgid" = xyes
@@ -28901,7 +28907,8 @@ then :
 
     if true
 then :
-  if test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes
+  if test "$ac_cv_func_getgrent" = yes &&
+   test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes
 then :
   py_cv_module_grp=yes
 else $as_nop
diff --git a/configure.ac b/configure.ac
index 9587e6d63499aa..08c139641e9c98 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4767,7 +4767,7 @@ AC_CHECK_FUNCS([ \
   copy_file_range ctermid dup dup3 execv explicit_bzero explicit_memset \
   faccessat fchmod fchmodat fchown fchownat fdopendir fdwalk fexecve \
   fork fork1 fpathconf fstatat ftime ftruncate futimens futimes futimesat \
-  gai_strerror getegid getentropy geteuid getgid getgrgid getgrgid_r \
+  gai_strerror getegid getentropy geteuid getgid getgrent getgrgid getgrgid_r \
   getgrnam_r getgrouplist getgroups gethostname getitimer getloadavg getlogin \
   getpeername getpgid getpid getppid getpriority _getpty \
   getpwent getpwnam_r getpwuid getpwuid_r getresgid getresuid getrusage getsid getspent \
@@ -7293,7 +7293,9 @@ PY_STDLIB_MOD([_socket],
                     -a "$ac_cv_header_netinet_in_h" = "yes"]))
 
 dnl platform specific extensions
-PY_STDLIB_MOD([grp], [], [test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes])
+PY_STDLIB_MOD([grp], [],
+  [test "$ac_cv_func_getgrent" = yes &&
+   test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes])
 PY_STDLIB_MOD([pwd], [], [test "$ac_cv_func_getpwuid" = yes -o "$ac_cv_func_getpwuid_r" = yes])
 PY_STDLIB_MOD([resource], [], [test "$ac_cv_header_sys_resource_h" = yes])
 PY_STDLIB_MOD([_scproxy],
diff --git a/pyconfig.h.in b/pyconfig.h.in
index d8a9f68951afbd..36a46b1d14909f 100644
--- a/pyconfig.h.in
+++ b/pyconfig.h.in
@@ -474,6 +474,9 @@
 /* Define to 1 if you have the `getgid' function. */
 #undef HAVE_GETGID
 
+/* Define to 1 if you have the `getgrent' function. */
+#undef HAVE_GETGRENT
+
 /* Define to 1 if you have the `getgrgid' function. */
 #undef HAVE_GETGRGID
 


Code review comments:
diff_hunk: @@ -0,0 +1 @@
+Add ``getgrent`` as a prerequisite for building the :mod:`grp` module.
path: Misc/NEWS.d/next/Build/2024-02-01-20-08-11.gh-issue-114875.x_2iZ9.rst
text: @user1:
```suggestion
Add :c:func:`!getgrent` as a prerequisite for building the :mod:`grp` module.
```

diff_hunk: @@ -7293,7 +7293,9 @@ PY_STDLIB_MOD([_socket],
                     -a "$ac_cv_header_netinet_in_h" = "yes"]))
 
 dnl platform specific extensions
-PY_STDLIB_MOD([grp], [], [test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes])
+PY_STDLIB_MOD([grp], [],
+  [test "$ac_cv_func_getgrent" = yes &&
+   test "$ac_cv_func_getgrgid" = yes -o "$ac_cv_func_getgrgid_r" = yes])
path: configure.ac
text: @user1:
Quoting the Autoconf guide for portable shell programming:

> The `-a`, `-o`, `(`, and `)` operands are not present in all implementations, and have been marked obsolete by Posix 2008. This is because there are inherent ambiguities in using them.

Let's fix the `-o` usage now that we have the opportunity:

```suggestion
  [test "$ac_cv_func_getgrent" = "yes" &&
   { test "$ac_cv_func_getgrgid" = "yes" || test "$ac_cv_func_getgrgid_r" = "yes"; }])
```

(You'll have to regen configure after applying this suggestion.)