-
-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
asdf integration + handling shims? #35
Comments
To be honest I stopped using pyenv's shims because they break everything, but this should be relatively easy to port without pretending to have pyenv IMO we should just add a specific adapter for That said, honestly it might work anyway because like I mentioned I ignore pyenv shims now anyway, they are terrible. |
When I say it might work anyway, I mean |
I’m thinking maybe we can introduce our own environment variable, so people can set to
This would have a sensitive default as well. Does asdf expose an environment variable similar to |
thanks @techalchemy, let me try that out @uranusjr yes it does have an equivalent variable. the only thing different is where the pythons are located
|
- Fixes #35 Signed-off-by: Dan Ryan <dan@danryan.co>
PR is up, let me know if it works for you |
@techalchemy that was super fast, thanks! I tried giving it a shot just now by replacing the vendored pythonfinder in pipenv but ran into an import error.
is there a better way to test? |
you have two options -- you can install pythonfinder directly: $ pip install -e git+https://github.com/sarugaku/pythonfinder.git@feature/asdf-support#egg=pythonfinder
$ pyfinder --findall or you can install the working branch I already ported this to (bearing in mind the pipenv repo is massive): pip intsall -e git+https://github.com/pypa/pipenv.git@update-vendor#egg=pipenv |
let me give that a shot. in the mean time, just ran into this pypa/pipenv#3214 while trying to install via pipenv |
I got this error (which has to do with shims)
The error might be confusing because my global python is set to both do you need to add: |
no it's just parsing your global file poorly... |
@AlJohri can you give me the output of |
I appreciate you taking the time out to figure this out!
Those two paths don't exist. I think you're looking for?
Here's a broader view of the tree structure if that helps:
pyenv is installed at the actual pythons are at:
and of course, the shims are at |
Ah yep exactly what I need. The file is a bit different. The pipenv issue is fixed on that branch also now |
Will handle this in the morning hopefully before I cut a release, assuming tests pass before that |
ok i think the new version will work... if you dont mind, it would be helpful if you could try it out |
sure, giving it a shot |
I tried with the same installation command as above, from the PR branch. with my global version set as before (
in case it was related to that, I tried to set my global version to just one python (
If you've spent too much time on this @techalchemy and need to cut a release, I can debug this over the weekend for a follow up release? |
Also, I set my global python to all three (
|
Note how all the pythons are pointing to shims instead of the true binaries though |
What's interesting is: py_version = get_python_version(path.path.absolute().as_posix())
instance_dict = cls.parse(py_version.strip()) This code is failing in the error -- I just swapped it to turn the path into an absolute path -- but then it raises this: if not isinstance(instance_dict.get("version"), Version) and not IGNORE_UNSUPPORTED:
raise ValueError("Not a valid python path: %s" % path.path)
return What's weird is it literally shouuld invoke every python it encounters on your path. I'm going to do two things -- remove the asdf shims and also double check the asdf version file -- is it actually located in def parse_asdf_version_order(filename=".tool-versions"):
version_order_file = normalize_path(os.path.join("~", filename))
if os.path.exists(version_order_file) and os.path.isfile(version_order_file):
with io.open(version_order_file, encoding="utf-8") as fh:
contents = fh.read()
python_section = next(iter(
line for line in contents.splitlines() if line.startswith("python")
), None)
if python_section:
python_key, versions = python_section.partition()
if versions:
return versions.split() Which I've just realized has no variable expansion so that may be the issue, let me push up the changes and we can see if that helps |
ok, branch pushed back up |
just installed the latest pipenv via homebrew and it seems like it was unable to find my asdf pythons? no errors though!
I'm just relieved those pesky shims are gone though 😂 |
Yes! That worked!
I will track down what's the canonical name env var name here asdf-vm/asdf#405 |
I got feedback from adsf on which env vars to rely on- I'll submit a PR with a fix. |
Hi, I recently switched over from pyenv to asdf which is a cross-language replacement. Now I don't need
pyenv
,rbenv
,nodenv
all installed separately.asdf uses python-build internally and installs everything into
~/.asdf/installs/python/
.$ ls -1 ~/.asdf/installs/python/ 2.7.15 3.6.7 3.7.1/
I was wondering if the pyenv integration was generic enough to be able to work with asdf as well
The binaries are located at
~/.asdf/installs/python/3.7.1/bin/python
for example which I think is identical to pyenv.I was thinking if I set
PYENV_ROOT=$HOME/.asdf/installs/python/
it might work out?I didn't have much success testing since the
shims
were getting in the way. I imagine this issue would be identical in pyenv or asdf. If my global version is set to 3.7.1, a shim would exist for pyhon3.6 and it would not be "active". For example, running the python3.6 shim while python3.7 is global would result in an error message like:So it has to go straight to PYENV_ROOT for the binaries and skip the shims. I think Pyenv shims break all attempted executions for pythons not in versions list #27 addesses this but wasn't sure if it would work generically enough for asdf as well.
EDIT: Looks like the shim path for asdf is
$HOME/.asdf/shims
so setting PYENV_ROOT as I did above would not work.Thanks!
The text was updated successfully, but these errors were encountered: