moovweb / gvm

Go Version Manager
http://github.com/moovweb/gvm
MIT License
10.32k stars 538 forks source link

gvm listall fails because version sort not available on mac #305

Open nohupped opened 6 years ago

nohupped commented 6 years ago

Hi there, After installing gvm, when I run a gvm listall, it fails with the output

~ ⬢ v6.10.2 🐹 v1.4
➜ gvm listall

gvm gos (available)

sort: invalid option -- V
Try `sort --help' for more information.

and the error I guess is coming from the commit d2a19aeb3b1ec738c25f970f7c59390dfebf9f25 which does a | sort -V. I am running from mac and my sort version is

➜ sort --version
sort (GNU coreutils) 5.93
Copyright (C) 2005 Free Software Foundation, Inc.
This is free software.  You may redistribute copies of it under the terms of
the GNU General Public License <http://www.gnu.org/licenses/gpl.html>.
There is NO WARRANTY, to the extent permitted by law.

Written by Mike Haertel and Paul Eggert.

Removing the -V is working for me.

The sort version that comes with my mac which is my work laptop is quite old, when compared to the ones that ship with linux, so I was wondering if it is possible to have a backward compatibility?

Thank you.

Brent-Knight-BL commented 6 years ago

I solved this locally by installing the gnu coreutils via brew:

brew install coreutils

and modifying the script to use gsort -V instead of sort -V

But this isn't a general fix because it would break Linux installs.

You could generalize this a little and make it work better across MOSX and Linux with a pattern like this (after telling MOSX users to brew install coreutils):

Setup:

SORT=$(which gsort || which sort)

and then at the call site(s):

$SORT

or

"${SORT}"

for example:

"${SORT}" -V

which ought to work on MOSX and Linux.

Brent-Knight-BL commented 6 years ago

Ah, good news: it looks like you have a SORT_PATH defined in tools, which would be a great place to make this change, centrally.

Also, this means there is a bug in listall, because sort is being invoked instead of "${SORT_PATH}", which is probably what was intended.

BenKnigge commented 4 years ago

If you have the time a pr would be greatly appreciate.