%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
This commit is contained in:
Miro Hrončok 2023-02-03 13:40:27 +01:00
parent a4d05ba2c2
commit 85fc41174d
3 changed files with 52 additions and 6 deletions

View File

@ -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

View File

@ -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):

View File

@ -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