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: Allow for custom global tool-versions location #1295

Closed
wants to merge 1 commit into from
Closed

feat: Allow for custom global tool-versions location #1295

wants to merge 1 commit into from

Conversation

brianvanburken
Copy link
Contributor

Summary

Since a recent bug fix to the behaviour around the env variable ADSF_DEFAULT_TOOL_VERSIONS_FILENAME, which I also abused to follow the XDG spec, I was unable to move my global .tool-versions. As was also reported by other people in #1248.

This PR allows the user to give a custom location via ASDF_CONFIG_DIR if set (for those who want it to move the file and don't follow the XDG spec), XDG_CONFIG_HOME, or falls back to HOME.

I'm open to suggestions if another approach is better.

Related issues: #687

@brianvanburken brianvanburken requested a review from a team as a code owner July 12, 2022 17:58
@jthegedus
Copy link
Contributor

jthegedus commented Jul 13, 2022

I only took a quick look, but I think there are some aspects missing from this. Importantly is the resolution of .tool-versions, it is just a walk up the directory tree, where the location of global is not special and simply likely to be part of the users current working directory. Your solution would break this behaviour.

I also think we could utilise Bats test conventions to unset the XDG value in a single place instead of in each individual test.

@jwierzbo
Copy link

@brianvanburken instead of using ASDF_CONFIG_DIR, could you provide new ENV - ASDF_GLOBAL_TOOL_VERSIONS_PATH? That would be more handful to use and also all logic could stay in place

@jthegedus
Copy link
Contributor

ASDF_GLOBAL_TOOL_VERSIONS_PATH would still require more work from this PR to actually work though, as current lookup of Global is at $HOME/ADSF_DEFAULT_TOOL_VERSIONS_FILENAME. We would need to change the lookup method.

@brianvanburken
Copy link
Contributor Author

@jthegedus Thank you for taking the time to look at my PR! I'll improve the testing using your feedback.

I'm not so familiar with the project so bear with me. I looked at the code as how it resolves global and local .tool-versions. As far as I can see, there are two branches; one branch checks directly the location of the global files. The other branch looks locally in the folder and walks upward as you described. My PR targets the global file and allows users to move this using different options. I think walking up the directory tree for the file is irrelevant, if I'm not mistaken. Could you tell me more specifically what aspect I'm missing here?

As for the ASDF_GLOBAL_TOOL_VERSIONS_PATH, it is a small change to make. But, I'm looking at a bit bigger picture. I saw that there is a ASDF_DATA_DIR variable where everything data related is stored for asdf. So in the same spirit, using ASDF_CONFIG_DIR, we could also implement a future change to store .asdfrc in that directory.

I'm currently finishing up some things at work before my vacation, so I'm a bit swamped. When I return, I'll finish the implementation in one week.

@brianvanburken
Copy link
Contributor Author

I've switched from asdf and won't know about the project and bash, along with time, to finish this. If anyone wants to pick up where I left off, feel free to take my code

@brianvanburken brianvanburken deleted the feature/custom-global-tool-versions branch April 18, 2023 12:57
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.

3 participants