Update local remote references during fetch even if no pack needs to be received

[orirawlings]

I recently ran into some behavior that was not consistent with the standard git tooling.

It seems that doing a (*Remote).Fetch() will not update the local remote refs (i.e. refs/remotes/origin/*) if we already have all the new commit objects locally. The standard git tooling will continue to update the local remote refs even if it doesn't need to fetch a pack file with new commit objects.

You can see an example below where we are in a repository that already has both 7a58be and 2d8e82.

$ git show-ref
7a58be863cf6c431619df28b709b3bc87f2a2e29 refs/heads/master
2d8e82007fa1df7f9e203d56543f95388cac9449 refs/remotes/origin/HEAD
2d8e82007fa1df7f9e203d56543f95388cac9449 refs/remotes/origin/master

But the current remote references are out of date:

$ git ls-remote origin
7a58be863cf6c431619df28b709b3bc87f2a2e29  HEAD
7a58be863cf6c431619df28b709b3bc87f2a2e29  refs/heads/master

The fetch will not pull down any new commit objects, but will proceed with updating the remote refs in the local repository.

$ git fetch origin
From github.com:orirawlings/test
   2d8e820..7a58be8  master     -> origin/master
$ git show-ref
7a58be863cf6c431619df28b709b3bc87f2a2e29 refs/heads/master
7a58be863cf6c431619df28b709b3bc87f2a2e29 refs/remotes/origin/HEAD
7a58be863cf6c431619df28b709b3bc87f2a2e29 refs/remotes/origin/master

I'm not really sure from an API design side if we want to continue to return the NoErrAlreadyUpToDate error, but the current PR changes preserve this in the response.

[mcuadros]

Thanks for the contribution, looks Ok. It only need an small change.

The NoErrAlreadyUpToDate should be returned only when nothing changed, in this case something changes so return nil.

Can you add this to the documentation?

[codecov]

Codecov Report

Merging #434 into master will decrease coverage by 0.57%.
The diff coverage is 70.58%.

@@            Coverage Diff             @@
##           master     #434      +/-   ##
==========================================
- Coverage   78.01%   77.44%   -0.58%     
==========================================
  Files         127      127              
  Lines        9171     9186      +15     
==========================================
- Hits         7155     7114      -41     
- Misses       1236     1306      +70     
+ Partials      780      766      -14

Impacted Files	Coverage Δ
remote.go	71.65% <70.58%> (+0.73%)	⬆️
plumbing/transport/ssh/common.go	0% <0%> (-50.91%)	⬇️
plumbing/transport/ssh/auth_method.go	33.33% <0%> (-24.77%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65bf694...78c539a. Read the comment docs.

[orirawlings]

I've updated this PR so that NoErrAlreadyUpToDate is only returned if no pack is fetched and none of the local references need to be created/updated.

I'm not really sure why the code coverage report dropped for some of the plumbing/transport/ssh files. I wouldn't expect that to be related to this change. False positive?

Also, not really sure what is going on with the AppVeyor build either. Looks like some latent issues with windows support.

[taralx]

Heh, funny. I reported this bug as #425.

[orirawlings]

Hmm. I must have overlooked the issue when I opened the PR. Thanks for pointing that out @taralx!

[mcuadros]

Yep, the SSH tests only runs under the master branch since the repository contains an encrypted key.
AppVeyor is there to remember us that Windows support should be fixed.