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

tea symbolic links should be development environment aware #667

Closed
felipecrs opened this issue Jul 30, 2023 · 10 comments
Closed

tea symbolic links should be development environment aware #667

felipecrs opened this issue Jul 30, 2023 · 10 comments

Comments

@felipecrs
Copy link
Contributor

felipecrs commented Jul 30, 2023

Otherwise things like pinning python version to 3.7 through .tea.yml and expecting VS Code's Python extension to pick it up from the PATH won't work.

The documentation mentions that for things where magic doesn't apply, like VS Code extensions for finding Python's version and so on, you can create a symbolic link (on Linux it should be hard links) so that VS Code can "see" tea's python as a binary in your PATH.

In the current state, tea's symlinks will only use the global tool version.

Example:

$ ln -vf $(which tea) ~/.local/bin/node
'/home/felipecrs/.local/bin/node' => '/home/felipecrs/.local/bin/tea'

$ which node
/home/felipecrs/.local/bin/node

$ cd $(mktemp -d)

$ echo 18.6.0 > .node-version

$ tea --env node --version
v18.6.0

$ node --version
v20.5.0

This is something Volta does by the way:

$ volta install node@20

$ ls -l $(which node)
lrwxrwxrwx 1 felipecrs felipecrs 37 Oct 25  2022 /home/felipecrs/.volta/bin/node -> /home/felipecrs/.volta/bin/volta-shim

$ node --version
v20.5.0

$ cd $(mktemp -d)

$ npm init -y

$ volta pin [email protected]

# Volta's symlinks parses the package.json to find the node version
$ node --version
v18.6.0
@mxcl
Copy link
Member

mxcl commented Jul 30, 2023

I think this only doesn't work because the dev-env wasn't updated, which is a separate “bug” that we have avoided fixing so far for performance reasons, but yep: it's confusing and not delightful.

If you were to cd . after changing .node-version I think it would work.

@felipecrs
Copy link
Contributor Author

@mxcl please note that tea --magic or tea --env isn't taken into account in my example (because VS Code won't have such thing either).

I believe the cd hook is only applicable in such cases, no?

@mxcl
Copy link
Member

mxcl commented Jul 30, 2023

oh ok, not a bug then, more a thing about how the symlinks to tea work, they are not dev-env aware without magic. The magic adds the env to the shell environment which tea then sees when it is invoked via symlink.

Which I think is the right choice.

magic is named accordingly and this feels like magical behavior.

Maybe I'm wrong… feel free to opine.

@felipecrs
Copy link
Contributor Author

felipecrs commented Jul 30, 2023

Sure, that sounds like magic indeed. But the issue I'm reporting is for when magic can't be used.

This is stated in the documentation:

In the latter example (VS Code), the editor runs within a development environment with a potential version of python or node (or anything else) "pinned" through .tea.yml.

However, when executing such binaries, VS Code won't have tea magic in place (unless an extension can overcome this limitation someday, but then we will have countless other editors or tools to tackle).

Thus, while creating symbolic links helps the editor to have some version of python ready, it does not help to have the correct version of python ready.

As mentioned above, this is something Volta addresses. Their symbolic links to node, npm or everything else first checks if they were invoked in a development environment and uses the correct version defined in such environment instead.

I can of course understand that such operation can be costly on the performance, but it should be doable (even if it requires special files like tea.lock.yaml to be present to work).

@felipecrs
Copy link
Contributor Author

felipecrs commented Jul 30, 2023

Maybe on first invocation it can take a longer time to "parse" the environment, but then subsequent calls can be speeded up by leveraging some cache mechanism. Example:

  1. Symlink invoked: check backwards recursively for a file named tea.yml (minimum performance impact)
  2. If one is found and defines such dependency, resolve its version (bigger performance impact IMO)
  3. Save the resolved version, the path and the checksum of such tea.yml to a cache file on ~/.tea
  4. Execute the binary with the resolved version

Later executions:

  1. Symlink invoked: check backwards recursively for a file named tea.yml (minimum performance impact)
  2. Check the cache for such tea.yaml, if it's there and it's valid (i.e. the checksum matches), use it (should have minimum performance impact as well, I believe)
  3. Execute the binary with the resolved version

@chudnyi
Copy link

chudnyi commented Jul 31, 2023

Thanks for great tool.

I expect behaviour of flag --env by default without having to set it on every call tea in my project.

@felipecrs
Copy link
Contributor Author

felipecrs commented Jul 31, 2023

That actually makes sense. Why not?

# no tea magic installed
$ tea node --version
v20.5.0

$ cd $(mktemp -d)

$ echo 18.17.0 > .node-version

# why not make --env on by default?
$ tea node --version
v18.17.0

# but we should allow to disable such behavior
$ tea --no-env node --version
v20.5.0

Again, if performance impact is the barrier, we can certainly work to keep it minimal.

@mxcl
Copy link
Member

mxcl commented Sep 11, 2023

v1 should fix these issues (envs are now more clearly created and less magical, so it's very clear when you have an active env) bar the primary one. v1 has tea install which installs a stub for a specific version of a tool.

However provided the dev environment is loaded (previously --env) that will take precendence, so this seems more like VSCode not respecting the dev environment which is a separate bug I intend to fix.

I'm considering though that perhaps our stubs should respect the dev env and otherwise use the version that was “installed” as that seems more correct, that could be done quite easily. Requires some thought. Please chime in!

@felipecrs
Copy link
Contributor Author

felipecrs commented Sep 11, 2023

I'm considering though that perhaps our stubs should respect the dev env and otherwise use the version that was “installed” as that seems more correct, that could be done quite easily. Requires some thought. Please chime in!

I totally agree. Functionality, it would work the same as I mentioned in #667 (comment), the difference being stubs in place of symlinks.

@mxcl
Copy link
Member

mxcl commented Oct 2, 2023

This is fixed per-se with the v1 release, though I think invocations of tools via scripting should operate in a dev-env aware fashion which currently they do not. I have made a note of this issue and we will discuss it in the attached forums.

@mxcl mxcl closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants