Bug/remove unused vendor libraries #198

[feuGeneA]

Remove unused vendor libraries

Below shell transcript shows that we had 5 unused vendor libraries, spanning 3698 lines of code and taking up 872 K of space. It also shows that eliminating the identified unused vendor libraries did not lead to any subsequent vendor libraries being unused.

Below shell transcript can be reproduced if you first paste the following into the command prompt. The resulting function will write unused vendor libraries to stdout, and will write all vendor library references to stderr. It presumes that you're within a status-go checkout whose root directory is named status-go.

  list_unused_vendor_libraries() {
    for vendorPath in `git rev-parse --show-toplevel`/vendor/* ; do \
      pushd $vendorPath >/dev/null ; \
      for library in * ; do \
        pushd ../../.. >/dev/null ; \
        echo library $(basename ${vendorPath})/$library references: >&2 ; \
        grep -nHre "$(basename ${vendorPath})/$library" ./status-{go,react} \
          | grep -ve "^./status-go/vendor/$(basename ${vendorPath})/$library" \
          | grep -ve "git/index" \
          | grep -ve ".sw[a-z] " >&2 \
    
        if [ $? -ne 0 ]; then \
          echo "$(basename ${vendorPath})/$library " ; \
          echo "unreferenced library $(basename ${vendorPath})/$library " >&2 ; \
        fi ; \
        popd >/dev/null ; \
      done ; \
      popd >/dev/null ; \
    done
  }

here's what we're deleting in this PR:

gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ list_unused_vendor_libraries 2>/dev/null
github.com/cnf
github.com/eapache
github.com/Gustav-Simonsson
github.com/microsoft
github.com/pkg
gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ cloc `list_unused_vendor_libraries 2>/dev/null`
      52 text files.
      50 unique files.
      16 files ignored.

http://cloc.sourceforge.net v 1.60  T=0.11 s (317.2 files/s, 42684.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Go                              33            571            573           3679
YAML                             3              2              0             19
-------------------------------------------------------------------------------
SUM:                            36            573            573           3698
-------------------------------------------------------------------------------
gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ du -k `list_unused_vendor_libraries 2>/dev/null` | cut -f 1 - | paste -s -d+ - | bc
872
gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ git rm -r `list_unused_vendor_libraries 2>/dev/null`
...
gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ list_unused_vendor_libraries 2>/dev/null
gene@precision5510:~/go/src/github.com/status-im/status-go/vendor$ 

Solves #198

• remove unused dependencies and propose script for doing so
• check whether github.com/{daaku,kardianos} can also be removed.
• investigate segfaulting e2e/node test in Travis run (result: also failing on upstream/origin/develop)

[andytudhope]

Rocking it @feuGeneA - will get the team to review when they can, GoTeam has been under the whip this week 😉

[divan]

Thanks for the contribution! Mentioned issue is superseded by #223 — so the proper solution would be to use go dep (and we had pending PR for that, I guess), but anyway, manually removing unused packages is also a nice thing to have.

Having that, I don't think we want to commit list-unused-vendor-libraries.sh into the source tree (as we're switching to go dep at some point anyway). So if you can remove .sh script from list of files added, it looks good to merge.

[adambabik]

@feuGeneA I found two more that might be unused. Can you please check these as well?

vendor/github.com/daaku
vendor/github.com/kardianos

[feuGeneA]

@adambabik those both look familiar to me. I will check on them.

@divan yes I saw the go dep stuff but this issue looked like an easy way to get my feet wet. :) Are you sure we shouldn't commit this script? What's the timeline on switching to go dep? How does that compare to the frequency with which vendor changes? I would suggest keeping the script around until the switch to go dep actually happens; but of course I defer to you on that.

[feuGeneA]

I see that in Travis the e2e/node test segfaulted with my changes in place. I'll have to look into that too.

[divan]

What's the timeline on switching to go dep?

Yesterday :)

How does that compare to the frequency with which vendor changes?

Good question. Vendor changes rarely and we really should switch to go dep asap, there is no reason not to do it. And introducing custom scripts to maintain source tree is one of the things that we really want to avoid.

[feuGeneA]

@adambabik they sounded familiar because I had seen them reported in the stderr of my shell command. In reviewing the report I had noticed them because they didn't have any references from .go files.

daaku is mentioned in status-react:

./status-react/node_modules/walker/package.json:36:    "url": "https://github.com/daaku/nodejs-walker/issues"
./status-react/node_modules/walker/package.json:45:  "homepage": "https://github.com/daaku/nodejs-walker",
./status-react/node_modules/walker/package.json:56:    "url": "git+https://github.com/daaku/nodejs-walker.git"
./status-react/node_modules/tmpl/package.json:35:    "url": "https://github.com/daaku/nodejs-tmpl/issues"
./status-react/node_modules/tmpl/package.json:47:    "url": "git+https://github.com/daaku/nodejs-tmpl.git"
./status-react/node_modules/makeerror/package.json:35:    "url": "https://github.com/daaku/nodejs-makeerror/issues"
./status-react/node_modules/makeerror/package.json:44:  "homepage": "https://github.com/daaku/nodejs-makeerror#readme",
./status-react/node_modules/makeerror/package.json:50:    "url": "git+https://github.com/daaku/nodejs-makeerror.git"

I ran the search over both source trees. That doesn't even make sense to do, does it? Should I just delete it from status-go/vendor?

kardianos is used in a Dockerfile:

./status-go/vendor/github.com/ethereum/go-ethereum/swarm/dev/Dockerfile:39:RUN go get -u github.com/kardianos/govendor

Is this Dockerfile perhaps part of the virtual mobile device local testing facilities? And, go get goes out to some server to download it, so doesn't need the files locally in the source tree, right? Just guessing, really..

I had forgotten about those two, to be honest. I'm glad you pointed them out. Please let me know your disposition on both of these. Thanks.

[feuGeneA]

@divan I've deleted the shell script. And, for posterity, I modified my initial comment on this PR to show the shell command I used to search for unused vendor libraries, in case that might be helpful for someone to refer back to later.

However, please wait a bit to click Merge, until we hear from @adambabik on whether daaku and kardianos should be deleted too.

[divan]

@feuGeneA thanks. And I believe it's safe to remove both, daaku and kardianos repos as well.

[adambabik]

@feuGeneA it looks like daaku is an author of some JS libraries as well used by status-react but status-go does not depend on any. It's safe to remove.

Regarding github.com/kardianos/govendor, you can delete it as well. We don't use that docker image and also it's part of go-ethereum, not status-go. Please delete as well.

[feuGeneA]

Thanks for the feedback guys.

FYI, daaku and kardianos spanned 292 lines and took up 112 K.

status-go/vendor$ cloc github.com/{daaku,kardianos}
      11 text files.
      11 unique files.                              
       4 files ignored.

http://cloc.sourceforge.net v 1.60  T=0.02 s (364.3 files/s, 21027.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Go                               7             52             60            292
-------------------------------------------------------------------------------
SUM:                             7             52             60            292
-------------------------------------------------------------------------------
status-go/vendor$ du -k github.com/{daaku,kardianos} | cut -f 1 - | paste -s -d+ - | bc
112

[feuGeneA]

@divan a mention here as a reminder to merge now that the script has been deleted and daaku and kardianos have been deleted. Thanks!