gods breaks the world (a.k.a. sort considered harmful)

stevegt commented

2019-10-07 01:14:58 +00:00

(Migrated from github.com)

tl;dr for later readers who come here from google -- see the more concise dockerized example in https://github.com/emirpasic/gods/issues/123#issuecomment-539232054

Can we rename the sort example to something like godsort?

The name sort conflicts with a basic system command.
Many/most go tutorials tell the user to put $GOPATH/bin at the front of their $PATH.
Other tutorials recommend appending ... to a project path to install all of the child packages.

That combination of factors will break a UNIX or Linux machine. Here's a fun example:

$ which sort
/usr/bin/sort
$ sort /etc/hosts | wc -l
103
$ go get github.com/emirpasic/gods
can't load package: package github.com/emirpasic/gods: no Go files in /home/stevegt/go/1.12.7/src/github.com/emirpasic/gods
$ go get github.com/emirpasic/gods/...  # ok, then maybe I'm supposed to do this...

[... time passes, we forget we even installed gods that way ...]

$ hash -r
$ sort /etc/passwd | wc -l
0

Without the explicit hash -r, the failure mode is even more subtle, only showing up in new shells. I'm going to take the time to add some detail here, both as background and as google fodder for anyone else hitting this.

In my case the first symptoms were that vim-go wasn't running guru correctly a day after I installed a bunch of go dev bits. I spent a while troubleshooting vim-go, finally tracked it down to goenv, and then noticed that both pyenv init - and goenv init - were throwing errors when opening new xterms. Here's one of the error messages:

-bash: eval: line 188: syntax error near unexpected token `)'
-bash: eval: line 188: `  )'

That's it. The entirety of the message. That message was showing up when .bashrc ran eval "$(pyenv init -)". The message originated several shell scripts deep, in some code which was supposed to collect, sort, and uniq the content of $HOME/.pyenv/libexec and include it to the left of a ) in a case statement in a shell script that is generated on the fly and returned to the above eval. (Fun, right?)

It happens that goenv and pyenv use the same algorithm and similar shell scripts.

I found myself adding lots of echo statements to those various shell scripts trying to narrow it down -- never tried actually testing the output of sort itself until hours later, partly because it's buried in the middle of a pipeline. Even after I broke up the pipeline into little pieces and found the empty sort output, it still took me a few minutes to figure out what happened:

$ echo foo | sort    # what, no output?
$ sort    # not waiting on stdin either?
$ sort -h   # what?
$ sort --help  # um.
$ which sort
/home/stevegt/go/1.12.7/bin/sort
$ strings /home/stevegt/go/1.12.7/bin/sort | tail
github.com/emirpasic/gods/utils.StringComparator
github.com/emirpasic/gods/utils.sortable.Len
github.com/emirpasic/gods/utils.sortable.Swap
github.com/emirpasic/gods/utils.sortable.Less
github.com/emirpasic/gods/utils.init
github.com/emirpasic/gods/utils.(*sortable).Len
github.com/emirpasic/gods/utils.(*sortable).Less
github.com/emirpasic/gods/utils.(*sortable).Swap
main.main
main.init

