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

feat: refactor version/aliases resolution #348

Merged

Conversation

augustobmoura
Copy link
Member

One of the main points pursued by asdf is to keep development environment reproducible and consistent across many machines. Therefore, versions must always be pinned on version files (including legacy version files). This issue has been discussed on multiple occasions, and the definitive position of asdf is that we shouldn't allow dynamic ranges/aliases in favor of being a deterministic tool. Every developer should have the same installed version.

We currently need a better implementation of aliases. The current behavior unintentionally allows unpinned versions inside .tool-version files, and it has become an endless source of bugs and bad interactions. I consider this first implementation a mistake, and we are probably better off without it.

However, given the overwhelming request for the functionality (#127, #193, #295, #334, #327). It is fair to offer an escape hatch from the regular version resolution algorithm, so users can tune the plugin to behave as they are used to. In the NodeJS world, having partial versions is a well-documented and prevalent approach. Ignoring this does more harm than good.

The new implementation

On this PR, I'm getting rid of the old symlink approach to aliases, it is more trouble than it's worth it, and there are a lot of foot guns when adding new code to the plugin (and asdf-core). It offers a limited amount of flexibility and unintentionally introduces a lot of non-deterministic behavior.

With the new approach, we offer a way for users to tap into the legacy version file parsing strategy. .tool-versions files will be unaltered, meaning they will only allow pinned full versions, as expected from asdf.

A new sub-command (asdf nodejs latest, akin to asdf latest) is also introduced to help achieve the two most requested strategies for resolving partial and codenames, latest-installed and latest-available. Users can also provide scripts to tailor their needs.

The user needs to set up the script for the dynamic resolution manually. This will be done by setting an environment variable containing the script for resolution:

export ASDF_NODEJS_RESOLVE_VERSION_CMD='asdf nodejs latest "$1"'

@augustobmoura augustobmoura force-pushed the feat/new-version-resolution branch from 68de13b to c73d0b7 Compare April 20, 2023 01:47
@augustobmoura
Copy link
Member Author

@jthegedus @Stratus3D, can you check this PR? I've been hacking ideas on how to solve the problem with the need for custom version resolutions for a few months already. And the best approach would be to give flexibility to users while guiding them to the correct deterministic path.

The default behavior would always be the simplest one. We only allow pinned full versions. However, users would now be able to customize the resolution for legacy version files. This is a big problem that is currently driving a lot of users away. That means they would need to explicitly decide and set up a way to handle dynamic versions if they wish to. We give some examples and some helpers, but the choice to do it would be from the user experience

@augustobmoura augustobmoura force-pushed the feat/new-version-resolution branch from c73d0b7 to ecac2cf Compare April 20, 2023 02:06
@augustobmoura augustobmoura force-pushed the feat/new-version-resolution branch from ecac2cf to 3d0239a Compare April 20, 2023 02:07
@Stratus3D
Copy link
Member

Hi @augustobmoura ! Sorry for the late review here. I've been crazy busy for the last couple of weeks and am just now getting around to open source PRs. I've read the description of this PR and I like it, I'll read the code next and get back to you in an hour or so.

Copy link
Member

@Stratus3D Stratus3D left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @augustobmoura ! I left some comments on the code, but you are much more familiar with this project than I am now, so please do whatever you think best here.

Overall I think this is an improvement, my only general concern here it might be hard for some users to know when code names can be used and when they cannot. I have a good understanding what the legacy version file feature is, but some users may not.

Thank you for all your work on this project!

@augustobmoura augustobmoura force-pushed the feat/new-version-resolution branch from f6956ff to 52cbbeb Compare May 18, 2023 17:03
@criaprime
Copy link

I still needed to be able to use lts-hydrogen in .tool-versions. It looks like rtx supports this: https://github.com/jdxcode/rtx

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

Successfully merging this pull request may close these issues.

None yet

3 participants