Fix non-functionality on Windows.

[jaccarmac]

Because of the .exe extension of Go-produced binaries on Windows, binstale could not tell that those binaries came from similarly-named Go packages (The go command used to get the names of installed packages returns a directory name, which obviously will not contain .exe).

To fix this, we normalize the names of packages as they come in via arguments and file system queries. This normalization occurs before the printing of executable names, but can be moved to after if users would prefer to see the file extension (I prefer not to, as there is a clearer mapping between binary and package this way.).

Arguments to binstale can now be in the form "binary" or "binary.exe"; Both formats end up producing identical output. This detail creates some strange but hopefully meaningless edge cases, however. "binstale .exe" will display a message telling the user that binary "" cannot be found. If two packages exist on the system, one named "github.com/example/binstale" and one named "github.com/example/binstale.exe", both "binstale binstale" and "binstale binstale.exe" will check the former binary's staleness, while only "binstale binstale.exe.exe" will check the latter's.

[jaccarmac]

Due to my Docker being very badly behaved at the moment, I haven't had a chance to explicitly test on Linux/OSX. Removing the file extensions from some binaries in $GOPATH/bin worked fine and the Travis tests ran, so I foresee no problems. Something to keep in mind before merging, though :-).

[dmitshur]

Hi @jaccarmac,

Thank you for reporting this issue and providing a PR that fixes it. I'd really like to resolve this issue, and I want to do it well. I have some ideas for how to do better and avoid the edge cases, if possible.

Are you interested in some review comments and working on making changes, or would you rather I took the PR from here?

[jaccarmac]

I'm absolutely interested in feedback and fixing the implementation!

I toyed with a few ways of doing this to various degrees of success; This one is what made the most sense at the time/required the least code for a working solution.

[dmitshur]

Great!

First, a high level thought I'd like to run by you.

I've noticed I made the mistake a few times of thinking binstale expects an import path, like pretty much all my other Go-related tools do. It made me wonder whether that'd actually be better if you could specify the binary name by providing its import path.

How useful is it to be able to specify just the binary name instead?

[jaccarmac]

I find binstale useful exactly because of the binary -> path mapping. There are already x tools to manage versioning in the other direction (path -> binary). So, to answer your question, I'd say very. I don't see a use case if the user has to know which package a binary came from beforehand.

[dmitshur]

Fair enough, and I agree.

I may continue to think about adding a way to for it to also accept import paths (rather than failing), but taking away just binary name arguments isn't going to fly. But that's separate.

Ok, so the current implementation is simple and precise, there's no ambiguity or guessing. I'd really like to preserve that property.

For Windows, we can detect it via runtime.GOOS value being windows. I don't think OS X and Linux should have any special behavior for ".exe" suffix, as unlikely as that might be.

The main remaining question in my mind is if, on Windows, we should require the user to type the exact binary name with ".exe" suffix, or require no ".exe" suffix, or accept both.

Ideally, I'd prefer... a simple strict rule (like always requiring the full binary name including ".exe"), rather than accepting both. But what do you think? I can be convinced that accepting both with and without ".exe" on Windows is okay.

[jaccarmac]

I prefer Unix-style no-extension, as seen in the output produced by my patch. So that's my preference for input as well, though it's nice to be able to do both. That said, I can't think of any way to resolve the binary/binary.exe/binary.exe.exe ambiguity if both syntaxes are allowed.

[jaccarmac]

The EXE file extension shows up nowhere except the filesystem anyway. Go's level of granularity is package-level, not filesystem-level, and I think it's good to be consistent to that in our tools.

[dmitshur]

I prefer Unix-style no-extension, as seen in the output produced by my patch. So that's my preference for input as well

Given that you're the first Windows user to even report this issue, and you went as far as creating a PR that fixes it, your opinion counts for more.

I'm not strongly opposed to either one or the other, but I do prefer picking a single format and going with it (at least to start with).

So I'm happy to go with Unix-style no-extension. Let's do that for now, and consider supporting both as a followup (I will likely resist it, unless there are very convincing arguments).

The EXE file extension shows up nowhere except the filesystem anyway. Go's level of granularity is package-level, not filesystem-level, and I think it's good to be consistent to that in our tools.

Agreed.

Another benefit with going Unix-style no-extension is that if you switch between a Linux and Windows computer, you'd type the exact same command in a terminal, e.g., binstale binstale. No need to remember to add the ".exe" suffix on Windows-only.

So, let's do the following:

1. If runtime.GOOS value equals to "windows", expect the actual binary on disk to have a ".exe" suffix relative to the inputs and the command names.
2. Add to commit message that this change "fixes #4".

Does that sound okay by you?

[jaccarmac]

See if that does the trick. If so, I'll squash down so the history stays clean. To actually ensure we search for a ".exe" suffix I needed to add some more plumbing.

[dmitshur]

@jaccarmac, thank you again for your contribution.

I'll leave review comments on this PR, and then implement them myself in order to resolve this issue tonight (since another person also reported it, I don't want to delay the fix). I'll squash your commits into one, and follow it up with my style changes.

[jaccarmac]

Thanks for the input! Should have thought to do a lint pass before pushing.

[dmitshur]

No problem. Please don't actually make those changes because I've already done that, just improving the documentation a bit before I push. :)