Skip to content
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

bug: direnv, no shims in PATH, asdf global, asdf direnv shell issue/question #149

Closed
scop opened this issue Apr 15, 2022 · 20 comments · Fixed by #169
Closed

bug: direnv, no shims in PATH, asdf global, asdf direnv shell issue/question #149

scop opened this issue Apr 15, 2022 · 20 comments · Fixed by #169
Labels
bug Something isn't working

Comments

@scop
Copy link
Contributor

scop commented Apr 15, 2022

Describe the Bug

I'm not sure if this is a bug in asdf/asdf-direnv/asdf-golang or just a pilot error, but I thought I'd try reporting it here anyway for thoughts.

The setup in question is

  • asdf v0.9.0, asdf shims not in PATH per the "pro tip"
  • asdf-golang current master, Go 1.17.8 and 1.18 installed with it, 1.17.8 set to global
  • asdf-direnv current master

I've implemented the "pro tip" basically by just this:

export ASDF_DIR=~/.asdf
PATH=$ASDF_DIR/bin:$PATH

...i.e. I'm not sourcing ~/.asdf/asdf.sh.

In this state, there are two problems

  1. the Go 1.17.8 global setting actually isn't global in a sense -- there is no go in $PATH at all
  2. I thought I could make one appear in a new shell with e.g. asdf direnv shell golang 1.18, but that doesn't work even though I'm getting "using asdf golang 1.18" in the output:
$ asdf direnv shell golang 1.18
direnv: using asdf golang 1.18
direnv: loading ~/.asdf/plugins/golang/bin/exec-env
$ go version
bash: go: command not found

To start to resolve this I've added ~/.envrc with contents use asdf. No ~/.tool-versions is present nor any other in effect (see correction in a comment below). That fixes 1) above, the Go set to global with asdf is now in my $PATH along with other its settings.

However, the problem with 2) remains. I would like to open a new shell with a different Go every now and then, and the way I read https://github.com/asdf-community/asdf-direnv#temporary-environments-for-one-shot-commands it would seem that asdf direnv shell golang x.xx.x should accomplish that. But it doesn't:

$ go version
go version go1.17.8 linux/amd64
$ asdf direnv shell golang 1.18
direnv: using asdf golang 1.18
direnv: loading ~/.asdf/plugins/golang/bin/exec-env
$ go version
go version go1.17.8 linux/amd64

I've found that a asdf global golang x.xx serves as a workaround for 2) -- it does change the Go version in the current shell (if the shell is using the global, naturally, and not overridden by a dir specific one). But the obvious problem is that it "leaks" to other new shells because of the altered global. asdf shell golang x.xx is not available, asdf barfs

Shell integration is not enabled. Please ensure you source asdf in your shell setup.

So, boils down to 2 questions:
a) Is adding use asdf to this effect in ~/.envrc something that one usually does with the "pro tip", or what's the "correct" way to get the asdf globals to work with it? If that's the way, perhaps amend the pro tip doc with that?
b) Is there something wrong with asdf direnv shell golang ..., or my use or expectation of it?

Steps to Reproduce

See above.

Expected Behaviour

I hope the description above covers this.

Actual Behaviour

I hope the description above covers this.

Debug Output

$ env ASDF_DIRENV_DEBUG=true direnv reload
direnv: loading ~/.envrc
direnv: using asdf
direnv: Creating env file /home/scop/.cache/asdf-direnv/env/1511531071-511866787-3894808579-3581694273
direnv: loading ~/.cache/asdf-direnv/env/1511531071-511866787-3894808579-3581694273
direnv: using asdf golang 1.17.8
direnv: loading ~/.asdf/plugins/golang/bin/exec-env
direnv: export +GOPATH +GOROOT ~PATH
$ cat ~/.cache/asdf-direnv/env/1511531071-511866787-3894808579-3581694273
### Do not edit. This was autogenerated by 'asdf direnv envrc' from /home/scop/.tool-versions ###
log_status using asdf golang 1.17.8
PATH_add /home/scop/.asdf/installs/golang/1.17.8/packages/bin
PATH_add /home/scop/.asdf/installs/golang/1.17.8/go/bin
ASDF_INSTALL_TYPE='version' ASDF_INSTALL_VERSION='1.17.8' ASDF_INSTALL_PATH='/home/scop/.asdf/installs/golang/1.17.8' source_env /home/scop/.asdf/plugins/golang/bin/exec-env
watch_file /home/scop/.tool-versions
@scop scop added the bug Something isn't working label Apr 15, 2022
@scop
Copy link
Contributor Author

scop commented Apr 15, 2022

