Closed soulmerge closed 5 years ago
I understand that you are trying to use vex in a somewhat unusual way that you would like to see work. I'd like to help. But since I wrote this and I'm the only person who has to backstop it, I reserve the right not to merge patches unless I like them. So, here is what I need to merge your patch.
I would like you to describe exactly what behavior you are trying to fix, so I can understand precisely what the intended effect of the patch is. These links don't tell me what your issue is with vex.
It would be nice if you could briefly address why this merits a change in vex per se - and not just because you want your patch merged because that will change the behavior. vex is just platform-agnostic Python code, so unless there is a bug in Python's ability to run platform-agnostic python code on OS X, we are kludging around a bug in specific OS X environments, which isn't the same as fixing an obvious bug in vex itself. What caused this to begin to be a problem, or only a problem in very specific environments? Are we addressing the real issue?
I need the patch to include a test which can be run automatically to determine that the intended effect has been achieved. The test should not contain e.g. if-checks against the platform. For your own sake, please don't bother working further on this patch unless some discussion has happened.
I'd also like you to explain more about why this change is the right approach, because I don't believe in making changes that are not well understood.
In this case, I'm concerned that we might be trying to use vex to modify the behavior of software other than vex. There is a constant temptation to throw more process-specific kludges into vex which is really intended and designed to be process-agnostic, and I want to resist that when I can because there is no logical stopping point. (for example, some people want vex to try to alter the way bash runs specifically in a complicated effort to guess what the user must want, instead of just treating bash as any other process and letting the user keep the control).
It makes me especially itchy to clobber undocumented bits of state used by external apps, because this just accumulates kludgy cruft and makes things clumsier for users who might want that state to pass through. I don't believe in "DWIM" and I'd rather resist the urge to guess. At minimum I need to understand what is consuming PYVENV_LAUNCHER and why that consumer cannot just stop being stupid. Environment variables are a mechanism for communication; how do I know what it is supposed to mean to delete PYVENV_LAUNCHER?
On the other hand, if we assume this patch is good enough for OS X, then why not just delete the same key on all platforms? By making another platform-specific branch we end up with a less-tested branch that is likely to accumulate bugs - particularly since we are interacting with a bit of state used in unspecified ways by unspecified third parties, which are liable to churn that breaks code making assumptions about things that aren't documented.
vex takes on the burden of directly interacting with virtualenv peculiarities because virtualenv's functionality isn't exposed as stable API, but the kludging has to stop somewhere. Why not here?
@sashahart This isn't all the detail you requested, but I can confirm simply removing the environment variable is the correct approach. This is how pipenv
resolves the problem and that PR also references tox
resolving it the same way.
https://github.com/pypa/pipenv/pull/113
And, just to clarify, I'm not using vex in any non standard way. This problem can be reproduced on OSX with:
vex -mr test
pip install flake8
flake8 --version # which fails
head -n 1 ~/.virtualenvs/test/bin/flake8
#!/Library/Frameworks/Python.framework/Versions/3.6/bin/python3.6
I think I made myself really clear on my expectations here, and after literally years no real engagement has happened.
I keep noticing that a lot of people on Github are talking in a harmfully imprecise way, exemplified here: subjective judgments or demands phrased as what "should" be done or what is "correct" without any of the kind of detail I've requested here. You can't confirm that removing the environment variable is "the correct approach" because that isn't a matter of program correctness, it is a matter of people asking for vex to include a workaround for other software's bugs. By just saying "correct approach" you are tacitly conflating your opinion, or someone else's opinion which you agree with, with "correct" and in the process you are substituting simple statements for the kinds of detail which I clearly requested.
Just doing whatever (enough) people say is "correct" without really looking at the issue is not how I work and I don't think it's how anyone should work.
If people go around and lobby the authors of a couple different tools to include some kludge, it doesn't make it more "correct" for every other tool to include the same kludge.
The correct approach is to solve bugs in the code that creates them. I have actual reasons for believing that. But the more time I take to explain my reasoning, the more time I waste as people simply make demands in issues without engaging on the technical details and that I don't accept
There is an issue with framework builds on Mac OS X. The details are explained perfectly in the following issue in the official virtualenv package: https://github.com/pypa/virtualenv/issues/322
In summary: the environment value
__PYVENV_LAUNCHER__
needs to be unset before entering the virtual environment. Here is the commit, that solved the issue in virtualenv: https://github.com/pypa/virtualenv/commit/6793d5f1099963b7fe20d8a0ddfb431ef09699a7