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

fix: checking version was too strict #92

Merged
merged 3 commits into from
Aug 1, 2024

Conversation

lnc3l0t
Copy link
Contributor

@lnc3l0t lnc3l0t commented Jul 30, 2024

In my system I have, for some reasons, a developer version of fzf:

> fzf --version
0.29 (devel)

Although this version meets the minimum required version of 0.29.0 specified by MIN_FZF_VERSION="0.29.0", the check_version function incorrectly fails the check.

This PR addresses the issue by modifying the check_version function. Specifically, the function now allows the version check to pass when the installed version has one fewer segment than the threshold version, provided that the missing segment in the threshold version is 0. For instance, if the threshold version is 0.29.0, then an installed version of 0.29 is considered sufficient, as the trailing 0 does not imply a higher version requirement.

Therefore my additional check

if (( i == ${#threshold_parts[@]} - 1 )) && ((${#ver_parts[@]} == ${#threshold_parts[@]} - 1)) && ((${threshold_parts[i]} == 0));

result in the case tool=0.29 (2 parts) and threshold=0.29.0 (3 parts) and i=2 in

if (( 2 == 3-1 )) && ((2 == 3-1)) && ((0 == 0));

check_version now doesn't fail when `tool` has version `0.X` and `threshold` is set to `0.X.0`
@LangLangBart
Copy link
Collaborator

Thanks, I can reproduce the issue as well. I will look into it soon.

@LangLangBart
Copy link
Collaborator

This will fail if the user has version 1 and the required version is 1.0.0.


can u try ?

--- a/gh-notify
+++ b/gh-notify
@@ -619,5 +619,5 @@ select_notif() {
 check_version() {
     local tool=$1 threshold=$2 on_error=${3:-die}
-    local user_version
+    local user_version user_version_part index
     declare -a ver_parts threshold_parts
     user_version=$(command $tool --version 2>&1 |
@@ -628,8 +628,9 @@ check_version() {
     IFS='.' read -ra threshold_parts <<<"$threshold"
 
-    for i in "${!threshold_parts[@]}"; do
-        if ((i >= ${#ver_parts[@]})) || ((ver_parts[i] < threshold_parts[i])); then
+    for index in "${!threshold_parts[@]}"; do
+        user_version_part=${ver_parts[index]:-0}
+        if ((user_version_part < threshold_parts[index])); then
             $on_error "Your '$tool' version '$user_version' is insufficient. The minimum required version is '$threshold'."
-        elif ((ver_parts[i] > threshold_parts[i])); then
+        elif ((user_version_part > threshold_parts[index])); then
             break
         fi

As @LangLangBart suggested a tool with version "1"
would pass the check against version "1.0"
but not against "1.0.0".

It now works for whatever subversion

NOTE: code is also cleaner
@lnc3l0t
Copy link
Contributor Author

lnc3l0t commented Aug 1, 2024

I agree with you @LangLangBart. Your version works for me.

Thank you, I didn't know the bash-variable-default-value syntax ${var:-def}.

@LangLangBart LangLangBart merged commit 556df2e into meiji163:main Aug 1, 2024
1 check passed
@LangLangBart
Copy link
Collaborator

Thank you.

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