One correction: I've found that there actually is a ~/.tool-versions around and in effect after doing e.g. a asdf global goenv 1.17.8 (with contents golang 1.17.8).

@scop scop changed the title bug: direnv, no shims in PATH, and asdf global issue/question bug: direnv, no shims in PATH, asdf global, asdf direnv shell issue/question Apr 15, 2022
@vic
Copy link
Contributor

vic commented Apr 15, 2022

a) Is adding use asdf to this effect in ~/.envrc something that one usually does with the "pro tip", or what's the "correct" way to get the asdf globals to work with it? If that's the way, perhaps amend the pro tip doc with that?

.envrc files are required by direnv to work. direnv will look for these files whenever you change directory and execute it's contents. in this case having a use asdf tells direnv to load the envrc file we generate from your project's .tool-versions.

b) Is there something wrong with asdf direnv shell golang ..., or my use or expectation of it?

I believe your expectation is right, I'd also expect asdf direnv shell golang 1.18 to load a new shell with that specific golang version (pretty much independently of what golang version is set on ~/.tool-versions or in $PWD/.tool-versions)

Indeed, this looks like a bug. We have a few tests for the shell command however none of them launches an interactive shell. I believe we could add one more test that $SHELL to a custom fake shell that simply prints what version was selected.

Could you please do the following? (running on your project directory) Be sure to run asdf current on the interactive shells and then exit them.

asdf current
go --version # or whatever flag you give to golang in order to print the version number
env ASDF_GOLANG_VERSION=1.17.8 asdf exec go --version 
env ASDF_GOLANG_VERSION=1.18 asdf exec go --version

# I believe this should work since we have tests for this use case.
env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18 -- go --version 

# This one - opening a new interactive shell - I believe wont work. This is the case we need tests for.
# My first guess is that the new shell is loading it's own shellrc and going back to golang 1.17.
env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18 # inside new shell run: `asdf current`.

# This one is the same as before but explicitly spawns a new bash shell without loading any shellrc.
env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18 -- bash --norc --noprofile # inside new shell run: `asdf current`

vic added a commit that referenced this issue Apr 15, 2022
One for testing that the tool version can override those from
global and local .tool-versions file.

One for testing that we spawn a SHELL and that it has the
correct version independent of global and local .tool-versions.

Related issue: #149
@vic
Copy link
Contributor

vic commented Apr 15, 2022

Just added a couple of tests which I believe we were missing.

These two new tests ensure that asdf direnv shell can provide a tool version independently of what's defined
in global and local .tool-versions files.

No code changes were necessary, so maybe adding those tests does not solve this issue and perhaps it's something on your environment we haven't yet figured out.

@scop
Copy link
Contributor Author

scop commented Apr 20, 2022

$ asdf current; go version
direnv          system          /home/scop/.tool-versions
golang          1.17.8          /home/scop/.tool-versions
perl            ______          No version is set. Run "asdf  perl "
go version go1.17.8 linux/amd64
$ env ASDF_GOLANG_VERSION=1.17.8 asdf exec go version
go version go1.17.8 linux/amd64
$ env ASDF_GOLANG_VERSION=1.18 asdf exec go version
go version go1.18 linux/amd64

So far as expected. But now it gets interesting -- this gives 1.17.8 vs the expected 1.18, i.e. not what your expectation above was:

$ env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18 -- go version

exhibit-a.txt

Next one fails pretty much exactly the same as the previous. asdf current shows 1.18 would be in effect, but 1.17.8 is what we get:

$ env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18

exhibit-b.txt

Last one fails exactly the same as the previous:

$ env ASDF_DIRENV_DEBUG=true asdf direnv shell golang 1.18 -- bash --norc --noprofile

exhibit-c.txt

@vic
Copy link
Contributor

vic commented Apr 20, 2022

Looking at exhibit-a.txt (and the code for asdf-golang) it looks like you have GOROOT and GOPATH are already set to those for go 1.17.8 before asdf direnv is loaded.

Perhaps, in your .envrc you could have:

unset GOROOT # clear system $GOROOT
unset GOPATH

use asdf  # the golang plugin should set correct GOROOT.

Screen Shot 2022-04-20 at 12 57 24

@vic
Copy link
Contributor

vic commented Apr 20, 2022

I'm also not a golang user, so I'm just guessing what if the executed binary is golang/1.8/bin/go but it finds a GOROOT for go 1.17, should go version report 1.17 ?

@scop
Copy link
Contributor Author

scop commented Apr 20, 2022

I believe GOROOT and GOPATH are set as result of use asdf. Anyway, adding the unsets before use asdf does not seem to make a difference. But I'll try to do some more testing to verify this.

