Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/2972.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Patch get_interpreter to handle missing cache and app_data - by :user:`esafak`
19 changes: 19 additions & 0 deletions src/virtualenv/discovery/builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,25 @@ def get_interpreter(
cache=None,
env: Mapping[str, str] | None = None,
) -> PythonInfo | None:
"""
Find an interpreter that matches a given specification.

:param key: the specification of the interpreter to find
:param try_first_with: a list of interpreters to try first
:param app_data: the application data folder
:param cache: a cache of python information
:param env: the environment to use
:return: the interpreter if found, otherwise None
"""
if cache is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this make the extraction of the discovery not possible?

Copy link
Contributor Author

@esafak esafak Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion? We can sleep on it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not on top of my mind, but perhaps is indicative that we cannot extract discovery?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could keep this for a breaking change requiring the cache. Maybe other users will have ideas.

Copy link
Contributor

@gaborbernat gaborbernat Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if you don't have a good use case why introduce the breaking changes? We could revert this set of changes into a single commit and if anyone has good ideas to solve it in the future they can always revert our revert and we can cross that bridge at that point?

Sorry for bringing this up only now my bad I definitely should have realized earlier that this is a lot more breaking than I initially expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case was extraction of discovery in order to reduce the package's footprint, was it not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case was extraction of discovery in order to reduce the package's footprint, was it not?

Not entirely, the goal was to allow other people use it without deepending on the heavier size of the virtualenv project. If we're not going to achieve that goal today the benefit of this separation significantly decreases too. Not to mention, originally was requested pipx, whose development now stalled due to poeple moving to uv tool.

# Import locally to avoid a circular dependency
from virtualenv.app_data import AppDataDisabled # noqa: PLC0415
from virtualenv.cache import FileCache # noqa: PLC0415

if app_data is None:
app_data = AppDataDisabled()
cache = FileCache(store_factory=app_data.py_info, clearer=app_data.py_info_clear)

spec = PythonSpec.from_string_spec(key)
LOGGER.info("find interpreter for spec %r", spec)
proposed_paths = set()
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/discovery/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,18 @@ def test_returns_second_python_specified_when_more_than_one_is_specified_and_env
assert result == mocker.sentinel.python_from_cli


def test_get_interpreter_no_cache_no_app_data():
"""Test that get_interpreter can be called without cache and app_data."""
# A call to a valid interpreter should succeed and return a PythonInfo object.
interpreter = get_interpreter(sys.executable, [])
assert interpreter is not None
assert Path(interpreter.executable).is_file()

# A call to an invalid interpreter should not fail and should return None.
interpreter = get_interpreter("a-python-that-does-not-exist", [])
assert interpreter is None


def test_discovery_absolute_path_with_try_first(tmp_path):
good_env = tmp_path / "good"
bad_env = tmp_path / "bad"
Expand Down
Loading