Windows fixes

[pauljz]

First round of Windows fixes. So far this is mostly simple fixes like dealing with path separators and path list separators properly.

There's some iffy stuff in the toolchain command code (which I also couldn't get working just yet due to symlink issues), but most of the rest of it should be pretty safe. I'll call that out in the comments on those lines.

The tests all pass locally for me on *nix, but that doesn't mean it's all happily working. Warrants some close scrutiny just due to how many things this touches.

Part of #149

[dmitshur]

Overall, looks good. I've left 3 comments above.

[pauljz]

Fixed those up. Does the error errors.New("SRCLIBPATH not set") look okay? Should be clearer than blowing up trying to access an element of an empty slice. :)

Went ahead and squashed this down to a single commit.

[dmitshur]

Hey @pauljz,

I've thoroughly reviewed the latest version of this PR (commit f7e36abfcdeb8b0a5e55396bb65b06120e0b8417), and overall it looks good. I've left 3 comments above. Two of them are easy to address. Please avoid squashing for now (so it's easier for me to review the upcoming changes).

The third comment, about the srclib.Path == "" guard is trickier.

-strings.SplitN(srclib.Path, ":", 2)[0]
+filepath.SplitList(srclib.Path)[0]

Looking through the code, there are some instances of the above code that do that check to avoid a possible panic, and there are still 5-8 other places that do not.

I think we should be consistent about that, but I'm not sure if adding 5-8 more guards is the way to go. Maybe it is. Do you have thoughts on this?

Thanks for your patience as we work out the remaining issues!

[dmitshur]

Looking through the code, there are some instances of the above code that do that check to avoid a possible panic, and there are still 5-8 other places that do not.

I think we should be consistent about that, but I'm not sure if adding 5-8 more guards is the way to go. Maybe it is. Do you have thoughts on this?

I'm leaning in the direction of just documenting that srclib.Path should not be empty as a precondition, and not add any guards. If srclib.Path is ever empty, even with current code, unexpected things will happen (but no panic). I think it should never be empty (there is code ensuring that), and if it happens to be, then a panic is an acceptable outcome, since something must be seriously wrong for that to ever happen.

[pauljz]

Yeah, given the checks that already happen in env.go it seems awfully redundant to have guards scattered throughout the code. I took out the other guards in this latest commit.

The other consistency things are all updated too.

[dmitshur]

Thanks, that looks great!

LGTM. This is good to merge now. There shouldn't be any behavior changes on OS X and Linux, but Windows (and any other OSes supported by Go) behavior should be better.

[pauljz]

Okay, squashed that last commit too.

[dmitshur]

Thank you @pauljz!