I could not find evidence that go version would be affected by GOROOT.

@scop
Copy link
Contributor Author

scop commented Apr 20, 2022

Verified that it's the ~/.envrc and use asdf indeed that sets GOROOT and GOPATH. They are not set by anything else on my shell startup.

@vic
Copy link
Contributor

vic commented Apr 20, 2022

use asdf indeed that sets GOROOT and GOPATH

Yes, looks like asdf-golang exec-env does that, but ONLY if not already defined.

These lines (from your exhibit-a.txt file) correspond to asdf-golang's exec-env line 4 and 8. And the value of $GOROOT seems to have 1.17.8 in it, same for $GOPATH

++++ basename /home/scop/.asdf/plugins/golang/bin/exec-env
+++ . ./exec-env
++++ '[' 1.18 '!=' system ']'
++++ [[ unset == \/\h\o\m\e\/\s\c\o\p\/\.\a\s\d\f\/\i\n\s\t\a\l\l\s\/\g\o\l\a\n\g\/\1\.\1\7\.\8\/\g\o ]]
++++ [[ unset == \/\h\o\m\e\/\s\c\o\p\/\.\a\s\d\f\/\i\n\s\t\a\l\l\s\/\g\o\l\a\n\g\/\1\.\1\7\.\8\/\p\a\c\k\a\g\e\s ]]

however, on line 3, $ASDF_INSTALL_VERSION is correctly set as 1.18.

Looking at the output on exhibit-a.txt I cannot tell where is GOROOT being set to 1.17.

@vic
Copy link
Contributor

vic commented Apr 21, 2022

Same from exhibit-b.txt, asdf current correctly reports golang 1.18 defined by using ASDF_GOLANG_VERSION.

but again, GOROOT seems to be that of 1.17 which is reported by go version.

direnv: loading ~/.asdf/plugins/golang/bin/exec-env
++++ basename /home/scop/.asdf/plugins/golang/bin/exec-env
+++ . ./exec-env
++++ '[' 1.18 '!=' system ']'
++++ [[ unset == \/\h\o\m\e\/\s\c\o\p\/\.\a\s\d\f\/\i\n\s\t\a\l\l\s\/\g\o\l\a\n\g\/\1\.\1\7\.\8\/\g\o ]]
++++ [[ unset == \/\h\o\m\e\/\s\c\o\p\/\.\a\s\d\f\/\i\n\s\t\a\l\l\s\/\g\o\l\a\n\g\/\1\.\1\7\.\8\/\p\a\c\k\a\g\e\s ]]
...
++ exec /bin/bash
$ asdf current
direnv          system          /home/scop/.tool-versions
golang          1.18            ASDF_GOLANG_VERSION environment variable
perl            ______          No version is set. Run "asdf <global|shell|local> perl <version>"
$ go version
go version go1.17.8 linux/amd64

@scop
Copy link
Contributor Author

scop commented Apr 22, 2022

Guess there are kind of two problems here:

  1. What sets GOROOT and GOPATH? They are set at the point you refer to, so exec-env won't touch them, but nothing besides asdf/asdf-direnv/direnv touches them in my setup.
  2. Perhaps more interestingly, why is PATH set up kind of in incomplete manner after asdf direnv shell golang 1.18.1?
$ echo $PATH # starting point
/home/scop/.asdf/installs/golang/1.17.9/bin:/home/scop/.asdf/installs/golang/1.17.9/go/bin:/home/scop/.asdf/installs/golang/1.17.9/packages/bin:[...]
$ asdf direnv shell golang 1.18.1
direnv: using asdf golang 1.18.1
direnv: loading ~/.asdf/plugins/golang/bin/exec-env
$ echo $PATH
/home/scop/.asdf/installs/golang/1.18.1/bin:/home/scop/.asdf/installs/golang/1.17.9/bin:/home/scop/.asdf/installs/golang/1.17.9/go/bin:/home/scop/.asdf/installs/golang/1.17.9/packages/bin:[...]

