Fixes #1356: Don't show error message twice

[dpursehouse]

Change-Id: Iff591518853b1c5c229b676d08d8f914f1eb6f62
Signed-off-by: David Pursehouse dpursehouse@collab.net

[dpursehouse]

Trace after applying this change:

test-project $ GIT_CURL_VERBOSE=1 GIT_TRACE=1 git push origin HEAD:refs/for/master
13:38:54.130468 git.c:350               trace: built-in: git 'push' 'origin' 'HEAD:refs/for/master'
13:38:54.131780 run-command.c:336       trace: run_command: 'ssh' '-p' '29418' 'admin@localhost' 'git-receive-pack '\''/test-project'\'''
13:38:54.219326 run-command.c:336       trace: run_command: '.git/hooks/pre-push' 'origin' 'ssh://admin@localhost:29418/test-project'
13:38:54.224146 git.c:563               trace: exec: 'git-lfs' 'pre-push' 'origin' 'ssh://admin@localhost:29418/test-project'
13:38:54.224732 run-command.c:336       trace: run_command: 'git-lfs' 'pre-push' 'origin' 'ssh://admin@localhost:29418/test-project'
trace git-lfs: run_command: git rev-list --objects --stdin --
trace git-lfs: run_command: git cat-file --batch-check
trace git-lfs: run_command: git cat-file --batch
trace git-lfs: run_command: 'git' config -l
trace git-lfs: tq: running as batched queue, batch size of 100
trace git-lfs: tq: sending batch of size 1
trace git-lfs: api: batch 1 files
trace git-lfs: HTTP: POST http://admin@localhost:8080/test-project/info/lfs/objects/batch
> POST /test-project/info/lfs/objects/batch HTTP/1.1
> Host: localhost:8080
> Accept: application/vnd.git-lfs+json; charset=utf-8
> Content-Type: application/vnd.git-lfs+json; charset=utf-8
> User-Agent: git-lfs/1.3.1 (GitHub; darwin amd64; go 1.6.3; git 080a707)
>
{"operation":"upload","objects":[{"oid":"cfa869ea8de71cc47c09170b906c1c7d79c585eb5053a4efd93dcbfe1c826170","size":1273278}]}trace git-lfs: HTTP: 422

< HTTP/1.1 422 Unprocessable Entity
< Content-Length: 139
< Date: Sat, 13 Aug 2016 04:38:54 GMT
<
trace git-lfs: api error: Client error: http://admin@localhost:8080/test-project/info/lfs/objects/batch from HTTP 422
Git LFS: (0 of 1 files) 0 B / 1.21 MB
Client error: http://admin@localhost:8080/test-project/info/lfs/objects/batch from HTTP 422
error: failed to push some refs to 'ssh://admin@localhost:29418/test-project'
test-project $

[dpursehouse]

@technoweenie could you have a look at this? It works, but I'm not entirely sure that it's correct.

[technoweenie]

We'd lose valuable error reporting by merging this. @ttaylorr and I have been talking about moving LFS to https://github.com/pkg/errors soon, which should fix how we bubble low level errors up to end users.

[dpursehouse]

I've rewritten the commit with a different approach. Now it only shows the second message if it's different to the one already shown.

As before, it works for me, but perhaps I'm missing something about why this inner message is needed. How can I reproduce a case where the messages are different?

(Note that I'm testing this against Gerrit Code Review with a work-in-progress implementation of LFS).

moving LFS to https://github.com/pkg/errors soon

Is "soon" soon enough that we can just give up trying to make an interim fix for this?

[technoweenie]

@dpursehouse Ah, this looks better. We plan to either ship v1.4.0 this week (with github.com/pkg/errors), or v1.3.2 with the latest bug fixes. If we're not ready for v1.4.0, I'll include this in v1.3.2.

[ttaylorr]

Let's inline this to be:

if msg := err.Error(); msg != innermsg {
        Error(msg)
}

[dpursehouse]

Do you still want me to make this change in this PR, or will you instead use the version in #1453 ?

[ttaylorr]

I think now that 1453 exists we can leave this as-is 😄

[ttaylorr]

Looks great 👍

[technoweenie]

I'm thinking we should merge this now. If we need v1.3.2, we can backport easily to the release-1.3 branch. If we use github.com/pkg/errors, then the code will change anyway.

Also, this code is very similar to ExitWithError(). The main difference is that ExitWithError() ends the current process, while the inline code in upload() from this PR prints multiple errors.

I added another commit to this PR in #1453.