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 goenv install for 1.3.1 and later #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alext
Copy link

@alext alext commented Sep 5, 2014

There was a problem with the vercomp function that caused it to echo
more than once for some comparisons. This would appear to be a side
effect of changing the stackoverflow function to echo instead of using
return value.

Comparing 1.2 and 1.3.1 is one of the cases that was echoing more than
once. This then caused the test for "$rtn" == "1" to be false, which
meant that it was trying to use the old source URL.

There was a problem with the vercomp function that caused it to echo
more than once for some comparisons.  This would appear to be a side
effect of changing the stackoverflow function to echo instead of using
return value.

Comparing 1.2 and 1.3.1 is one of the cases that was echoing more than
once.  This then caused the test for `"$rtn" == "1"` to be false, which
meant that it was trying to use the old source URL.
@meatballhat
Copy link

👏 :shipit:

(assuming it works! 😆)

@alext
Copy link
Author

alext commented Sep 9, 2014

@Solistra I can't reproduce that issue. It downloaded 1.0.3 for me correctly. Additionally, I extracted the function into this test script:

#! /usr/bin/env bash

function vercomp () {
    # http://stackoverflow.com/questions/4023830
    # 0: '='
    # 1: '>'
    # 2: '<'
    if [[ $1 == $2 ]]
    then
        echo 0
        return
    fi
    local IFS=.
    local i ver1=($1) ver2=($2)
    # fill empty fields in ver1 with zeros
    for ((i=${#ver1[@]}; i<${#ver2[@]}; i++))
    do
        ver1[i]=0
    done
    for ((i=0; i<${#ver1[@]}; i++))
    do
        if [[ -z ${ver2[i]} ]]
        then
            # fill empty fields in ver2 with zeros
            ver2[i]=0
        fi
        if ((10#${ver1[i]} > 10#${ver2[i]}))
        then
            echo 1
            return
        fi
        if ((10#${ver1[i]} < 10#${ver2[i]}))
        then
            echo 2
            return
        fi
    done
#    echo 0
}

for ver in 1.0 1.0.3 1.1 1.1.1 1.2 1.2.1 1.3 1.3.1; do
  echo "${ver}         $(vercomp ${ver} 1.2)"
done

and it returned the values expected:

1.0       2
1.0.3       2
1.1       2
1.1.1       2
1.2       0
1.2.1       1
1.3       1
1.3.1       1

alext added a commit to alphagov/packager that referenced this pull request Sep 9, 2014
@jszwedko
Copy link

jszwedko commented Oct 8, 2014

👍 any news on this @wfarr ?

@alext alext changed the title Fix goenv install for 1.3.1 Fix goenv install for 1.3.1 and later Jan 23, 2015
alext added a commit to alphagov/packager that referenced this pull request Jan 24, 2015
This switches to my fork of goenv because the upstream doesn't seem to
be active any more. Version 0.0.6 includes 2 fixes:

* fix goenv install to work with the new download URLs for Go 1.3.1+
  (wfarr/goenv#13)
* update goenv exec to allow running any command on the PATH
  (wfarr/goenv#17)
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