feat(Provider): Add ResolveSourceIdentity() to the source provider interface#45
Open
dmcilvaney wants to merge 1 commit intomicrosoft:mainfrom
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “source identity” concept to source providers and the source manager so callers can compute a deterministic, reproducible identifier for a component’s source (e.g., commit hash for dist-git, content hash for local specs).
Changes:
- Introduce
SourceIdentityProviderand extendComponentSourceProvider/SourceManagerwithResolveSourceIdentity(...). - Implement identity resolution for upstream providers (Fedora provider resolves an effective commit; RPM contents provider hashes downloaded RPM content).
- Add local spec-directory identity hashing via a new
ResolveLocalSourceIdentity(...)helper.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/providers/sourceproviders/sourceproviders_test/sourcemanager_mocks.go | Updates SourceManager mock to include ResolveSourceIdentity (test support). |
| internal/providers/sourceproviders/sourcemanager.go | Adds SourceIdentityProvider, wires identity resolution into SourceManager. |
| internal/providers/sourceproviders/rpmcontentsprovider.go | Implements identity for RPM-backed sources by hashing the downloaded RPM. |
| internal/providers/sourceproviders/localidentity.go | Adds deterministic hashing of all files under a local spec directory. |
| internal/providers/sourceproviders/identityprovider_test.go | Adds unit tests around identity resolution helpers/providers. |
| internal/providers/sourceproviders/fedorasourceprovider.go | Implements upstream identity resolution based on pinned commit / snapshot time / current HEAD. |
625fb53 to
5c27839
Compare
reubeno
reviewed
Mar 31, 2026
f231810 to
67dc565
Compare
67dc565 to
f159df5
Compare
f159df5 to
aa96fa6
Compare
dmcilvaney
commented
Mar 31, 2026
| // Collect all files in the spec directory. | ||
| var filePaths []string | ||
|
|
||
| err := afero.Walk(filesystem, specDir, |
Contributor
Author
There was a problem hiding this comment.
Is there any concern about recursive fingerprinting of toml files...
Probably an edge case we can just accept for now.
aa96fa6 to
4ac2bc4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.