_tl;dr for later readers who come here from google -- see the more concise dockerized example in https://github.com/emirpasic/gods/issues/123#issuecomment-539232054_ Can we rename the `sort` example to something like `godsort`? - The name `sort` conflicts with a basic system command. - Many/most go tutorials tell the user to put $GOPATH/bin at the front of their $PATH. - Other tutorials recommend appending `...` to a project path to install all of the child packages. That combination of factors will break a UNIX or Linux machine. Here's a fun example: ``` $ which sort /usr/bin/sort $ sort /etc/hosts | wc -l 103 $ go get github.com/emirpasic/gods can't load package: package github.com/emirpasic/gods: no Go files in /home/stevegt/go/1.12.7/src/github.com/emirpasic/gods $ go get github.com/emirpasic/gods/... # ok, then maybe I'm supposed to do this... [... time passes, we forget we even installed gods that way ...] $ hash -r $ sort /etc/passwd | wc -l 0 ``` Without the explicit `hash -r`, the failure mode is even more subtle, only showing up in new shells. I'm going to take the time to add some detail here, both as background and as google fodder for anyone else hitting this. In my case the first symptoms were that `vim-go` wasn't running `guru` correctly a day after I installed a bunch of go dev bits. I spent a while troubleshooting `vim-go`, finally tracked it down to `goenv`, and then noticed that both `pyenv init -` and `goenv init -` were throwing errors when opening new xterms. Here's one of the error messages: ``` -bash: eval: line 188: syntax error near unexpected token `)' -bash: eval: line 188: ` )' ``` That's it. The entirety of the message. That message was showing up when .bashrc ran `eval "$(pyenv init -)"`. The message originated several shell scripts deep, in some code which was supposed to collect, sort, and uniq the content of `$HOME/.pyenv/libexec` and include it to the left of a `)` in a `case` statement in a shell script that is generated on the fly and returned to the above `eval`. (Fun, right?) It happens that goenv and pyenv use the same algorithm and similar shell scripts. I found myself adding lots of `echo` statements to those various shell scripts trying to narrow it down -- never tried actually testing the output of `sort` itself until hours later, partly because it's buried in the middle of a pipeline. Even after I broke up the pipeline into little pieces and found the empty `sort` output, it still took me a few minutes to figure out what happened: ``` $ echo foo | sort # what, no output? $ sort # not waiting on stdin either? $ sort -h # what? $ sort --help # um. $ which sort /home/stevegt/go/1.12.7/bin/sort $ strings /home/stevegt/go/1.12.7/bin/sort | tail github.com/emirpasic/gods/utils.StringComparator github.com/emirpasic/gods/utils.sortable.Len github.com/emirpasic/gods/utils.sortable.Swap github.com/emirpasic/gods/utils.sortable.Less github.com/emirpasic/gods/utils.init github.com/emirpasic/gods/utils.(*sortable).Len github.com/emirpasic/gods/utils.(*sortable).Less github.com/emirpasic/gods/utils.(*sortable).Swap main.main main.init ```

emirpasic commented

2019-10-07 09:30:07 +00:00

(Migrated from github.com)

This library does not export sort as a standalone tool (so it should not go into bin) and looking at the result from which sort it seems that you have implemented this yourself using GoDS sort only as function. You essentially have to rename this within your code to compile your program into something else than sort. Let me know if I have understood the problem correctly here and if that solves your issue, last things GoDS should do is to break the World ;D

This library does not export sort as a standalone tool (so it should not go into bin) and looking at the result from `which sort` it seems that you have implemented this yourself using GoDS sort only as function. You essentially have to rename this within your code to compile your program into something else than `sort`. Let me know if I have understood the problem correctly here and if that solves your issue, last things GoDS should do is to break the World ;D

stevegt commented

2019-10-07 22:32:06 +00:00

(Migrated from github.com)

This library does not export sort as a standalone tool

But it does. You have an executable, package main, with a main() function in it, and the path is sort/sort.go. That will build an executable named sort. That executable will wind up in $GOPATH/bin every time anyone says go get github.com/emirpasic/gods/....

and looking at the result from which sort it seems that you have implemented this yourself

Actually, that's your code. Here it is again, this time in a pristine container, using Docker's official golang docker image -- nothing from me:

$ docker run --rm -it golang bash
root@c77392d55c2f:/go# echo $PATH
/go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
root@c77392d55c2f:/go# which sort
/usr/bin/sort
root@c77392d55c2f:/go# go get github.com/emirpasic/gods/...
root@c77392d55c2f:/go# hash -r
root@c77392d55c2f:/go# which sort
/go/bin/sort
root@c77392d55c2f:/go# strings /go/bin/sort | tail
github.com/emirpasic/gods/utils.StringComparator
github.com/emirpasic/gods/utils.sortable.Len
github.com/emirpasic/gods/utils.sortable.Swap
github.com/emirpasic/gods/utils.sortable.Less
github.com/emirpasic/gods/utils.init
github.com/emirpasic/gods/utils.(*sortable).Len
github.com/emirpasic/gods/utils.(*sortable).Less
github.com/emirpasic/gods/utils.(*sortable).Swap
main.main
main.init

Do you have access to a Linux machine or container where you can duplicate this? If so, please do so. If not, then you might want to take my word for it. I lost an entire afternoon to this, first figuring out what broke, then trying my best to provide a complete description of the problem for you, and even sending you pull request #124 so all you have to do is click a button. I included humor as much as I could to soften the blow, but seriously, this is bad.

