Skip to content

fix: set ASDF_INSTALL_* environment variables when running list-bin-paths script#2176

Closed
atoato88 wants to merge 3 commits intoasdf-vm:masterfrom
atoato88:set-envvar-for-reshim
Closed

fix: set ASDF_INSTALL_* environment variables when running list-bin-paths script#2176
atoato88 wants to merge 3 commits intoasdf-vm:masterfrom
atoato88:set-envvar-for-reshim

Conversation

@atoato88
Copy link
Copy Markdown
Contributor

@atoato88 atoato88 commented Sep 4, 2025

Summary

This PR sets ASDF_INSTALL_* environment variables when running list-bin-paths script.
See #2175 for detail.

Fixes: #2175

@atoato88 atoato88 requested a review from a team as a code owner September 4, 2025 21:32
Comment thread internal/shims/shims_test.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes asdf reshim’s invocation of the list-bin-paths plugin callback so it receives the documented ASDF_INSTALL_* environment variables, aligning behavior with the plugin authoring docs and resolving #2175.

Changes:

  • Pass ASDF_INSTALL_TYPE, ASDF_INSTALL_VERSION, and ASDF_INSTALL_PATH to the list-bin-paths callback.
  • Update shims code to thread conf and version into ExecutableDirs so it can compute and provide these env vars.
  • Add/adjust unit tests to validate env var availability and updated call signatures.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/shims/shims.go Sets ASDF_INSTALL_* env vars when running list-bin-paths via ExecutableDirs.
internal/shims/shims_test.go Updates ExecutableDirs tests for the new signature and adds coverage ensuring the callback can read ASDF_INSTALL_*.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

func TestExecutableDirs(t *testing.T) {
conf, plugin := generateConfig(t)
installVersion(t, conf, plugin, "1.2.3")
versionStruct := toolversions.Version{Type: "version", Value: "1.2.3"}
Comment thread internal/shims/shims_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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.

Hi @atoato88 , sorry for the late review here! Thanks for the PR, this is a good fix! I had some feedback and so did Copilot. I'm going to try to address everything myself and then get these changes merged.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes asdf reshim/shim generation behavior by ensuring the list-bin-paths plugin callback is executed with the documented ASDF_INSTALL_* environment variables available, aligning runtime behavior with plugin author expectations (per #2175).

Changes:

  • Pass ASDF_INSTALL_TYPE, ASDF_INSTALL_VERSION, and ASDF_INSTALL_PATH when running the list-bin-paths callback.
  • Update ExecutableDirs to accept conf and version so it can compute ASDF_INSTALL_PATH.
  • Add/adjust tests to validate the new environment variables are exposed to list-bin-paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
internal/shims/shims.go Sets ASDF_INSTALL_* env vars when invoking the list-bin-paths callback and updates call sites accordingly.
internal/shims/shims_test.go Updates tests for the new ExecutableDirs signature and adds a test asserting ASDF_INSTALL_* vars are visible in the callback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

func TestExecutableDirs(t *testing.T) {
conf, plugin := generateConfig(t)
installVersion(t, conf, plugin, "1.2.3")
versionStruct := toolversions.Version{Type: "version", Value: "1.2.3"}
func TestExecutableDirs(t *testing.T) {
conf, plugin := generateConfig(t)
installVersion(t, conf, plugin, "1.2.3")
versionStruct := toolversions.Version{Type: "version", Value: "1.2.3"}
assert.Nil(t, err)

executables, err := ExecutableDirs(conf, plugin, versionStruct)
assert.Nil(t, err)
@Stratus3D
Copy link
Copy Markdown
Member

Thanks for the PR @atoato88 ! Changes have been merged and will go out in the next patch release.

@Stratus3D Stratus3D closed this Apr 18, 2026
@atoato88
Copy link
Copy Markdown
Contributor Author

Thank you for the comment.
I appreciate the feedback.
This was the first time I'd seen Copilot edit the content of a comment. 😄

@atoato88 atoato88 deleted the set-envvar-for-reshim branch April 18, 2026 01:44
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.

bug: ASDF_INSTALL_* environment variables are not set when running the list-bin-paths script

3 participants