-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose deprecation data in Bundle #59
Changes from all commits
6d6b2e4
bb038d6
ef8c8ac
3fa6575
e1d3105
3914531
ea8f696
be5d907
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,12 +73,18 @@ type Builder struct { | |
// selected version of each module registry package. | ||
resolvedRegistry map[registryPackageVersion]sourceaddrs.RemoteSource | ||
|
||
// packageVersionDeprecations tracks potential deprecations for | ||
// each package version. If a package version is not deprecated, its mapped value will be nil. | ||
// This data, including both package versions and their potential deprecations, is gathered from the registry client and cached. It is included in the bundle, | ||
// where it can be used to warn about deprecated package versions. | ||
packageVersionDeprecations map[registryPackageVersion]*RegistryVersionDeprecation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious why this is a map of pointer-to- Having a pointer here means we need to be diligent about nil checks, or we'll panic. Would there be a disadvantage to this being There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comes from the approach we've taken with how we denote deprecated versions. In essence, when there are no deprecations, the registry api doesn't include it in the Module Version response. So in this way |
||
|
||
// registryPackageVersions caches responses from module registry calls to | ||
// look up the available versions for a particular module package. Although | ||
// these could potentially change while we're running, we assume that the | ||
// lifetime of a particular Builder is short enough for that not to | ||
// matter. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth noting here that this cache includes the deprecation data, as well as the package versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Included 👍 be5d907 |
||
registryPackageVersions map[regaddr.ModulePackage]versions.List | ||
registryPackageVersions map[regaddr.ModulePackage][]ModulePackageInfo | ||
|
||
mu sync.Mutex | ||
} | ||
|
@@ -100,14 +106,15 @@ func NewBuilder(targetDir string, fetcher PackageFetcher, registryClient Registr | |
return nil, fmt.Errorf("invalid target directory: %w", err) | ||
} | ||
return &Builder{ | ||
targetDir: absDir, | ||
fetcher: fetcher, | ||
registryClient: registryClient, | ||
analyzed: make(map[remoteArtifact]struct{}), | ||
remotePackageDirs: make(map[sourceaddrs.RemotePackage]string), | ||
remotePackageMeta: make(map[sourceaddrs.RemotePackage]*PackageMeta), | ||
resolvedRegistry: make(map[registryPackageVersion]sourceaddrs.RemoteSource), | ||
registryPackageVersions: make(map[regaddr.ModulePackage]versions.List), | ||
targetDir: absDir, | ||
fetcher: fetcher, | ||
registryClient: registryClient, | ||
analyzed: make(map[remoteArtifact]struct{}), | ||
remotePackageDirs: make(map[sourceaddrs.RemotePackage]string), | ||
remotePackageMeta: make(map[sourceaddrs.RemotePackage]*PackageMeta), | ||
resolvedRegistry: make(map[registryPackageVersion]sourceaddrs.RemoteSource), | ||
packageVersionDeprecations: make(map[registryPackageVersion]*RegistryVersionDeprecation), | ||
registryPackageVersions: make(map[regaddr.ModulePackage][]ModulePackageInfo), | ||
}, nil | ||
} | ||
|
||
|
@@ -343,7 +350,8 @@ func (b *Builder) findRegistryPackageSource(ctx context.Context, sourceAddr sour | |
trace := buildTraceFromContext(ctx) | ||
|
||
pkgAddr := sourceAddr.Package() | ||
availableVersions, ok := b.registryPackageVersions[pkgAddr] | ||
availablePackageInfos, ok := b.registryPackageVersions[pkgAddr] | ||
var availableVersions versions.List | ||
if !ok { | ||
var reqCtx context.Context | ||
if cb := trace.RegistryPackageVersionsStart; cb != nil { | ||
|
@@ -360,14 +368,15 @@ func (b *Builder) findRegistryPackageSource(ctx context.Context, sourceAddr sour | |
} | ||
return sourceaddrs.RemoteSource{}, fmt.Errorf("failed to query available versions for %s: %w", pkgAddr, err) | ||
} | ||
vs := resp.Versions | ||
vs.Sort() | ||
availableVersions = vs | ||
b.registryPackageVersions[pkgAddr] = availableVersions | ||
|
||
availablePackageInfos = resp.Versions | ||
b.registryPackageVersions[pkgAddr] = resp.Versions | ||
availableVersions = extractVersionListFromResponse(availablePackageInfos) | ||
if cb := trace.RegistryPackageVersionsSuccess; cb != nil { | ||
cb(reqCtx, pkgAddr, availableVersions) | ||
} | ||
} else { | ||
availableVersions = extractVersionListFromResponse(availablePackageInfos) | ||
if cb := trace.RegistryPackageVersionsAlready; cb != nil { | ||
cb(ctx, pkgAddr, availableVersions) | ||
} | ||
|
@@ -401,6 +410,25 @@ func (b *Builder) findRegistryPackageSource(ctx context.Context, sourceAddr sour | |
} | ||
realSourceAddr = resp.SourceAddr | ||
b.resolvedRegistry[pkgVer] = realSourceAddr | ||
|
||
var versionDeprecation *ModulePackageVersionDeprecation | ||
for _, v := range availablePackageInfos { | ||
if selectedVersion.Same(v.Version) { | ||
versionDeprecation = v.Deprecation | ||
break | ||
} | ||
|
||
} | ||
var deprecation *RegistryVersionDeprecation | ||
if versionDeprecation != nil { | ||
deprecation = &RegistryVersionDeprecation{ | ||
Version: selectedVersion.String(), | ||
Reason: versionDeprecation.Reason, | ||
Link: versionDeprecation.Link, | ||
} | ||
} | ||
b.packageVersionDeprecations[pkgVer] = deprecation | ||
|
||
if cb := trace.RegistryPackageSourceSuccess; cb != nil { | ||
cb(reqCtx, pkgAddr, selectedVersion, realSourceAddr) | ||
} | ||
|
@@ -576,7 +604,7 @@ func (b *Builder) writeManifest(filename string) error { | |
}) | ||
|
||
registryObjs := make(map[regaddr.ModulePackage]*manifestRegistryMeta) | ||
for rpv, sourceAddr := range b.resolvedRegistry { | ||
for rpv, sourceInfo := range b.resolvedRegistry { | ||
manifestMeta, ok := registryObjs[rpv.pkg] | ||
if !ok { | ||
root.RegistryMeta = append(root.RegistryMeta, manifestRegistryMeta{ | ||
|
@@ -586,10 +614,13 @@ func (b *Builder) writeManifest(filename string) error { | |
manifestMeta = &root.RegistryMeta[len(root.RegistryMeta)-1] | ||
registryObjs[rpv.pkg] = manifestMeta | ||
} | ||
deprecation := b.packageVersionDeprecations[rpv] | ||
manifestMeta.Versions[rpv.version.String()] = manifestRegistryVersion{ | ||
SourceAddr: sourceAddr.String(), | ||
SourceAddr: sourceInfo.String(), | ||
Deprecation: deprecation, | ||
} | ||
} | ||
|
||
sort.Slice(root.RegistryMeta, func(i, j int) bool { | ||
return root.Packages[i].SourceAddr < root.Packages[j].SourceAddr | ||
}) | ||
|
@@ -713,3 +744,12 @@ func packagePrepareWalkFn(root string, ignoreRules *ignorefiles.Ruleset) filepat | |
return nil | ||
} | ||
} | ||
|
||
func extractVersionListFromResponse(modPackageInfos []ModulePackageInfo) versions.List { | ||
vs := make(versions.List, len(modPackageInfos)) | ||
for index, v := range modPackageInfos { | ||
vs[index] = v.Version | ||
} | ||
vs.Sort() | ||
return vs | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,8 @@ type Bundle struct { | |
remotePackageDirs map[sourceaddrs.RemotePackage]string | ||
remotePackageMeta map[sourceaddrs.RemotePackage]*PackageMeta | ||
|
||
registryPackageSources map[regaddr.ModulePackage]map[versions.Version]sourceaddrs.RemoteSource | ||
registryPackageSources map[regaddr.ModulePackage]map[versions.Version]sourceaddrs.RemoteSource | ||
registryPackageVersionDeprecations map[regaddr.ModulePackage]map[versions.Version]*RegistryVersionDeprecation | ||
} | ||
|
||
// OpenDir opens a bundle rooted at the given base directory. | ||
|
@@ -51,10 +52,11 @@ func OpenDir(baseDir string) (*Bundle, error) { | |
} | ||
|
||
ret := &Bundle{ | ||
rootDir: rootDir, | ||
remotePackageDirs: make(map[sourceaddrs.RemotePackage]string), | ||
remotePackageMeta: make(map[sourceaddrs.RemotePackage]*PackageMeta), | ||
registryPackageSources: make(map[regaddr.ModulePackage]map[versions.Version]sourceaddrs.RemoteSource), | ||
rootDir: rootDir, | ||
remotePackageDirs: make(map[sourceaddrs.RemotePackage]string), | ||
remotePackageMeta: make(map[sourceaddrs.RemotePackage]*PackageMeta), | ||
registryPackageSources: make(map[regaddr.ModulePackage]map[versions.Version]sourceaddrs.RemoteSource), | ||
registryPackageVersionDeprecations: make(map[regaddr.ModulePackage]map[versions.Version]*RegistryVersionDeprecation), | ||
} | ||
|
||
manifestSrc, err := os.ReadFile(filepath.Join(rootDir, manifestFilename)) | ||
|
@@ -108,11 +110,17 @@ func OpenDir(baseDir string) (*Bundle, error) { | |
vs = make(map[versions.Version]sourceaddrs.RemoteSource) | ||
ret.registryPackageSources[pkgAddr] = vs | ||
} | ||
deprecations := ret.registryPackageVersionDeprecations[pkgAddr] | ||
if deprecations == nil { | ||
deprecations = make(map[versions.Version]*RegistryVersionDeprecation) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary reallocation I think (see previous comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose that similar to the above we could just assign the nested map at the index of vs := ret.registryPackageSources[pkgAddr]
if vs == nil {
vs = make(map[versions.Version]sourceaddrs.RemoteSource)
ret.registryPackageSources[pkgAddr] = vs
} |
||
ret.registryPackageVersionDeprecations[pkgAddr] = deprecations | ||
} | ||
for versionStr, mv := range rpm.Versions { | ||
version, err := versions.ParseVersion(versionStr) | ||
if err != nil { | ||
return nil, fmt.Errorf("invalid registry package version %q: %w", versionStr, err) | ||
} | ||
deprecations[version] = mv.Deprecation | ||
sourceAddr, err := sourceaddrs.ParseRemoteSource(mv.SourceAddr) | ||
if err != nil { | ||
return nil, fmt.Errorf("invalid registry package source address %q: %w", mv.SourceAddr, err) | ||
|
@@ -354,6 +362,10 @@ func (b *Bundle) RegistryPackageVersions(pkgAddr regaddr.ModulePackage) versions | |
return ret | ||
} | ||
|
||
func (b *Bundle) RegistryPackageVersionDeprecation(pkgAddr regaddr.ModulePackage, version versions.Version) *RegistryVersionDeprecation { | ||
return b.registryPackageVersionDeprecations[pkgAddr][version] | ||
} | ||
|
||
// RegistryPackageSourceAddr returns the remote source address corresponding | ||
// to the given version of the given module package, or sets its second return | ||
// value to false if no such version is included in the bundle. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a code comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Included 👍 ea8f696