More respectful convention for caching

[aanghelidi]

A PR that aims to close #342.

All the logic is inside _get_internal_cache_path in _cache.py.

[woodruffw]

Thanks @aanghelidi, this looks pretty reasonable to me. pip-audit's choice of cache location isn't a stability commitment we make, so we should be fine to change this without making a major release.

A couple of thoughts:

• Maybe we could use platformdirs to abstract the cache location discovery for us? OTOH the current logic in _get_internal_cache_path is pretty simple; curious what you think.
• Maybe we should blow away the old cache if a new path is discovered? Otherwise, users will end up silently keeping _PIP_AUDIT_INTERNAL_CACHE around in their $HOME taking up space, which isn't very polite of us.

For the second point, something like this is what I have in mind:

# if the selected internal cache isn't the old one, delete the old one.
if internal_cache_path != _PIP_AUDIT_LEGACY_INTERNAL_CACHE:
    shutil.rmtree(_PIP_AUDIT_LEGACY_INTERNAL_CACHE)

CC @di for thoughts as well 🙂

[aanghelidi]

Thanks @woodruffw for the review 🙂 TIL about platformdirs ! This is indeed a really handy package. Regarding the aforementioned points:

• There is several pros about using platformdirs:

  • simplicity. Using platformdirs, the implementation of pip-audit's cache location is a one-liner.
  • we do not reinvent the wheel. This means less maintenance for the project.
  • consistency. Upon research, it seems that platformdirs is already used in several pypa projects:
    • hatch: see here
    • pipx: see here
    • setuptools: see here (as a vendored library)

• Regarding the potential con(s):

  • dependency cost. We add one more dependency to the project. Although platformdirs itself has no dependencies, it's still a decision to make. Maybe this is not worth adding given the simplicity of _get_internal_cache_path implementation (which I have the feeling could be simplified).

• Regarding PIP_AUDIT_INTERNAL_CACHE, it is indeed not nice to keep the old cache in $HOME. Removing it seems to be a good idea. Maybe, as an alternative, using shutil.move to move the old cache to the new location could be an idea?

I added a commit using platformdirs for the cache location, let me know what you think 🙂

[woodruffw]

• Regarding PIP_AUDIT_INTERNAL_CACHE, it is indeed not nice to keep the old cache in $HOME. Removing it seems to be a good idea. Maybe, as an alternative, using shutil.move to move the old cache to the new location could be an idea?

I think we should just blow it away for now -- #794 covers some larger architectural changes we need to make to handle newer versions of pip's cache anyways, so these old paths may not be super useful when moved to the new location.

I added a commit using platformdirs for the cache location, let me know what you think 🙂

LGTM, thank you!

[aanghelidi]

I think we should just blow it away for now -- #794 covers some larger architectural changes we need to make to handle newer versions of pip's cache anyways, so these old paths may not be super useful when moved to the new location

Thank you for the context @woodruffw ! I added a extra commit to delete the old cache that goes into that direction. Let me know what you think 🙂

[woodruffw]

No need for a helper here; let's just inline this into _get_cache_dir IMO.

[aanghelidi]

The last commit now inline the deletion into _get_cache_dir ✔️

[woodruffw]

Thanks @aanghelidi, LGTM!

[woodruffw]

Sorry, one last request: could you add a CHANGELOG entry for this change? See the other entries in the log for our preferred style.

[hseg]

Tests fails on my machine -- with $XDG_CACHE_HOME=~/.local/var/cache/, I get

WARNING  pip_audit._cache:_cache.py:81 pip 1.0.0 doesn't support the `cache dir` subcommand, using /home/gesh/.local/var/cache/pip-audit instead
FAILED test/test_cache.py::test_get_cache_dir_do_not_use_pip[on Linux] - AssertionError: assert PosixPath('/home/gesh/.local/var/cache/pip-audit') == PosixPath('/home/gesh/.c...
FAILED test/test_cache.py::test_get_cache_dir_pip_disabled_in_environment[on Linux] - AssertionError: assert PosixPath('/home/gesh/.local/var/cache/pip-audit') == PosixPath('/home/gesh/.c...
FAILED test/test_cache.py::test_get_cache_dir_old_pip[on Linux] - AssertionError: assert PosixPath('/home/gesh/.local/var/cache/pip-audit') == PosixPath('/home/gesh/.c...

which is odd since I have

$ pip --version
pip 25.0 from /usr/lib/python3.13/site-packages/pip (python 3.13)

To clarify, the AssertionError is (in one example)

def test_get_cache_dir_old_pip(monkeypatch, sys_platform, expected):
        # Check the case where we have an old `pip`
        monkeypatch.setattr(cache, "_PIP_VERSION", Version("1.0.0"))
        # Check cross-platforms
        _patch_platformdirs(monkeypatch, sys_platform)
    
        # When we supply a cache directory, always use that
        cache_dir = _get_cache_dir(Path("/tmp/foo/cache_dir"))
        assert cache_dir.as_posix() == "/tmp/foo/cache_dir"
    
        # In this case, we can't query `pip` to figure out where its HTTP cache is
        # Instead, we use `~/.pip-audit-cache`
        cache_dir = _get_cache_dir(None)
>       assert cache_dir == expected
E       AssertionError: assert PosixPath('/home/gesh/.local/var/cache/pip-audit') == PosixPath('/home/gesh/.cache/pip-audit')

test/test_cache.py:136: AssertionError

[hseg]

Judging by the log output and the testcases, I suspect what's going on is you've
correctly implemented the "check XDG variables and fallback" logic, but are not
accounting for it in the test cases.
In that case, it'd be enough to unset $XDG_CACHE_HOME in the three
above-mentioned tests (and add a test exercising the expected behaviour that if
$XDG_CACHE_HOME is set but pip cannot be relied on to give the path the
obtained cache directory is indeed $XDG_CACHE_HOME/pip-audit -- perhaps by
setting $XDG_CACHE_HOME to a canary value)