Skip to content

Commit

Permalink
TemporaryPipInstallResolver: Prevent pip install from polluting stdout
Browse files Browse the repository at this point in the history
I noticed this while playing around with --install-deps and --json, and
at the same time piping the JSON output into `jq` for filtering/colors:
The `pip install` that is triggered by --install-deps will print status
messages to both stdout and stderr, and since these are not handled by
our subprocess.run() call, they will appear directly on _our_
stdout/stderr. When piping our stdout to `jq` this causes `jq` to get
those status messages (preceding the JSON output) on its stdin, and it
predictably aborts with an error message.

This commit fixes the issue in two ways:

First, we pass --quiet and --disable-pip-version-check to `pip install`,
to silence some common/unnecesary output. This seems sufficient to make
`pip install` silent when everything works as expected. However, when
warnings and errors might still be printed in exceptional cases.

Next, we capture the stdout + stderr from `pip install` and only print
them in case the `pip install` command failed.

With both of these fixes, we ensure that `pip install` only makes noise
when it fails, and that noise only appears on our stderr (in the shape
of a warning log message). For example, if I try to --install-deps with
an intentional typo ("pyyamll):

  WARNING:fawltydeps.packages:Command failed: ['/run/user/1000/tmpr5l6m94i/bin/pip', 'install', '--no-deps', '--quiet', '--disable-pip-version-check', 'pyyamll', 'scikit-learn']
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  WARNING:fawltydeps.packages:Failed to install 'pyyamll'
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  ERROR:fawltydeps.main:Unresolved dependencies: pyyamll
  FawltyDeps is unable to find the above packages with the configured package resolvers. Consider using --pyenv if these packages are already installed somewhere, or --custom-mapping-file to take full control of the package-to-import-names mapping.
  • Loading branch information
jherland committed Aug 29, 2023
1 parent 3f7c9a6 commit b909900
Showing 1 changed file with 21 additions and 3 deletions.
24 changes: 21 additions & 3 deletions fawltydeps/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,33 @@ def installed_requirements(
marker_file = venv_dir / ".installed"
if not marker_file.is_file():
venv.create(venv_dir, clear=True, with_pip=True)
argv = [f"{venv_dir}/bin/pip", "install", "--no-deps"]
proc = subprocess.run(argv + requirements, check=False)
# Capture output from `pip install` to prevent polluting our own stdout
pip_install_runner = partial(
subprocess.run,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
check=False,
)
argv = [
f"{venv_dir}/bin/pip",
"install",
"--no-deps",
"--quiet",
"--disable-pip-version-check",
]
proc = pip_install_runner(argv + requirements)
if proc.returncode: # pip install failed
logger.warning("Command failed: %s", argv + requirements)
if proc.stdout.strip():
logger.warning("Output:\n%s", proc.stdout)
logger.info("Retrying each requirement individually...")
for req in requirements:
proc = subprocess.run(argv + [req], check=False)
proc = pip_install_runner(argv + [req])
if proc.returncode: # pip install failed
logger.warning("Failed to install %s", repr(req))
if proc.stdout.strip():
logger.warning("Output:\n%s", proc.stdout)
marker_file.touch()
yield venv_dir

Expand Down

0 comments on commit b909900

Please sign in to comment.