From ecf0a140a8c1a7a3d2beea6c39027001a90b5f91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= <miro@hroncok.cz>
Date: Fri, 3 Feb 2023 13:40:27 +0100
Subject: [PATCH] %pyproject_buildrequires: Avoid leaking stdout from
 subprocesses

When the build backend prints to stdout via non-Python means,
for example when a setup.py script calls a verbose program via os.system(),
the output leaked to stdout of %pyproject_buildrequires was treated as generated BuildRequires.

Fore example, if the setup.py script has:

    rv = os.system('/usr/bin/patch -N -p3 -d build/lib < lib/py-lmdb/env-copy-txn.patch')

(From https://github.com/jnwatson/py-lmdb/blob/py-lmdb_1.0.0/setup.py#L117)

The stdout of /usr/bin/patch leaked to stdout of %pyproject_buildrequires:

    [lmdb-1.0.0]$ /usr/bin/python3 -Bs /usr/lib/rpm/redhat/pyproject_buildrequires.py --python3_pkgversion 3 2>/dev/null
    python3dist(setuptools) >= 40.8
    python3dist(wheel)
    patching file lmdb.h
    patching file mdb.c
    python3dist(wheel)
    patching file lmdb.h
    patching file mdb.c

This resulted in DNF errors like this:

    No matching package to install: 'lmdb.h'
    No matching package to install: 'mdb.c'
    No matching package to install: 'patching'

Moreover, it resulted in bogus BuildRequires that may have existed (e.g. "file").

By replacing the usage of contextlib.redirect_stdout
(which only redirects Python's sys.stdout)
with a custom context manager that captures stdout on file descriptor level
(in addition to Python's sys.stdout),
we avoid this leak.

File descriptor magic heavily inspired by the capfd pytest fixture.

Related: rhbz#2168193
---
 pyproject-rpm-macros.spec              |  5 +++-
 pyproject_buildrequires.py             | 34 ++++++++++++++++++++++----
 pyproject_buildrequires_testcases.yaml | 18 ++++++++++++++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/pyproject-rpm-macros.spec b/pyproject-rpm-macros.spec
index 667cda0..f1c991c 100644
--- a/pyproject-rpm-macros.spec
+++ b/pyproject-rpm-macros.spec
@@ -12,7 +12,7 @@ License:        MIT
 #   Increment Y and reset Z when new macros or features are added
 #   Increment Z when this is a bugfix or a cosmetic change
 # Dropping support for EOL Fedoras is *not* considered a breaking change
-Version:        1.6.0
+Version:        1.6.1
 Release:        1%{?dist}
 
 # Macro files
@@ -148,6 +148,9 @@ export HOSTNAME="rpmbuild"  # to speedup tox in network-less mock, see rhbz#1856
 
 
 %changelog
+* Fri Feb 03 2023 Miro Hrončok <mhroncok@redhat.com> - 1.6.1-1
+- %%pyproject_buildrequires: Avoid leaking stdout from subprocesses
+
 * Fri Jan 20 2023 Miro Hrončok <miro@hroncok.cz> - 1.6.0-1
 - Add pyproject-srpm-macros with a minimal %%pyproject_buildrequires macro
 
diff --git a/pyproject_buildrequires.py b/pyproject_buildrequires.py
index 01c57f1..323ab2a 100644
--- a/pyproject_buildrequires.py
+++ b/pyproject_buildrequires.py
@@ -4,9 +4,9 @@ import os
 import sys
 import importlib.metadata
 import argparse
+import tempfile
 import traceback
 import contextlib
-from io import StringIO
 import json
 import subprocess
 import re
@@ -48,11 +48,35 @@ from pyproject_convert import convert
 
 @contextlib.contextmanager
 def hook_call():
-    captured_out = StringIO()
-    with contextlib.redirect_stdout(captured_out):
+    """Context manager that records all stdout content (on FD level)
+    and prints it to stderr at the end, with a 'HOOK STDOUT: ' prefix."""
+    tmpfile = io.TextIOWrapper(
+        tempfile.TemporaryFile(buffering=0),
+        encoding='utf-8',
+        errors='replace',
+        write_through=True,
+    )
+
+    stdout_fd = 1
+    stdout_fd_dup = os.dup(stdout_fd)
+    stdout_orig = sys.stdout
+
+    # begin capture
+    sys.stdout = tmpfile
+    os.dup2(tmpfile.fileno(), stdout_fd)
+
+    try:
         yield
-    for line in captured_out.getvalue().splitlines():
-        print_err('HOOK STDOUT:', line)
+    finally:
+        # end capture
+        sys.stdout = stdout_orig
+        os.dup2(stdout_fd_dup, stdout_fd)
+
+        tmpfile.seek(0)  # rewind
+        for line in tmpfile:
+            print_err('HOOK STDOUT:', line, end='')
+
+        tmpfile.close()
 
 
 def guess_reason_for_invalid_requirement(requirement_str):
diff --git a/pyproject_buildrequires_testcases.yaml b/pyproject_buildrequires_testcases.yaml
index 75934df..1a2e6bb 100644
--- a/pyproject_buildrequires_testcases.yaml
+++ b/pyproject_buildrequires_testcases.yaml
@@ -818,3 +818,21 @@ Pre-releases are accepted:
     python3dist(wheel)
   stderr_contains: "Requirement satisfied: cffi"
   result: 0
+
+
+Wrapped subprocess prints to stdout from setup.py:
+  installed:
+    setuptools: 50
+    wheel: 1
+  include_runtime: false
+  setup.py: |
+    import os
+    os.system('echo LEAK?')
+    from setuptools import setup
+    setup(name='test', version='0.1')
+  expected: |
+    python3dist(setuptools) >= 40.8
+    python3dist(wheel)
+    python3dist(wheel)
+  stderr_contains: "HOOK STDOUT: LEAK?"
+  result: 0