Better handling on platforms where os/user.Current() is not implemented

[chungers]

Closes #287

Code should not panic when the package os/user's Current() function isn't implemented. Instead try to get the environment variable HOME.

Changed the 'constructor' function for the directory discovery to accept a directory parameter. This better articulates the dependency (the name of the directory) and allows future tests to better control the directory to scan for the discovery code under testing.

Changed the mains to use the Dir() function which computes the default directory, as the input.

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "287-os-user" [email protected]:chungers/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842353372976
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[codecov-io]

Current coverage is 69.45% (diff: 22.22%)

Merging #294 into master will decrease coverage by 0.13%

@@             master       #294   diff @@
==========================================
  Files            25         25          
  Lines          1302       1303     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            906        905     -1   
- Misses          319        321     +2   
  Partials         77         77          

Powered by Codecov. Last update 8fa8129...ec923eb

[wfarner]

I find the redundancy of discovery.NewPluginDiscovery(discovery.Dir()) unfortunate. What's the motivation behind requiring the arg if we're going to use the same value everywhere?

[FrenchBen]

Previously the constructor grabbed the directory from the env os.Getenv(PluginDirEnvVar) when it needed to be different.

I'm not sure I follow what we gain by explicitly passing it to the constructor?

os.Setenv("INFRAKIT_PLUGINS_DIR","/my/test/dir")
plugins, err := discovery.NewPluginDiscovery()

vs

plugins, err := discovery.NewPluginDiscovery("/my/test/dir")

[chungers]

By having the parameter in the function we are

• being explicit about the requirements to construct a discovery object (the directory to scan).
• following a pattern similar to IoC (inversion of control) or dependency injection -- meaning it's up to the caller to configure the object at construction time (NewPluginDiscovery is like a constructor in that it creates and returns a new instance).
• some of the tests I am working on need to set this directory explicitly and the code is easier to follow with the second form, especially when the tests and os process boundaries aren't clear and tests may be executed concurrently.

Having said that, I am not crazy about all the changes in the main.go files. Let me do this:

1. Add a new function that allows setting of the directory

2. Change the implementation of NewPluginDiscovery() to call that new function with theDir()` function which returns a default location

3. Revert the main.go files back to using the 'default constructor' form so that we have a simple, uniform default.

   @wfarner

[wfarner]

Yes, supporting but not requiring the arg sounds like the right thing to do.

[FrenchBen]

LGTM with one comment.

Also it'd be cleaner if the commits were squashed