Why only .../1.18.1/bin gets prepended? (Aside: it's useless, no such dir exists.) Why isn't .../1.18.1/go/bin (this would be the most important one) nor .../1.18.1/packages/bin like they are for 1.17.9?

Not really asking these questions from you, more like reminding myself where to pick up debugging more when I find some more time. Perhaps I should also try this out with some other plugin besides golang to see if it fares any better.

@scop
Copy link
Contributor Author

scop commented Apr 22, 2022

#151 fixes the incomplete $PATH setup. Problem with $GOPATH and $GOROOT remains, will try to find time for that soonish.

@scop
Copy link
Contributor Author

scop commented Apr 22, 2022

use asdf indeed that sets GOROOT and GOPATH

Yes, looks like asdf-golang exec-env does that, but ONLY if not already defined.

Hmm, this could arguably be a bug and it should possibly instead set both unconditionally. (Well in my case I would prefer that it would never touch either no matter what, but that's an orthogonal issue.) asdf's plugin creator help isn't really helpful on this.

vic added a commit that referenced this issue Apr 22, 2022
This helps prevent bugs like those uncovered on #151.

Related issue #149.
scop added a commit to scop/asdf-golang that referenced this issue Apr 23, 2022
Otherwise, existing values from the environment may continue to point to
a wrong version.

Refs asdf-community/asdf-direnv#149
@scop
Copy link
Contributor Author

scop commented Apr 23, 2022

I've filed asdf-community/asdf-golang#80 which fixes the remaining issues for me. In the meantime, I'm doing

use asdf
unset -v GOROOT GOPATH

...in all my .envrcs, which implements my preferred behavior.

Leaving this open still to consider and resolve this (or not) from the initial comment:

a) Is adding use asdf to this effect in ~/.envrc something that one usually does with the "pro tip", or what's the "correct" way to get the asdf globals to work with it? If that's the way, perhaps amend the pro tip doc with that?

@pikeas
Copy link

pikeas commented Apr 28, 2022

a) Is adding use asdf to this effect in ~/.envrc something that one usually does with the "pro tip", or what's the "correct" way to get the asdf globals to work with it? If that's the way, perhaps amend the pro tip doc with that?

Just bit by this as well on a new setup. I removed ~/.asdf/shims and have ASDF_DEFAULT_TOOL_VERSIONS_FILENAME=~/.config/asdf/tool-versions. That file has python 3.10.4, which is managed by asdf, but without the shim directory on $PATH, I get my system Python instead.

(And because recent versions of homebrew Python create only python3.x and not python, the system appears even more broken.)

This leaves new asdf-direnv users with a "broken" setup, or at least one that no longer works the way it used to until further modifications are made. The intention here is good, to completely remove the shim lookup. However, it would be great to update the docs with an explicit note about the consequences, as many asdf users have asdf shims taking precedence over system-installed versions on $PATH.

vic added a commit that referenced this issue Apr 28, 2022
@vic
Copy link
Contributor

vic commented Apr 28, 2022

Added an assertion during our shell command tests, to make sure those test are done having asdf shims out of PATH. Since that seems to be mentioned as cause for the apparent bug. But I've had no luck reproducing the problem. If anyone is able to provide a failing test we can start fixing it and make sure it's fixed for once.

@vic
Copy link
Contributor

vic commented Apr 29, 2022

Tried to reproduce this myself with no luck. Having no asdf-shims on $PATH (only asdf/bin), having a global ASDF_DEFAULT_TOOL_VERSIONS_FILENAME and a local tool-versions file. Doing asdf direnv shell with a totally different version (not the same in global/local tool-versions files). And everything looks just fine :/

Screen Shot 2022-04-29 at 17 42 52

@scop
Copy link
Contributor Author

scop commented May 2, 2022

I can see a few differences between @pikeas' report and @vic's reproducer try:

  • the former mentions the problem being fallback to the system version, latter does not mention system version being in the mix at all (don't know if there's a system node version installed)
  • the former is for python, the latter tries reproducing with node

Don't know if either of these makes a difference, but I suppose they could, and it would be good to try eliminating the differences in search of a reproducer.

@pikeas
Copy link

pikeas commented May 2, 2022

What I experienced should be reproducible:

  • clean PATH with no shim dir
  • no sourcing of asdf on shell startup (bashrc, config.fish, etc)

When invoking an interpreter, it will necessarily be one that is not managed by asdf, because asdf's directories aren't on PATH. That's exactly as expected and intended. The reason I commented is documentation - the "pro-tip" encourages removing the shim dir from PATH, and so it would nice if the docs a) remind/warn users that this will change their system behavior and b) make recommendations on other ways to achieve the old behavior (add use asdf to ~/.envrc, tell them they shouldn't want this, etc).

scop added a commit to scop/asdf-golang that referenced this issue Jan 1, 2023
Otherwise, existing values from the environment may continue to point to
a wrong version.

Refs asdf-community/asdf-direnv#149
jfly added a commit that referenced this issue Feb 3, 2023
@jfly
Copy link
Contributor

jfly commented Feb 3, 2023

@pikeas, I think that's a good suggestion to add some documentation about the consequences of removing the shims. I've done that over in #169.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants