-
Notifications
You must be signed in to change notification settings - Fork 838
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
fix: properly pass missing env vars in Plugin.Update #1899
Conversation
8a9ae5a
to
e782ebc
Compare
…and unsetting it in `env_teardown` until asdf-vm/asdf#1899 is merged
internal/plugins/plugins.go
Outdated
@@ -255,6 +255,9 @@ func (p Plugin) Update(conf config.Config, ref string, out, errout io.Writer) (s | |||
} | |||
|
|||
env := map[string]string{ | |||
"HOME": os.Getenv("HOME"), | |||
"ASDF_DIR": os.Getenv("ASDF_DIR"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ASDF_DIR
isn't something that is used in asdf 0.16.0+. Are there still plugins relying on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there still plugins relying on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why we shouldn't propagate all env variables of the current process by default here (similar to #1958 )?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @Stratus3D thank you!
e782ebc
to
74d22f9
Compare
74d22f9
to
6b5a9fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if plugin callbacks need arbitrary environment variables I think we should pass-through any environment variable that is already set. Like we do for shims when executing the real executable.
6b5a9fb
to
69b55ca
Compare
69b55ca
to
71e8cbb
Compare
Closed in favor of #1982 |
Replaced by #1982 |
@Stratus3D Closed in favor of #1982 |
Summary
Properly pass missing env vars in Plugin.Update
Fixes:
Thank you!