Skip to content

Commit ab0038c

Browse files
authored
Merge branch '3.13' into backport-29acc08-3.13
2 parents 8a0e92b + ebc047f commit ab0038c

File tree

6 files changed

+130
-54
lines changed

6 files changed

+130
-54
lines changed

Android/android.py

Lines changed: 80 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from asyncio import wait_for
1616
from contextlib import asynccontextmanager
1717
from datetime import datetime, timezone
18+
from enum import IntEnum, auto
1819
from glob import glob
1920
from os.path import abspath, basename, relpath
2021
from pathlib import Path
@@ -61,6 +62,19 @@
6162
hidden_output = []
6263

6364

65+
# Based on android/log.h in the NDK.
66+
class LogPriority(IntEnum):
67+
UNKNOWN = 0
68+
DEFAULT = auto()
69+
VERBOSE = auto()
70+
DEBUG = auto()
71+
INFO = auto()
72+
WARN = auto()
73+
ERROR = auto()
74+
FATAL = auto()
75+
SILENT = auto()
76+
77+
6478
def log_verbose(context, line, stream=sys.stdout):
6579
if context.verbose:
6680
stream.write(line)
@@ -505,47 +519,47 @@ async def logcat_task(context, initial_devices):
505519
pid = await wait_for(find_pid(serial), startup_timeout)
506520

507521
# `--pid` requires API level 24 or higher.
508-
args = [adb, "-s", serial, "logcat", "--pid", pid, "--format", "tag"]
522+
#
523+
# `--binary` mode is used in order to detect which messages end with a
524+
# newline, which most of the other modes don't indicate (except `--format
525+
# long`). For example, every time pytest runs a test, it prints a "." and
526+
# flushes the stream. Each "." becomes a separate log message, but we should
527+
# show them all on the same line.
528+
args = [adb, "-s", serial, "logcat", "--pid", pid, "--binary"]
509529
logcat_started = False
510530
async with async_process(
511-
*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
531+
*args, stdout=subprocess.PIPE, stderr=None
512532
) as process:
513-
while line := (await process.stdout.readline()).decode(*DECODE_ARGS):
514-
if match := re.fullmatch(r"([A-Z])/(.*)", line, re.DOTALL):
533+
while True:
534+
try:
535+
priority, tag, message = await read_logcat(process.stdout)
515536
logcat_started = True
516-
level, message = match.groups()
517-
else:
518-
# If the regex doesn't match, this is either a logcat startup
519-
# error, or the second or subsequent line of a multi-line
520-
# message. Python won't produce multi-line messages, but other
521-
# components might.
522-
level, message = None, line
537+
except asyncio.IncompleteReadError:
538+
break
523539

524540
# Exclude high-volume messages which are rarely useful.
525541
if context.verbose < 2 and "from python test_syslog" in message:
526542
continue
527543

528544
# Put high-level messages on stderr so they're highlighted in the
529545
# buildbot logs. This will include Python's own stderr.
530-
stream = (
531-
sys.stderr
532-
if level in ["W", "E", "F"] # WARNING, ERROR, FATAL (aka ASSERT)
533-
else sys.stdout
534-
)
535-
536-
# To simplify automated processing of the output, e.g. a buildbot
537-
# posting a failure notice on a GitHub PR, we strip the level and
538-
# tag indicators from Python's stdout and stderr.
539-
for prefix in ["python.stdout: ", "python.stderr: "]:
540-
if message.startswith(prefix):
541-
global python_started
542-
python_started = True
543-
stream.write(message.removeprefix(prefix))
544-
break
546+
stream = sys.stderr if priority >= LogPriority.WARN else sys.stdout
547+
548+
# The app's stdout and stderr should be passed through transparently
549+
# to our own corresponding streams.
550+
if tag in ["python.stdout", "python.stderr"]:
551+
global python_started
552+
python_started = True
553+
stream.write(message)
554+
stream.flush()
545555
else:
546556
# Non-Python messages add a lot of noise, but they may
547-
# sometimes help explain a failure.
548-
log_verbose(context, line, stream)
557+
# sometimes help explain a failure. Format them in the same way
558+
# as `logcat --format tag`.
559+
formatted = f"{priority.name[0]}/{tag}: {message}"
560+
if not formatted.endswith("\n"):
561+
formatted += "\n"
562+
log_verbose(context, formatted, stream)
549563

