Add deprecated dependency warnings#213
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new CLI command to report deprecated installed dependency versions by checking resolved versions (lockfiles and Go modules) against registry metadata. This fits alongside existing “enrichment” commands like outdated/licenses by providing another external-metadata check.
Changes:
- Introduces
git pkgs deprecatedwithgit pkgs deprecationsalias and--ecosystem/--commit/--branch/--format jsonflags. - Implements registry lookups for exact installed versions and outputs results in text/JSON.
- Updates docs and command lists/tests to include the new command.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new deprecated command and updates the external-metadata command summary. |
| docs/internals.md | Adds deprecated to the list of commands that fetch external metadata. |
| cmd/root.go | Registers the new deprecated command with the root CLI. |
| cmd/pager_test.go | Ensures the pager flag is accepted for the new command. |
| cmd/deprecated.go | Implements the deprecated-version lookup + text/JSON output. |
| cmd/deprecated_test.go | Adds unit tests for PURL generation and deprecated filtering/message extraction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
andrew
left a comment
There was a problem hiding this comment.
Thanks for picking this up.
The main thing to sort out before merging is request volume. fetchDeprecatedVersionData calls registries.BulkFetchVersions directly, which fires one HTTP request per PURL with no caching. On a repo with a few hundred lockfile entries that's a few hundred registry hits every time the command runs.
The existing network-backed commands (outdated, licenses) go through the cache layer: check db.GetCachedPackages / db.GetCachedVersions with the 24h TTL, fetch only the misses via github.com/git-pkgs/enrichment, then write results back with db.SaveVersions. See cmd/outdated.go around line 190 for the pattern. deprecated should follow the same path.
CachedVersion doesn't currently have a status field so this will need either a column on the versions table or, if ecosyste.ms exposes deprecation at the package level, storing it on CachedPackage instead. Happy to discuss which way to go before you rework it.
Two smaller things:
- When
--format jsonis set, the empty-result paths still printNo resolved dependencies found./No deprecated dependencies found.as plain text. They should emit[]so the output is always valid JSON. Same issue was just fixed forlistin #208. - The README change at line 14 rewrites the existing sentence and drops the ecosyste.ms link from the description of
outdated/licenses. Please keep that sentence as it was and just adddeprecatedto the list of network commands.
thanks, fixed... deprecated now uses the versions cache with the existing 24h TTL pattern: it reads cached exact-version status first, only fetches cache misses, and writes the fetched status/message metadata back into the cache. I added status, status_checked_at, and metadata fields to versions so cached rows can distinguish “deprecation status was checked and is empty” from older enrichment-only rows that never had status data. i kept the miss fetch through registries because enrichment.VersionInfo does not currently expose deprecation status, but the request volume issue is addressed by caching those exact-version registry results... also fixed the JSON empty-result paths to emit [] and restored the README sentence with the ecosyste.ms link while adding deprecated to the network-backed commands list.... |
andrew
left a comment
There was a problem hiding this comment.
The cache rework looks right. StatusCheckedAt separate from enriched_at is a good call, and the UPSERT CASE WHEN excluded.status_checked_at != '' means outdated/freshness writes won't clobber stored status. I'd pointed at enrichment for the fetch but VersionInfo has no status field so going direct to registries for misses is correct here.
The JSON empty-result and README points are sorted.
One thing left: deprecatedLookupTimeout = 60 * time.Second wraps the whole BulkFetchVersions call. On a cold cache with a few hundred misses that bulk fetch can take longer than a minute even with concurrency. The other network commands (resolve, add, remove, vulns) use 5 minutes; please match that.
This will also need a rebase onto main. #212 is landing ahead of it and adds an identical versionFromPURL helper plus overlapping lines in root.go, pager_test.go, internals.md and the README quick-start block, so you'll hit a duplicate symbol and a few text conflicts. Drop the local versionFromPURL in deprecated.go and use the one from freshness.go when you rebase.
8d554e6 to
5d51c22
Compare
thanks, fixed... i rebased the branch onto main, resolved the freshness/deprecated conflicts and removed the duplicate versionFromPURL helper from deprecated.go so it uses the helper from freshness.go... I also changed the deprecated registry miss lookup timeout from 60 seconds to 5 minutes to match the longer-running network commands.... tests pass with go test ./cmd -run 'Deprecated|Freshness', go test ./internal/database, go test ./..., and git diff --check... |
andrew
left a comment
There was a problem hiding this comment.
Thanks, both points are addressed: the lookup timeout now matches the other network commands at 5 minutes, and the rebase onto main is clean with the duplicate versionFromPURL dropped in favour of the one in freshness.go.
The cache path, schema bump, and UPSERT guards are unchanged from what I reviewed last time and still look right.
Closes #21
Summary
Adds a new
git pkgs deprecatedcommand, withgit pkgs deprecationsas an alias.The command checks resolved installed dependency versions against registry metadata and reports versions marked as deprecated.
Details
--ecosystem,--commit,--branchand--format jsonTests
go build ./...go test ./...go test -race ./...go tool golangci-lint run ./...git diff --check