Fix fetch tags

[zacharya19]

Quick fix for #371.
Not sure it's the best way, but this bug is very disturbing to me, so I fixed it quickly.

In tags we do not need to check hash, we need to check if it's name exists in the local storer, so I saved all local tags in slice and then if the reference is a tag, I say it's exists if it's in the slice.

Better idea?

[smola]

Prefer var tags []plumbing.Hash initialization.

[smola]

Check return error.

[smola]

@zacharya19 Thanks for the contribution!

I wonder how this works. If I understood correctly, this implies that the repository contains a tag returned by IterReferences() that points to a hash which does not exist in the localStorer.

That should not happen at all. Could you give more details about a repository exhibiting this error? Did you clone it with go-git from the beginning? Or did you clone with git and then pulled with go-git? I suspect that if this fix works, it's only because there is a related bug that has lead the repository into an inconsistent state.

[codecov]

Codecov Report

Merging #407 into master will decrease coverage by 0.51%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #407      +/-   ##
==========================================
- Coverage   78.03%   77.51%   -0.52%     
==========================================
  Files         127      127              
  Lines        9186     9310     +124     
==========================================
+ Hits         7168     7217      +49     
- Misses       1237     1313      +76     
+ Partials      781      780       -1

Impacted Files	Coverage Δ
remote.go	72.25% <87.5%> (+0.6%)	⬆️
plumbing/transport/ssh/common.go	2.81% <0%> (-48.1%)	⬇️
plumbing/transport/ssh/auth_method.go	33.33% <0%> (-24.77%)	⬇️
plumbing/transport/http/upload_pack.go	61.81% <0%> (-12.15%)	⬇️
plumbing/protocol/packp/srvresp.go	81.13% <0%> (-6.75%)	⬇️
plumbing/format/idxfile/decoder.go	62.31% <0%> (ø)	⬆️
plumbing/protocol/packp/updreq.go	80% <0%> (ø)	⬆️
repository.go	73.26% <0%> (+0.09%)	⬆️
plumbing/protocol/packp/uppackresp.go	89.18% <0%> (+0.3%)	⬆️
... and 5 more

---

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 2d02297...ed1382f. Read the comment docs.

[zacharya19]

@smola I use PlainOpen to open a repo I cloned with git cli.
But there is something strange here, with my fix I get new tags with the same info but other name (I have tag named "release-1.23.1" and after I run the code, my local git has "release-1.23.1" and "1.23.1" with the same hash and message).
I'm not really sure why.

[zacharya19]

BTW - the tag could not by exists locally if the hash does not exists locally, am I wrong?

[smola]

@zacharya19 This code implies that tags have hashes of objects that are not in the object storage. A tag is either a tag object (in the object storage) or a reference to a non-tag object (in the object storage too). So if there are cases where localStorer.IterReferences() returns tags pointing to hashes that are not in the object storage (objectExists(localStorer, hash) == false) it might indicate a bug previos in the execution.

if !ref.IsTag() {
	exists, err = objectExists(localStorer, hash)
	if err != nil {
		return err
	}
} else {
	exists = hashInList(hash, tags)
}

[zacharya19]

Not sure I got it, since the problem here is the opposite, on the remote we see tags that point to an hash that exists locally, but the tag it self does not exist.

[smola]

@zacharya19 Ok. This can happen with lightweight tags. Although, this still wouldn't fix the case where a tag T1already exist locally pointing to hash H1 and the server contains a new hash T2 pointing to H1 too.

[zacharya19]

@smola I made some changes, let me know what you think.

[mcuadros]

@smola

[smola]

Thank you @zacharya19, looks good!

[smola]

We should add a test for this though.

[smola]

I'm working on a proper test before moving on.

[zacharya19]

@smola I added basic test, let me know what you think.

[mcuadros]

@zacharya19 thanks for your efforts and your time, but I created another PR #485 with a canonical solution.