search

sourcegraph / go-vcs

manipulate and inspect VCS repositories in Go
https://sourcegraph.com/sourcegraph/go-vcs
Other
79 stars 20 forks source link

vcs/gitcmd: unset GIT_ASKPASS prior to appension #87

Closed slimsag closed 8 years ago

slimsag commented 8 years ago

or else the new one will not take effect if there is a prior one in play.

This was discovered to be incorrect as part of a separate / unrelated change, but nonetheless was incorrect code.

keegancsmith commented 8 years ago

LGTM

dmitshur commented 8 years ago

The same change should be done for the cmd.Env = []string{"GIT_SSH=" + gitSSHWrapper} line in the block above.

Edit: Never mind, I just realized it's clearing all previous environment variables, if any, since it's directly setting the Env variable rather than appending to it. But I'm unsure if that makes sense in the general case; it also seems inconsistent.

I'd also suggest just implementing a Set method instead of doing Unset and then a manual append. You can copy a tested implementation from here.