[release-branch.go1.19] cmd/go: avoid overwriting cached Origin metadata

Fixes #54633.
Updates #54631.

Change-Id: I17d2fa282642aeb1ae2a6e29a0756b8960bea34b
Reviewed-on: https://go-review.googlesource.com/c/go/+/425255
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit e3004d96b6899945d76e5dd373756324c8ff98fb)
Reviewed-on: https://go-review.googlesource.com/c/go/+/425211
This commit is contained in:
Bryan C. Mills 2022-08-23 18:00:34 -04:00 committed by Heschi Kreinick
parent d39fd4f2b7
commit d2bcb22ce0
4 changed files with 43 additions and 17 deletions

View File

@ -37,7 +37,9 @@ const (
// A Repo represents a code hosting source. // A Repo represents a code hosting source.
// Typical implementations include local version control repositories, // Typical implementations include local version control repositories,
// remote version control servers, and code hosting sites. // remote version control servers, and code hosting sites.
// A Repo must be safe for simultaneous use by multiple goroutines. //
// A Repo must be safe for simultaneous use by multiple goroutines,
// and callers must not modify returned values, which may be cached and shared.
type Repo interface { type Repo interface {
// CheckReuse checks whether the old origin information // CheckReuse checks whether the old origin information
// remains up to date. If so, whatever cached object it was // remains up to date. If so, whatever cached object it was

View File

@ -348,12 +348,21 @@ func (r *gitRepo) Latest() (*RevInfo, error) {
if refs["HEAD"] == "" { if refs["HEAD"] == "" {
return nil, ErrNoCommits return nil, ErrNoCommits
} }
info, err := r.Stat(refs["HEAD"]) statInfo, err := r.Stat(refs["HEAD"])
if err != nil { if err != nil {
return nil, err return nil, err
} }
// Stat may return cached info, so make a copy to modify here.
info := new(RevInfo)
*info = *statInfo
info.Origin = new(Origin)
if statInfo.Origin != nil {
*info.Origin = *statInfo.Origin
}
info.Origin.Ref = "HEAD" info.Origin.Ref = "HEAD"
info.Origin.Hash = refs["HEAD"] info.Origin.Hash = refs["HEAD"]
return info, nil return info, nil
} }
@ -560,7 +569,7 @@ func (r *gitRepo) fetchRefsLocked() error {
return nil return nil
} }
// statLocal returns a RevInfo describing rev in the local git repository. // statLocal returns a new RevInfo describing rev in the local git repository.
// It uses version as info.Version. // It uses version as info.Version.
func (r *gitRepo) statLocal(version, rev string) (*RevInfo, error) { func (r *gitRepo) statLocal(version, rev string) (*RevInfo, error) {
out, err := Run(r.dir, "git", "-c", "log.showsignature=false", "log", "--no-decorate", "-n1", "--format=format:%H %ct %D", rev, "--") out, err := Run(r.dir, "git", "-c", "log.showsignature=false", "log", "--no-decorate", "-n1", "--format=format:%H %ct %D", rev, "--")

View File

@ -182,23 +182,27 @@ func mergeOrigin(m1, m2 *codehost.Origin) *codehost.Origin {
if !m2.Checkable() { if !m2.Checkable() {
return m2 return m2
} }
merged := new(codehost.Origin)
*merged = *m1 // Clone to avoid overwriting fields in cached results.
if m2.TagSum != "" { if m2.TagSum != "" {
if m1.TagSum != "" && (m1.TagSum != m2.TagSum || m1.TagPrefix != m2.TagPrefix) { if m1.TagSum != "" && (m1.TagSum != m2.TagSum || m1.TagPrefix != m2.TagPrefix) {
m1.ClearCheckable() merged.ClearCheckable()
return m1 return merged
} }
m1.TagSum = m2.TagSum merged.TagSum = m2.TagSum
m1.TagPrefix = m2.TagPrefix merged.TagPrefix = m2.TagPrefix
} }
if m2.Hash != "" { if m2.Hash != "" {
if m1.Hash != "" && (m1.Hash != m2.Hash || m1.Ref != m2.Ref) { if m1.Hash != "" && (m1.Hash != m2.Hash || m1.Ref != m2.Ref) {
m1.ClearCheckable() merged.ClearCheckable()
return m1 return merged
} }
m1.Hash = m2.Hash merged.Hash = m2.Hash
m1.Ref = m2.Ref merged.Ref = m2.Ref
} }
return m1 return merged
} }
// addVersions fills in m.Versions with the list of known versions. // addVersions fills in m.Versions with the list of known versions.

View File

@ -220,6 +220,17 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
return revErr, err return revErr, err
} }
mergeRevOrigin := func(rev *modfetch.RevInfo, origin *codehost.Origin) *modfetch.RevInfo {
merged := mergeOrigin(rev.Origin, origin)
if merged == rev.Origin {
return rev
}
clone := new(modfetch.RevInfo)
*clone = *rev
clone.Origin = merged
return clone
}
lookup := func(v string) (*modfetch.RevInfo, error) { lookup := func(v string) (*modfetch.RevInfo, error) {
rev, err := repo.Stat(v) rev, err := repo.Stat(v)
// Stat can return a non-nil rev and a non-nil err, // Stat can return a non-nil rev and a non-nil err,
@ -227,7 +238,7 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
if rev == nil && err != nil { if rev == nil && err != nil {
return revErr, err return revErr, err
} }
rev.Origin = mergeOrigin(rev.Origin, versions.Origin) rev = mergeRevOrigin(rev, versions.Origin)
if err != nil { if err != nil {
return rev, err return rev, err
} }
@ -256,12 +267,12 @@ func queryProxy(ctx context.Context, proxy, path, query, current string, allowed
if err := allowed(ctx, module.Version{Path: path, Version: current}); errors.Is(err, ErrDisallowed) { if err := allowed(ctx, module.Version{Path: path, Version: current}); errors.Is(err, ErrDisallowed) {
return revErr, err return revErr, err
} }
info, err := repo.Stat(current) rev, err = repo.Stat(current)
if info == nil && err != nil { if rev == nil && err != nil {
return revErr, err return revErr, err
} }
info.Origin = mergeOrigin(info.Origin, versions.Origin) rev = mergeRevOrigin(rev, versions.Origin)
return info, err return rev, err
} }
} }