fix: append pip errors to log file rather than overwrite

[corneliusroemer]

Resolves #1132

An alternative would be to create new log files for each pip invocation, but this would require deeper changes. Appending seems like a decent solution as a first fix.

• I have added an entry to docs/changelog.md

Summary of changes

Test plan

Tested by running

python src/pipx install visidata pycowsay --python "3.11"
python src/pipx reinstall-all --python "3.12"

The logs now contain pip error output from both errors: visidata and pycowsay. Exactly what should happen.

[gaborbernat]

Automated tests please.

[corneliusroemer]

@gaborbernat I'm struggling to think of a good way to produce multiple error messages in a deterministic way. The PR that added the log files tests the functionality by doing pipx install weblate=4.3.1 which seems to be a package spec for which install fails, but since there's no pipx install-all I can't think of how to extend this to produce multiple errors. Any thoughts?

pipx/tests/test_install.py

Lines 256 to 266 in 9639e2c

	def test_install_pip_failure(pipx_temp_env, capsys):
	assert run_pipx_cli(["install", "weblate==4.3.1", "--verbose"])
	captured = capsys.readouterr()
	assert "Fatal error from pip" in captured.err
	pip_log_file_match = re.search(r"Full pip output in file:\s+(\S.+)$", captured.err, re.MULTILINE)
	assert pip_log_file_match
	assert Path(pip_log_file_match[1]).exists()
	assert re.search(r"pip (failed|seemed to fail) to build package", captured.err)

The whole logfile content doesn't seem to be tested at all. Just the existence of the error log file. It could be empty and tests would pass at the moment, it seems.

[gaborbernat]

The whole logfile content doesn't seem to be tested at all. Just the existence of the error log file. It could be empty and tests would pass at the moment, it seems.

Yeah, this would fix that. Can't you do this:

python src/pipx install visidata pycowsay --python "3.11"
python src/pipx reinstall-all --python "3.12"

but use the same python version, with the seeded pacakges?

[corneliusroemer]

Sorry I'm not quite sure what you mean by "seeded packages".

I'm also not sure that this particular combination of packages and python versions will be a stable way to get multiple errors in the future.

[gaborbernat]

These https://github.com/pypa/pipx/blob/main/testdata/tests_packages/primary_packages.txt#L1, you can pick to known failing versions there

[corneliusroemer]

Hmm, there's only one expected fail that I can see weblate==4.3.1 True # expected fail in tests and I'm not sure how I can reinstall a package that fails to install in the first place.

I managed to get multiple errors due to a bug that will get fixed eventually. The only way I can think of getting multiple errors is an upgrade where the old versions installed, but the new latest ones throw errors.

This would require two packages that fail for the latest version in perpetuity. Not sure something like that exists on Pypi.

[corneliusroemer]

What if we added a test to check basic content of the error file, but not this particular case of multiple errors in one file? Append will only produce different results in a small subset of use cases and it's much harder to test.

[gaborbernat]

Ok 👌

[gaborbernat]

Seems stalled, we can reopen if you pick it up again.