gh-116622: Fix testPyObjectPrintOSError on Android (#122487)

mhsmith · web-flow · commit 82db5728136e · 2024-07-31T08:21:43.000+08:00
Adds extra handling for way BSD/Android return errors from calls to fwrite.
diff --git a/Android/android.py b/Android/android.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 import argparse
+from glob import glob
 import os
 import re
 import shutil
@@ -16,16 +17,21 @@
 CROSS_BUILD_DIR = CHECKOUT / "cross-build"
 
 
-def delete_if_exists(path):
-    if path.exists():
+def delete_glob(pattern):
+    # Path.glob doesn't accept non-relative patterns.
+    for path in glob(str(pattern)):
+        path = Path(path)
         print(f"Deleting {path} ...")
-        shutil.rmtree(path)
+        if path.is_dir() and not path.is_symlink():
+            shutil.rmtree(path)
+        else:
+            path.unlink()
 
 
 def subdir(name, *, clean=None):
     path = CROSS_BUILD_DIR / name
     if clean:
-        delete_if_exists(path)
+        delete_glob(path)
     if not path.exists():
         if clean is None:
             sys.exit(
@@ -150,10 +156,17 @@ def configure_host_python(context):
 
 
 def make_host_python(context):
+    # The CFLAGS and LDFLAGS set in android-env include the prefix dir, so
+    # delete any previously-installed Python libs and include files to prevent
+    # them being used during the build.
     host_dir = subdir(context.host)
+    prefix_dir = host_dir / "prefix"
+    delete_glob(f"{prefix_dir}/include/python*")
+    delete_glob(f"{prefix_dir}/lib/libpython*")
+
     os.chdir(host_dir / "build")
     run(["make", "-j", str(os.cpu_count())], host=context.host)
-    run(["make", "install", f"prefix={host_dir}/prefix"], host=context.host)
+    run(["make", "install", f"prefix={prefix_dir}"], host=context.host)
 
 
 def build_all(context):
@@ -164,7 +177,7 @@ def build_all(context):
 
 
 def clean_all(context):
-    delete_if_exists(CROSS_BUILD_DIR)
+    delete_glob(CROSS_BUILD_DIR)
 
 
 # To avoid distributing compiled artifacts without corresponding source code,
diff --git a/Android/testbed/app/build.gradle.kts b/Android/testbed/app/build.gradle.kts
@@ -7,10 +7,17 @@ plugins {
 
 val PYTHON_DIR = File(projectDir, "../../..").canonicalPath
 val PYTHON_CROSS_DIR = "$PYTHON_DIR/cross-build"
+
 val ABIS = mapOf(
     "arm64-v8a" to "aarch64-linux-android",
     "x86_64" to "x86_64-linux-android",
-)
+).filter { File("$PYTHON_CROSS_DIR/${it.value}").exists() }
+if (ABIS.isEmpty()) {
+    throw GradleException(
+        "No Android ABIs found in $PYTHON_CROSS_DIR: see Android/README.md " +
+        "for building instructions."
+    )
+}
 
 val PYTHON_VERSION = File("$PYTHON_DIR/Include/patchlevel.h").useLines {
     for (line in it) {
diff --git a/Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst b/Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst
@@ -0,0 +1,3 @@
+Make :any:`PyObject_Print` work around a bug in Android and OpenBSD which
+prevented it from throwing an exception when trying to write to a read-only
+stream.
diff --git a/Objects/object.c b/Objects/object.c
@@ -536,6 +536,7 @@ int
 PyObject_Print(PyObject *op, FILE *fp, int flags)
 {
     int ret = 0;
+    int write_error = 0;
     if (PyErr_CheckSignals())
         return -1;
 #ifdef USE_STACKCHECK
@@ -574,14 +575,20 @@ PyObject_Print(PyObject *op, FILE *fp, int flags)
                     ret = -1;
                 }
                 else {
-                    fwrite(t, 1, len, fp);
+                    /* Versions of Android and OpenBSD from before 2023 fail to
+                       set the `ferror` indicator when writing to a read-only
+                       stream, so we need to check the return value.
+                       (https://github.com/openbsd/src/commit/fc99cf9338942ecd9adc94ea08bf6188f0428c15) */
+                    if (fwrite(t, 1, len, fp) != (size_t)len) {
+                        write_error = 1;
+                    }
                 }
                 Py_DECREF(s);
             }
         }
     }
     if (ret == 0) {
-        if (ferror(fp)) {
+        if (write_error || ferror(fp)) {
             PyErr_SetFromErrno(PyExc_OSError);
             clearerr(fp);
             ret = -1;

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Make :any:`PyObject_Print` work around a bug in Android and OpenBSD which
	`2`	`+prevented it from throwing an exception when trying to write to a read-only`
	`3`	`+stream.`
Original file line number	Diff line number	Diff line change
`@@ -536,6 +536,7 @@ int`
`536`	`536`	`PyObject_Print(PyObject op, FILE fp, int flags)`
`537`	`537`	`{`
`538`	`538`	`int ret = 0;`
	`539`	`+ int write_error = 0;`
`539`	`540`	`if (PyErr_CheckSignals())`
`540`	`541`	`return -1;`
`541`	`542`	`#ifdef USE_STACKCHECK`
`@@ -574,14 +575,20 @@ PyObject_Print(PyObject op, FILE fp, int flags)`
`574`	`575`	`ret = -1;`
`575`	`576`	`}`
`576`	`577`	`else {`
`577`		`- fwrite(t, 1, len, fp);`
	`578`	`+ /* Versions of Android and OpenBSD from before 2023 fail to`
	`579`	+ set the `ferror` indicator when writing to a read-only
	`580`	`+ stream, so we need to check the return value.`
	`581`	`+ (https://github.com/openbsd/src/commit/fc99cf9338942ecd9adc94ea08bf6188f0428c15) */`
	`582`	`+ if (fwrite(t, 1, len, fp) != (size_t)len) {`
	`583`	`+ write_error = 1;`
	`584`	`+ }`
`578`	`585`	`}`
`579`	`586`	`Py_DECREF(s);`
`580`	`587`	`}`
`581`	`588`	`}`
`582`	`589`	`}`
`583`	`590`	`if (ret == 0) {`
`584`		`- if (ferror(fp)) {`
	`591`	`+ if (write_error \|\| ferror(fp)) {`
`585`	`592`	`PyErr_SetFromErrno(PyExc_OSError);`
`586`	`593`	`clearerr(fp);`
`587`	`594`	`ret = -1;`