550564
# If the device disconnects while logcat is running, which always
551565
# happens in --managed mode, some versions of adb return non-zero.
@@ -556,6 +570,44 @@ async def logcat_task(context, initial_devices):
556570
raise CalledProcessError(status, args)
557571

558572

573+
# Read one binary log message from the given StreamReader. The message format is
574+
# described at https://android.stackexchange.com/a/74660. All supported versions
575+
# of Android use format version 2 or later.
576+
async def read_logcat(stream):
577+
async def read_bytes(size):
578+
return await stream.readexactly(size)
579+
580+
async def read_int(size):
581+
return int.from_bytes(await read_bytes(size), "little")
582+
583+
payload_len = await read_int(2)
584+
if payload_len < 2:
585+
# 1 byte for priority, 1 byte for null terminator of tag.
586+
raise ValueError(f"payload length {payload_len} is too short")
587+
588+
header_len = await read_int(2)
589+
if header_len < 4:
590+
raise ValueError(f"header length {header_len} is too short")
591+
await read_bytes(header_len - 4) # Ignore other header fields.
592+
593+
priority_int = await read_int(1)
594+
try:
595+
priority = LogPriority(priority_int)
596+
except ValueError:
597+
priority = LogPriority.UNKNOWN
598+
599+
payload_fields = (await read_bytes(payload_len - 1)).split(b"\0")
600+
if len(payload_fields) < 2:
601+
raise ValueError(
602+
f"payload {payload!r} does not contain at least 2 "
603+
f"null-separated fields"
604+
)
605+
tag, message, *_ = [
606+
field.decode(*DECODE_ARGS) for field in payload_fields
607+
]
608+
return priority, tag, message
609+
610+
559611
def stop_app(serial):
560612
run([adb, "-s", serial, "shell", "am", "force-stop", APP_ID], log=False)
561613

Android/testbed/app/build.gradle.kts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,13 @@ android {
9494
}
9595

9696
// This controls the API level of the maxVersion managed emulator, which is used
97-
// by CI and cibuildwheel. 34 takes up too much disk space (#142289), 35 has
98-
// issues connecting to the internet (#142387), and 36 and later are not
99-
// available as aosp_atd images yet.
100-
targetSdk = 33
97+
// by CI and cibuildwheel.
98+
// * 33 has excessive buffering in the logcat client
99+
// (https://cs.android.com/android/_/android/platform/system/logging/+/d340721894f223327339010df59b0ac514308826).
100+
// * 34 consumes too much disk space on GitHub Actions (#142289).
101+
// * 35 has issues connecting to the internet (#142387).
102+
// * 36 and later are not available as aosp_atd images yet.
103+
targetSdk = 32
101104

102105
versionCode = 1
103106
versionName = "1.0"
@@ -130,9 +133,10 @@ android {
130133
path("src/main/c/CMakeLists.txt")
131134
}
132135

133-
// Set this property to something non-empty, otherwise it'll use the default
134-
// list, which ignores asset directories beginning with an underscore.
135-
aaptOptions.ignoreAssetsPattern = ".git"
136+
// Set this property to something nonexistent but non-empty. Otherwise it'll use the
137+
// default list, which ignores asset directories beginning with an underscore, and
138+
// maybe also other files required by tests.
139+
aaptOptions.ignoreAssetsPattern = "android-testbed-dont-ignore-anything"
136140

137141
compileOptions {
138142
sourceCompatibility = JavaVersion.VERSION_1_8
@@ -234,6 +238,12 @@ androidComponents.onVariants { variant ->
234238
from(cwd)
235239
}
236240
}
241+
242+
// A filename ending with .gz will be automatically decompressed
243+
// while building the APK. Avoid this by adding a dash to the end,
244+
// and add an extra dash to any filenames that already end with one.
245+
// This will be undone in MainActivity.kt.
246+
rename(""".*(\.gz|-)""", "$0-")
237247
}
238248
}
239249