> This library does not export sort as a standalone tool But it does. You have an executable, package `main`, with a `main()` function in it, and the path is `sort/sort.go`. That will build an executable named `sort`. That executable will wind up in $GOPATH/bin every time anyone says `go get github.com/emirpasic/gods/...`. > and looking at the result from `which sort` it seems that you have implemented this yourself Actually, that's your code. Here it is again, this time in a pristine container, using Docker's official golang docker image -- nothing from me: ``` $ docker run --rm -it golang bash root@c77392d55c2f:/go# echo $PATH /go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin root@c77392d55c2f:/go# which sort /usr/bin/sort root@c77392d55c2f:/go# go get github.com/emirpasic/gods/... root@c77392d55c2f:/go# hash -r root@c77392d55c2f:/go# which sort /go/bin/sort root@c77392d55c2f:/go# strings /go/bin/sort | tail github.com/emirpasic/gods/utils.StringComparator github.com/emirpasic/gods/utils.sortable.Len github.com/emirpasic/gods/utils.sortable.Swap github.com/emirpasic/gods/utils.sortable.Less github.com/emirpasic/gods/utils.init github.com/emirpasic/gods/utils.(*sortable).Len github.com/emirpasic/gods/utils.(*sortable).Less github.com/emirpasic/gods/utils.(*sortable).Swap main.main main.init ``` Do you have access to a Linux machine or container where you can duplicate this? If so, please do so. If not, then you might want to take my word for it. I lost an entire afternoon to this, first figuring out what broke, then trying my best to provide a complete description of the problem for you, and even sending you pull request #124 so all you have to do is click a button. I included humor as much as I could to soften the blow, but seriously, this is bad.

emirpasic commented

2019-10-07 22:47:54 +00:00

(Migrated from github.com)

@stevegt you are right, missed the fact that you stated this to be part of the example, was looking elsewhere given i glazed at this quickly from work.

yes, i am on a linux machine and i can see why it happens. strangely enough i have never noticed this issue. i suppose it's how i've configured my paths for linux tools to take higher precedence than go's bin path.

i'll merge your PR, but i suppose it's wise to do as i have done, rather than allowing some library to override your default linux commands in the background... that is something really really bad, scary actually! :D

@stevegt you are right, missed the fact that you stated this to be part of the example, was looking elsewhere given i glazed at this quickly from work. yes, i am on a linux machine and i can see why it happens. strangely enough i have never noticed this issue. i suppose it's how i've configured my paths for linux tools to take higher precedence than go's bin path. i'll merge your PR, but i suppose it's wise to do as i have done, rather than allowing some library to override your default linux commands in the background... that is something really really bad, scary actually! :D

👍 1

stevegt commented

2019-10-08 04:01:07 +00:00

(Migrated from github.com)

i'll merge your PR, but i suppose it's wise to do as i have done, rather than allowing some library to override your default linux commands in the background... that is something really really bad, scary actually! :D

Thank you! Yep, I'm going to change the way I configure $PATH on the machines I manage.

That vulnerable $PATH configuration appears in tutorials, the docker image, and in the setup for goenv, which is where I got it from: https://github.com/syndbg/goenv/blob/master/INSTALL.md.

At least the official Go docs don't have the vulnerability. In there, $GOPATH/bin goes last: https://tip.golang.org/doc/code.html#GOPATH

> i'll merge your PR, but i suppose it's wise to do as i have done, rather than allowing some library to override your default linux commands in the background... that is something really really bad, scary actually! :D Thank you! Yep, I'm going to change the way I configure $PATH on the machines I manage. That vulnerable $PATH configuration appears in tutorials, the docker image, and in the setup for goenv, which is where I got it from: https://github.com/syndbg/goenv/blob/master/INSTALL.md. At least the official Go docs don't have the vulnerability. In there, $GOPATH/bin goes last: https://tip.golang.org/doc/code.html#GOPATH

stevegt commented

2019-10-15 20:11:34 +00:00

(Migrated from github.com)

Just a follow-up note that the goenv package's PATH ordering during install is now fixed via pull request https://github.com/syndbg/goenv/pull/100. Thanks again, @emirpasic.

Just a follow-up note that the `goenv` package's PATH ordering during install is now fixed via pull request https://github.com/syndbg/goenv/pull/100. Thanks again, @emirpasic.

Rows
Columns

gods breaks the world (a.k.a. sort considered harmful) #123