If we need to, we should be able to make them affected by the option again. This seems like the most likely change to impact people, so it's worth being aware of how we might cleanly roll it back without undoing all the work.

Right, this should be relatively easy, asPyConfig already has asite_import field, which we can use to check to change the behavior.

I think the specifics of a possible rollback would depend on the reported issues. I would like to still keep the new behavior long term, if there are any issues, I think we should consider rolling back with a deprecation period, but depending on how critical and in what way the new behavior is problematic, we may just want to roll back permanently.

These warnings are also worth watching out for, though hopefully they only ever indicate actual problems.

If the values don't match, should we update them anyway like we used to?

Good point! We could run this through a deprecation period, setting the value for now and stopping in two versions, though, I think triggering these warnings likely highlights a bug in user code and giving how niche use-cases like these are, I am not sure if that's needed, and it's extra overhead on our part. I'm inclined to keep the code as-is, and if we see any issues during the pre-release phase, change to the deprecation approach before a final release.

@hugovk do you have any preference as the RM?

No preference as RM.

You could have the warnings for one release, then based on feedback (if any!) decide if you want to deprecate/change behaviour.

Let's go with the warnings and re-evaluate based on feedback, then.

This change makeshome a mandatory field inpyvenv.cfg I believe (mandatory in the sense that you will get a warning if it isn't set). There is no such constraint in PEP 405 (https://peps.python.org/pep-0405/).

I found this because I am digging into a different problem related to resolving executable symlinks when creating venvs (#106045).

IMO, the logic for determining what the path should be belongs in one place (getpath now) and this additional check that guarantees that thehome is exactlydirname(executable) is redundant and not necessarily a correct/standardised assumption.

Ifhome is absent, then we just can't launch Python. The PEP may not have realised just how impossible it is to launch Python when nobody will tell you where Python is installed.

The PEP may not have realised just how impossible it is to launch Python when nobody will tell you where Python is installed.

I guess you are talking about the case where you have a venv based on copy, not on symlinks? Because for the latter, I think we have all we need.

I'm glad that@FFY00 pointed me at#127895, as I findhome inpyvenv.cfg to be a strange choice which forces us to heuristics in order to (hopefully) find theactual executable.

This change makes home a mandatory field in pyvenv.cfg

I think this should be documented, as it may be something that people have designed around. For example, there is a comment ingetpath.py which states:

Currently, we don't consider the presence of a pyvenv.cfg file without a 'home' key to signify the existence of a virtual environment — we quietly ignore them.

This comment is now outdated (it isn't quiet 😂)

if home is absent, you can't launch python
venv based on symlinks

Yeah, like@pelson says, pyvenv.cfg having home wasn't actually necessary when symlinks were used. The logic would follow the symlink and treat that as home (or more specifically, use it as a potential candidate).

Dereferencing the symlink to find home seems pretty intentional? There's various calls to realpath() that inform later logic. Similarly the top-of-file comments describe this behavior (i.e. it was intentional prior to this change).

home key is now required; if its missing its not considered a venv

For rules_python, this becomes problematic because it creates a conundrum:

• We can't write an absolute path and files are read-only after building an application.
• The home keymust be an absolute path (if it's relative (and empty means ".", iirc), its relative to the current working directory, which could be anywhere)

The net effect is there isn't any content we can write to pyvenv.cfg that allows Python to find its home and recognize the venv.

I like all these changes! Makes the intent much clearer. The overall change it is worth it for these, IMHO.

Yes, and from playing with thegetpath, I believe a significant portion of the code could be simplified and better written to show intent by strictly adhering to the defined input/output parameters, as listed inhttps://docs.python.org/3/c-api/init_config.html#python-path-configuration. I found a lot of the complexity comes from taking into account non-input parameters, which as far as I can tell, is done to make writing the tests easier, by allowing them to overwrite certain parameters, mostly to get around file system checks. This feels wrong, and makes thegetpath extremely difficult to parse and understand what is supposed to be happening. It could be avoided by improving the test suite, maybe by making the helper functions operate on a mocked file system.

I found a lot of the complexity comes from taking into account non-input parameters, which as far as I can tell, is done to make writing the tests easier, by allowing them to overwrite certain parameters, mostly to get around file system checks.

It's certainly intended to be a pure module, I'd hate to see "real" file system checks showing up in too many places. Those should be done first and passed in as data, or at least handled through the functions made available for executinggetpath (which are easily mocked out for tests).

But other than that, it basically follows the flow of thegetpath.c andgetpathp.c files that I translated to get here. So yeah, it's a mess, but only because they were a worse mess! With controlled input/output parameters and very controlled external state, it should be possible to safely refactor the whole thing now.

if home is absent, you can't launch python
venv based on symlinks

Yeah, like@pelson says, pyvenv.cfg having home wasn't actually necessary when symlinks were used. The logic would follow the symlink and treat that as home (or more specifically, use it as a potential candidate).

Dereferencing the symlink to find home seems pretty intentional? There's various calls to realpath() that inform later logic. Similarly the top-of-file comments describe this behavior (i.e. it was intentional prior to this change).

home key is now required; if its missing its not considered a venv

For rules_python, this becomes problematic because it creates a conundrum:

• We can't write an absolute path and files are read-only after building an application.
• The home keymust be an absolute path (if it's relative (and empty means ".", iirc), its relative to the current working directory, which could be anywhere)

The net effect is there isn't any content we can write to pyvenv.cfg that allows Python to find its home and recognize the venv.