Android/testbed/app/src/main/java/org/python/testbed/MainActivity.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ class PythonTestRunner(val context: Context) {
8080
continue
8181
}
8282
input.use {
83-
File(targetSubdir, name).outputStream().use { output ->
83+
// Undo the .gz workaround from build.gradle.kts.
84+
val outputName = name.replace(Regex("""(.*)-"""), "$1")
85+
File(targetSubdir, outputName).outputStream().use { output ->
8486
input.copyTo(output)
8587
}
8688
}

Lib/_android_support.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,13 @@ def write(self, prio, tag, message):
168168
# message.
169169
message = message.replace(b"\x00", b"\xc0\x80")
170170

171+
# On API level 30 and higher, Logcat will strip any number of leading
172+
# newlines. This is visible in all `logcat` modes, even --binary. Work
173+
# around this by adding a leading space, which shouldn't make any
174+
# difference to the log's usability.
175+
if message.startswith(b"\n"):
176+
message = b" " + message
177+
171178
with self._lock:
172179
now = time()
173180
self._bucket_level += (

Lib/test/test_android.py

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,18 @@ def write(s, lines=None, *, write_len=None):
183183

184184
# Multi-line messages. Avoid identical consecutive lines, as
185185
# they may activate "chatty" filtering and break the tests.
186-
write("\nx", [""])
186+
#
187+
# Additional spaces will appear in the output where necessary to
188+
# protect leading newlines.
189+
write("\nx", [" "])
187190
write("\na\n", ["x", "a"])
188-
write("\n", [""])
191+
write("\n", [" "])
192+
write("\n\n", [" ", " "])
189193
write("b\n", ["b"])
190-
write("c\n\n", ["c", ""])
194+
write("c\n\n", ["c", " "])
191195
write("d\ne", ["d"])
192196
write("xx", [])
193-
write("f\n\ng", ["exxf", ""])
197+
write("f\n\ng", ["exxf", " "])
194198
write("\n", ["g"])
195199

196200
# Since this is a line-based logging system, line buffering
@@ -201,15 +205,16 @@ def write(s, lines=None, *, write_len=None):
201205
# However, buffering can be turned off completely if you want a
202206
# flush after every write.
203207
with self.reconfigure(stream, write_through=True):
204-
write("\nx", ["", "x"])
205-
write("\na\n", ["", "a"])
206-
write("\n", [""])
208+
write("\nx", [" ", "x"])
209+
write("\na\n", [" ", "a"])
210+
write("\n", [" "])
211+
write("\n\n", [" ", " "])
207212
write("b\n", ["b"])
208-
write("c\n\n", ["c", ""])
213+
write("c\n\n", ["c", " "])
209214
write("d\ne", ["d", "e"])
210215
write("xx", ["xx"])
211-
write("f\n\ng", ["f", "", "g"])
212-
write("\n", [""])
216+
write("f\n\ng", ["f", " ", "g"])
217+
write("\n", [" "])
213218

214219
# "\r\n" should be translated into "\n".
215220
write("hello\r\n", ["hello"])
@@ -329,19 +334,16 @@ def write(b, lines=None, *, write_len=None):
329334
# currently use `logcat -v tag`, which shows each line as if it
330335
# was a separate log entry, but strips a single trailing
331336
# newline.
332-
#
333-
# On newer versions of Android, all three of the above tools (or
334-
# maybe Logcat itself) will also strip any number of leading
335-
# newlines.
336-
write(b"\nx", ["", "x"] if api_level < 30 else ["x"])
337-
write(b"\na\n", ["", "a"] if api_level < 30 else ["a"])
338-
write(b"\n", [""])
337+
write(b"\nx", [" ", "x"])
338+
write(b"\na\n", [" ", "a"])
339+
write(b"\n", [" "])
340+
write(b"\n\n", [" ", ""])
339341
write(b"b\n", ["b"])
340342
write(b"c\n\n", ["c", ""])
341343
write(b"d\ne", ["d", "e"])
342344
write(b"xx", ["xx"])
343345
write(b"f\n\ng", ["f", "", "g"])
344-
write(b"\n", [""])
346+
write(b"\n", [" "])
345347

346348
# "\r\n" should be translated into "\n".
347349
write(b"hello\r\n", ["hello"])
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The Android testbed now distinguishes between stdout/stderr messages which
2+
were triggered by a newline, and those triggered by a manual call to
3+
``flush``. This fixes logging of progress indicators and similar content.

0 commit comments

Comments
 (0)