Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-12-08 06:10:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

cmd/go/internal/modfetch/codehost: refactor gitRepo.loadRefs to be harder to misuse

Browse source 
Previously, callers of loadRefs were expected to always
call via gitRepo.refsOnce.Do and check r.refsErr. This hasn't always
been the case.

This change makes loadRefs cache its own result with r.refsOnce and
return refs and refsErr. Callers can use it more like a normal
function.

CL 297950 is related. Previously, a commit like 0123456789ab could be
resolved to a v0.0.0 pseudo-version when tags couldn't be fetched, but
a shorter commit like 0123456 or a branch name like "master" couldn't
be resolved the same way. With this change, tags must be fetched
successfully ('git ls-remote' must succeed).

For #42751

Change-Id: I49c9346e6c72609ee4f8b10cfe1f69781e78457e
Reviewed-on: https://go-review.googlesource.com/c/go/+/338191
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This commit is contained in:
Jay Conrod 2021-07-28 14:16:47 -07:00
parent ec27168712
commit 8d2066177d
1 changed files with 72 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

73
src/cmd/go/internal/modfetch/codehost/git.go
View file

@ -170,8 +170,9 @@ func (r *gitRepo) loadLocalTags() {
} }
// loadRefs loads heads and tags references from the remote into the map r.refs. // loadRefs loads heads and tags references from the remote into the map r.refs.
// Should only be called as r.refsOnce.Do(r.loadRefs). // The result is cached in memory.
func (r *gitRepo) loadRefs() { func (r *gitRepo) loadRefs() (map[string]string, error) {
r.refsOnce.Do(func() {
// The git protocol sends all known refs and ls-remote filters them on the client side, // The git protocol sends all known refs and ls-remote filters them on the client side,
// so we might as well record both heads and tags in one shot. // so we might as well record both heads and tags in one shot.
// Most of the time we only care about tags but sometimes we care about heads too. // Most of the time we only care about tags but sometimes we care about heads too.
@ -197,32 +198,35 @@ func (r *gitRepo) loadRefs() {
return return
} }
r.refs = make(map[string]string) refs := make(map[string]string)
for _, line := range strings.Split(string(out), "\n") { for _, line := range strings.Split(string(out), "\n") {
f := strings.Fields(line) f := strings.Fields(line)
if len(f) != 2 { if len(f) != 2 {
continue continue
} }
if f[1] == "HEAD" || strings.HasPrefix(f[1], "refs/heads/") || strings.HasPrefix(f[1], "refs/tags/") { if f[1] == "HEAD" || strings.HasPrefix(f[1], "refs/heads/") || strings.HasPrefix(f[1], "refs/tags/") {
r.refs[f[1]] = f[0] refs[f[1]] = f[0]
} }
} }
for ref, hash := range r.refs { for ref, hash := range refs {
if strings.HasSuffix(ref, "^{}") { // record unwrapped annotated tag as value of tag if strings.HasSuffix(ref, "^{}") { // record unwrapped annotated tag as value of tag
r.refs[strings.TrimSuffix(ref, "^{}")] = hash refs[strings.TrimSuffix(ref, "^{}")] = hash
delete(r.refs, ref) delete(refs, ref)
} }
} }
r.refs = refs
})
return r.refs, r.refsErr
} }
func (r *gitRepo) Tags(prefix string) ([]string, error) { func (r *gitRepo) Tags(prefix string) ([]string, error) {
r.refsOnce.Do(r.loadRefs) refs, err := r.loadRefs()
if r.refsErr != nil { if err != nil {
return nil, r.refsErr return nil, err
} }
tags := []string{} tags := []string{}
for ref := range r.refs { for ref := range refs {
if !strings.HasPrefix(ref, "refs/tags/") { if !strings.HasPrefix(ref, "refs/tags/") {
continue continue
} }
@ -237,14 +241,14 @@ func (r *gitRepo) Tags(prefix string) ([]string, error) {
} }
func (r *gitRepo) Latest() (*RevInfo, error) { func (r *gitRepo) Latest() (*RevInfo, error) {
r.refsOnce.Do(r.loadRefs) refs, err := r.loadRefs()
if r.refsErr != nil { if err != nil {
return nil, r.refsErr return nil, err
} }
if r.refs["HEAD"] == "" { if refs["HEAD"] == "" {
return nil, ErrNoCommits return nil, ErrNoCommits
} }
return r.Stat(r.refs["HEAD"]) return r.Stat(refs["HEAD"])
} }
// findRef finds some ref name for the given hash, // findRef finds some ref name for the given hash,
@ -252,8 +256,11 @@ func (r *gitRepo) Latest() (*RevInfo, error) {
// There may be multiple ref names for a given hash, // There may be multiple ref names for a given hash,
// in which case this returns some name - it doesn't matter which. // in which case this returns some name - it doesn't matter which.
func (r *gitRepo) findRef(hash string) (ref string, ok bool) { func (r *gitRepo) findRef(hash string) (ref string, ok bool) {
r.refsOnce.Do(r.loadRefs) refs, err := r.loadRefs()
for ref, h := range r.refs { if err != nil {
return "", false
}
for ref, h := range refs {
if h == hash { if h == hash {
return ref, true return ref, true
} }
@ -295,29 +302,32 @@ func (r *gitRepo) stat(rev string) (*RevInfo, error) {
// Maybe rev is the name of a tag or branch on the remote server. // Maybe rev is the name of a tag or branch on the remote server.
// Or maybe it's the prefix of a hash of a named ref. // Or maybe it's the prefix of a hash of a named ref.
// Try to resolve to both a ref (git name) and full (40-hex-digit) commit hash. // Try to resolve to both a ref (git name) and full (40-hex-digit) commit hash.
r.refsOnce.Do(r.loadRefs) refs, err := r.loadRefs()
if err != nil {
return nil, err
}
// loadRefs may return an error if git fails, for example segfaults, or // loadRefs may return an error if git fails, for example segfaults, or
// could not load a private repo, but defer checking to the else block // could not load a private repo, but defer checking to the else block
// below, in case we already have the rev in question in the local cache. // below, in case we already have the rev in question in the local cache.
var ref, hash string var ref, hash string
if r.refs["refs/tags/"+rev] != "" { if refs["refs/tags/"+rev] != "" {
ref = "refs/tags/" + rev ref = "refs/tags/" + rev
hash = r.refs[ref] hash = refs[ref]
// Keep rev as is: tags are assumed not to change meaning. // Keep rev as is: tags are assumed not to change meaning.
} else if r.refs["refs/heads/"+rev] != "" { } else if refs["refs/heads/"+rev] != "" {
ref = "refs/heads/" + rev ref = "refs/heads/" + rev
hash = r.refs[ref] hash = refs[ref]
rev = hash // Replace rev, because meaning of refs/heads/foo can change. rev = hash // Replace rev, because meaning of refs/heads/foo can change.
} else if rev == "HEAD" && r.refs["HEAD"] != "" { } else if rev == "HEAD" && refs["HEAD"] != "" {
ref = "HEAD" ref = "HEAD"
hash = r.refs[ref] hash = refs[ref]
rev = hash // Replace rev, because meaning of HEAD can change. rev = hash // Replace rev, because meaning of HEAD can change.
} else if len(rev) >= minHashDigits && len(rev) <= 40 && AllHex(rev) { } else if len(rev) >= minHashDigits && len(rev) <= 40 && AllHex(rev) {
// At the least, we have a hash prefix we can look up after the fetch below. // At the least, we have a hash prefix we can look up after the fetch below.
// Maybe we can map it to a full hash using the known refs. // Maybe we can map it to a full hash using the known refs.
prefix := rev prefix := rev
// Check whether rev is prefix of known ref hash. // Check whether rev is prefix of known ref hash.
for k, h := range r.refs { for k, h := range refs {
if strings.HasPrefix(h, prefix) { if strings.HasPrefix(h, prefix) {
if hash != "" && hash != h { if hash != "" && hash != h {
// Hash is an ambiguous hash prefix. // Hash is an ambiguous hash prefix.
@ -335,9 +345,6 @@ func (r *gitRepo) stat(rev string) (*RevInfo, error) {
hash = rev hash = rev
} }
} else { } else {
if r.refsErr != nil {
return nil, r.refsErr
}
return nil, &UnknownRevisionError{Rev: rev} return nil, &UnknownRevisionError{Rev: rev}
} }
@ -535,12 +542,12 @@ func (r *gitRepo) ReadFileRevs(revs []string, file string, maxSize int64) (map[s
// Build list of known remote refs that might help. // Build list of known remote refs that might help.
var redo []string var redo []string
r.refsOnce.Do(r.loadRefs) refs, err := r.loadRefs()
if r.refsErr != nil { if err != nil {
return nil, r.refsErr return nil, err
} }
for _, tag := range need { for _, tag := range need {
if r.refs["refs/tags/"+tag] != "" { if refs["refs/tags/"+tag] != "" {
redo = append(redo, tag) redo = append(redo, tag)
} }
} }