`_install_tool()` needs to output pip's stderr and stdout

[2bndy5]

cpp-linter-hooks/cpp_linter_hooks/util.py

Lines 62 to 72 in 069417d

	def _install_tool(tool: str, version: str) -> Optional[Path]:
	"""Install a tool using pip, suppressing output."""
	try:
	subprocess.check_call(
	[sys.executable, "-m", "pip", "install", f"{tool}=={version}"],
	stdout=subprocess.DEVNULL,
	stderr=subprocess.DEVNULL,
	)
	return shutil.which(tool)
	except subprocess.CalledProcessError:
	return None

It seems this function fails with _install_tool("clang-format", "9"). It would be nice to know why, but this function does not forward any output from pip when invoking pip exits with non-zero code.

Proposal

For a full descriptiopn of what happened:

    try:
        subprocess.run(
            [sys.executable, "-m", "pip", "install", f"{tool}=={version}"],
            capture=True,
            check=True,
        )
        return shutil.which(tool)
    except subprocess.CalledProcessError as exc: 
        LOG.error("pip failed to install %s %s", tool, version)
        LOG.error(exc.stdout.decode(encoding="utf-8"))
        LOG.error(exc.stderr.decode(encoding="utf-8"))
        return None

[shenxianpeng]

Suppressing its standard output was intentional as a pre-commit hooks. I could support it when the user passes the --verbose option the the pre-commit hooks

[2bndy5]

What I propose is to only output the stdout and stderr pipes when there is an error. Otherwise, we (and the users) can't tell what went wrong when invoking pip. For example, see the CI log that led me here

[2bndy5]

If you are still unsure, the function should return either a Path (when success) or the pip output (when failed). That way the error can be adequately handled upstream.