vcs/gitcmd: need a way to UpdateEverything and propogate error/stderr

[devastating]

Hello,

We use http://gitolite.com/gitolite/ as our Git host, and an issue we have is that if a "git push" encounters error with one of the mirroring server (e.g. fail to push to mirror A, but succeed to mirror B), the following "git fetch" command will put a warning message in stderr, even if the "git fetch" works correctly.

This breaks the usage of "gitcmd.UpdateEverything" as it tries to parse the stderr. Could I work with your team to come up with a fix so that we could optionally skip the parsing stderr part?

I've come up with a diff devastating@c42b5bc to attempt to address the issue. It should not break anything, but does create a new public API. Do you have any suggestions? Thank you so much.

Nick

[dmitshur]

the following "git fetch" command will put a warning message in stderr, even if the "git fetch" works correctly.

What kind of warning message is it? Is it maybe possible to detect it in parseRemoteUpdate and skip over it?

Another option, albeit not very clean, is to add a new field to RemoteOpts to disable parsing result of updating everything. It's not very clean because RemoteOpts are embedded in CloneOpt, which is used by Clone, so it doesn't really belong there.

[devastating]

Unfortunately, I think the error message is gitolite-only so I don't think we should parse it. You can see the similar issue in this thread: https://groups.google.com/forum/#!topic/gitolite/Yhr-HGgaqu4

We've also considered adding additional ops to RemoteOpts like you suggested, and like you said, it did not seem to fit there.

As a result, in the commit I made, I've added a new API, since we are going to add public method/member anyway. Any other thoughts are welcome.

[devastating]

@shurcooL any other suggestions that I could make a change?

[dmitshur]

We've also considered adding additional ops to RemoteOpts like you suggested, and like you said, it did not seem to fit there.

As a result, in the commit I made, I've added a new API, since we are going to add public method/member anyway. Any other thoughts are welcome.

A new method is going to affect the users of this package too, so if you're changing the API, it probably makes sense to do what leads to the best API in the end, rather than trying to make a minimal change.

You can look into the discussion at #79 for further information or insight.

I don't have further suggestions at this time.

That said, I'm not sure who has ownership (i.e., can make decisions about breaking API changes) over this package now.

[devastating]

@shurcooL Thank you for your response..seems like this was an existing issue for a while (whether or not parsing the output).