From 85fc41174dbdbc26fd3b456c08d6c15dc623e91e 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.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2166888
---
 pyproject-rpm-macros.spec              |  6 ++++-
 pyproject_buildrequires.py             | 34 ++++++++++++++++++++++----
 pyproject_buildrequires_testcases.yaml | 18 ++++++++++++++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/pyproject-rpm-macros.spec b/pyproject-rpm-macros.spec
index 16bd9d9..ea02e7f 100644
--- a/pyproject-rpm-macros.spec
+++ b/pyproject-rpm-macros.spec
@@ -10,7 +10,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
@@ -146,6 +146,10 @@ 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
+- Fixes: rhbz#2166888
+
 * 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