fix: correct handling of `ASDF_FORCE_PREPEND` environment variable by machupicchubeta · Pull Request #2011 · asdf-vm/asdf

machupicchubeta · 2025-03-09T15:19:54Z

Summary

The error occurs if I set "yes" to FORCE_PREPEND env as described in the documentation.

Error message

strconv.ParseBool: parsing \"yes\": invalid syntax

Other Information

Referenced Documents

https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend

The error occurs if I set "yes" to `FORCE_PREPEND` env as described in the documentation. Error message: ``` strconv.ParseBool: parsing \"yes\": invalid syntax ``` Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend

internal/config/config_test.go

…ironment variable Those test cases aims to check whether `ForcePrepend` was actually set to the correct/expected value. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend

If the `ASDF_FORCE_PREPEND` environment variable is not set on macOS, the behavior is the same as when "yes" is set. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend

`ASDF_FORCE_PREPEND` is a specification that changes behavior depending on whether the string is "yes" or not. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend

internal/config/config.go

internal/config/config_test.go

See also: - https://pkg.go.dev/testing#T.Setenv

andrecloutier

Looks good to me!

Stratus3D · 2025-03-14T20:05:47Z

internal/config/config.go


 func defaultConfig(dataDir, configFile string) *Config {
+	forcePrepend := forcePrependDefault
+	forcePrependEnv := os.Getenv("ASDF_FORCE_PREPEND")


We might want to be able to handle inconsistent case here. Some folks might set this to YES by mistake and I think that should be handled.

I think it means case-insensitive. Sounds good to me.
I'll try to write a commit.

Stratus3D

Thanks for the PR @machupicchubeta ! I really appreciate the added tests!

machupicchubeta · 2025-03-15T06:45:21Z

@andrecloutier @Stratus3D
Thank you for the review and merge!

@Stratus3D

I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to YES by mistake and I think that should be handled.

@Stratus3D

I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to `YES` by mistake and I think that should be handled.

@andrecloutier

Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)

@andrecloutier

Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)

@andrecloutier

Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)

@andrecloutier

Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)

@andrecloutier

Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. BREAKING CHANGE: So far, `ASDF_FORCE_PREPEND` environment variable setting has been ignored, but this commit may change the behavior depending on the setting. If it is not set, macOS behavior will not change, but Linux behavior will change. If you want to get the same behavior as before, it is recommended to set the relevant environment variable. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)

@Stratus3D

I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to `YES` by mistake and I think that should be handled.

machupicchubeta requested a review from a team as a code owner March 9, 2025 15:19

machupicchubeta force-pushed the force_prepend branch from 654ba96 to 82c9dc0 Compare March 9, 2025 15:40

andrecloutier reviewed Mar 9, 2025

View reviewed changes

internal/config/config_test.go Outdated Show resolved Hide resolved

machupicchubeta force-pushed the force_prepend branch 3 times, most recently from 559004e to b017252 Compare March 10, 2025 15:21

machupicchubeta added 2 commits March 11, 2025 00:26

machupicchubeta force-pushed the force_prepend branch from b017252 to 934f15d Compare March 10, 2025 15:26

andrecloutier reviewed Mar 11, 2025

View reviewed changes

internal/config/config.go Show resolved Hide resolved

andrecloutier reviewed Mar 11, 2025

View reviewed changes

internal/config/config_test.go Outdated Show resolved Hide resolved

andrecloutier reviewed Mar 11, 2025

View reviewed changes

internal/config/config_test.go Outdated Show resolved Hide resolved

machupicchubeta added 2 commits March 12, 2025 20:58

refactor: Simplify

d76ef9b

fix: Use t.Setenv instead of os.Setenv in test codes

a930763

See also: - https://pkg.go.dev/testing#T.Setenv

andrecloutier approved these changes Mar 13, 2025

View reviewed changes

Stratus3D reviewed Mar 14, 2025

View reviewed changes

Stratus3D approved these changes Mar 14, 2025

View reviewed changes

Merge branch 'master' into force_prepend

485b2ae

Stratus3D changed the title ~~fix: Avoid ParseBool error~~ fix: correct handling of ASDF_FORCE_PREPEND environment variable Mar 14, 2025

Stratus3D merged commit 43a84a0 into asdf-vm:master Mar 14, 2025
9 checks passed

Stratus3D mentioned this pull request Mar 14, 2025

chore: release 0.16.6 #2007

Merged

machupicchubeta deleted the force_prepend branch March 15, 2025 06:42

machupicchubeta changed the title ~~fix: correct handling of ASDF_FORCE_PREPEND environment variable~~ fix: Correct handling of ASDF_FORCE_PREPEND environment variable Mar 15, 2025

machupicchubeta changed the title ~~fix: Correct handling of ASDF_FORCE_PREPEND environment variable~~ fix: correct handling of ASDF_FORCE_PREPEND environment variable Mar 15, 2025

machupicchubeta mentioned this pull request Mar 19, 2025

chore: ignore case for value of ASDF_FORCE_PREPEND environment variable #2026

Merged

machupicchubeta mentioned this pull request Mar 20, 2025

fix!: Be aware of ForcePrepend #2028

Closed

BrewTestBot mentioned this pull request Mar 21, 2025

asdf 0.16.6 Homebrew/homebrew-core#213897

Merged

Stratus3D mentioned this pull request Jun 7, 2025

chore(master): release 0.19.0 #2125

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct handling of `ASDF_FORCE_PREPEND` environment variable#2011

fix: correct handling of `ASDF_FORCE_PREPEND` environment variable#2011
Stratus3D merged 7 commits intoasdf-vm:masterfrom
machupicchubeta:force_prepend

machupicchubeta commented Mar 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrecloutier left a comment

Uh oh!

Stratus3D Mar 14, 2025

Uh oh!

machupicchubeta Mar 15, 2025

Uh oh!

Stratus3D left a comment

Uh oh!

Uh oh!

machupicchubeta commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

machupicchubeta commented Mar 9, 2025

Summary

Error message

Other Information

Referenced Documents

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrecloutier left a comment

Choose a reason for hiding this comment

Uh oh!

Stratus3D Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

machupicchubeta Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

Stratus3D left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

machupicchubeta commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants