Skip to content

Commit 603370f

Browse files
committed
wip 4
1 parent 708b0b7 commit 603370f

File tree

7 files changed

+150
-31
lines changed

7 files changed

+150
-31
lines changed

internal/app/azldev/cmds/component/identity.go

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package component
66
import (
77
"fmt"
88
"log/slog"
9+
"path/filepath"
910
"sync"
1011

1112
"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev"
@@ -89,20 +90,14 @@ func ComputeComponentIdentities(
8990

9091
distroRef := env.Config().Project.DefaultDistro
9192

92-
// Resolve the distro definition (fills in default version).
93-
distroRef, distro, err := resolveDistroForIdentity(env, distroRef)
93+
// Resolve the distro definition (fills in default version for the fingerprint).
94+
distroRef, _, err = resolveDistroForIdentity(env, distroRef)
9495
if err != nil {
9596
slog.Debug("Could not resolve distro", "error", err)
9697
}
9798

98-
// Create a source manager — it handles provider construction and dispatch.
99-
srcManager, err := sourceproviders.NewSourceManager(env, distro)
100-
if err != nil {
101-
return nil, fmt.Errorf("creating source manager:\n%w", err)
102-
}
103-
10499
return computeIdentitiesParallel(
105-
env, comps.Components(), distroRef, srcManager,
100+
env, comps.Components(), distroRef,
106101
)
107102
}
108103

@@ -116,7 +111,6 @@ func computeIdentitiesParallel(
116111
env *azldev.Env,
117112
comps []components.Component,
118113
distroRef projectconfig.DistroReference,
119-
srcManager sourceproviders.SourceManager,
120114
) ([]ComponentIdentityResult, error) {
121115
progressEvent := env.StartEvent("Computing component identities",
122116
"count", len(comps))
@@ -150,7 +144,7 @@ func computeIdentitiesParallel(
150144
}
151145

152146
result, computeErr := computeSingleIdentity(
153-
env, comp, distroRef, srcManager,
147+
env, comp, distroRef,
154148
)
155149

156150
resultsChan <- indexedResult{index: compIdx, result: result, err: computeErr}
@@ -189,7 +183,6 @@ func computeSingleIdentity(
189183
env *azldev.Env,
190184
comp components.Component,
191185
distroRef projectconfig.DistroReference,
192-
srcManager sourceproviders.SourceManager,
193186
) (ComponentIdentityResult, error) {
194187
config := comp.GetConfig()
195188
componentName := comp.GetName()
@@ -198,12 +191,12 @@ func computeSingleIdentity(
198191
AffectsCommitCount: countAffectsCommits(config, componentName),
199192
}
200193

201-
// Resolve source identity via the source manager (handles local/upstream dispatch + retry).
202-
sourceIdentity, resolveErr := srcManager.ResolveSourceIdentity(env, comp)
203-
if resolveErr != nil {
194+
// Resolve source identity, selecting the appropriate method based on source type (local vs. upstream etc.).
195+
sourceIdentity, err := resolveSourceIdentityForComponent(env, comp)
196+
if err != nil {
204197
return ComponentIdentityResult{}, fmt.Errorf(
205198
"source identity resolution failed for %#q:\n%w",
206-
componentName, resolveErr)
199+
componentName, err)
207200
}
208201

209202
identityOpts.SourceIdentity = sourceIdentity
@@ -259,3 +252,52 @@ func countAffectsCommits(
259252

260253
return commits
261254
}
255+
256+
// resolveSourceIdentityForComponent returns a deterministic identity string for the
257+
// component's source. Local components are handled directly (spec directory hash) to
258+
// avoid constructing a full source manager. Upstream components go through the source
259+
// manager which handles provider dispatch and retry.
260+
func resolveSourceIdentityForComponent(
261+
env *azldev.Env, comp components.Component,
262+
) (string, error) {
263+
sourceType := comp.GetConfig().Spec.SourceType
264+
265+
// Local components: hash the spec directory directly without a source manager.
266+
if sourceType == projectconfig.SpecSourceTypeLocal ||
267+
sourceType == projectconfig.SpecSourceTypeUnspecified {
268+
specDir := ""
269+
if comp.GetConfig().Spec.Path != "" {
270+
specDir = filepath.Dir(comp.GetConfig().Spec.Path)
271+
}
272+
273+
identity, localErr := sourceproviders.NewLocalIdentityProvider(env.FS(), specDir).
274+
ResolveSourceIdentity(env, comp)
275+
if localErr != nil {
276+
return "", fmt.Errorf("resolving local source identity for %#q:\n%w",
277+
comp.GetName(), localErr)
278+
}
279+
280+
return identity, nil
281+
}
282+
283+
// Upstream components: need a source manager with the component's resolved distro.
284+
distro, err := sourceproviders.ResolveDistro(env, comp)
285+
if err != nil {
286+
return "", fmt.Errorf("resolving distro for component %#q:\n%w",
287+
comp.GetName(), err)
288+
}
289+
290+
srcManager, err := sourceproviders.NewSourceManager(env, distro)
291+
if err != nil {
292+
return "", fmt.Errorf("creating source manager for component %#q:\n%w",
293+
comp.GetName(), err)
294+
}
295+
296+
identity, err := srcManager.ResolveSourceIdentity(env, comp)
297+
if err != nil {
298+
return "", fmt.Errorf("resolving upstream source identity for %#q:\n%w",
299+
comp.GetName(), err)
300+
}
301+
302+
return identity, nil
303+
}

internal/fingerprint/fingerprint.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func ComputeIdentity(
9191

9292
inputs.OverlayFileHashes = overlayHashes
9393

94-
// 4. Combine all inputs into the overall fingerprint.
94+
// 3. Combine all inputs into the overall fingerprint.
9595
return &ComponentIdentity{
9696
Fingerprint: combineInputs(inputs),
9797
Inputs: inputs,

internal/fingerprint/fingerprint_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,31 @@ func TestComputeIdentity_MergeUpdatesFromPropagation(t *testing.T) {
535535
assert.NotEqual(t, fpBefore, fpAfter,
536536
"merged distro default must change the fingerprint")
537537
}
538+
539+
func TestComputeIdentity_SnapshotChangeDoesNotAffectFingerprint(t *testing.T) {
540+
ctx := newTestFS(t, nil)
541+
542+
comp := projectconfig.ComponentConfig{
543+
Spec: projectconfig.SpecSource{
544+
SourceType: projectconfig.SpecSourceTypeUpstream,
545+
UpstreamName: "curl",
546+
UpstreamCommit: "abc1234",
547+
UpstreamDistro: projectconfig.DistroReference{
548+
Name: "fedora",
549+
Version: "41",
550+
Snapshot: "2025-01-01T00:00:00Z",
551+
},
552+
},
553+
}
554+
distro := baseDistroRef()
555+
556+
fp1 := computeFingerprint(t, ctx, comp, distro, 0)
557+
558+
// Change only the snapshot timestamp.
559+
comp.Spec.UpstreamDistro.Snapshot = "2026-06-15T00:00:00Z"
560+
fp2 := computeFingerprint(t, ctx, comp, distro, 0)
561+
562+
assert.Equal(t, fp1, fp2,
563+
"changing upstream distro snapshot must NOT change fingerprint "+
564+
"(snapshot is excluded; resolved commit hash is what matters)")
565+
}

internal/projectconfig/distro.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type DistroReference struct {
1818
// Version of the referenced distro.
1919
Version string `toml:"version,omitempty" json:"version,omitempty" jsonschema:"title=Version,description=Version of the referenced distro"`
2020
// Snapshot date/time for source code if specified components will use source as it existed at this time.
21-
Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time"`
21+
Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time" fingerprint:"-"`
2222
}
2323

2424
// Implements the [Stringer] interface for [DistroReference].

internal/projectconfig/fingerprint_test.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,31 +38,43 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) {
3838

3939
// Maps "StructName.FieldName" for every field that should carry a
4040
// `fingerprint:"-"` tag. Catches accidental tag removal.
41+
//
42+
// Each entry documents WHY the field is excluded from the fingerprint:
4143
expectedExclusions := map[string]bool{
42-
// ComponentConfig
43-
"ComponentConfig.Name": true,
44+
// ComponentConfig.Name — metadata, already the map key in project config.
45+
"ComponentConfig.Name": true,
46+
// ComponentConfig.SourceConfigFile — internal bookkeeping reference, not a build input.
4447
"ComponentConfig.SourceConfigFile": true,
4548

46-
// ComponentBuildConfig
49+
// ComponentBuildConfig.Failure — CI policy (expected failure tracking), not a build input.
4750
"ComponentBuildConfig.Failure": true,
48-
"ComponentBuildConfig.Hints": true,
51+
// ComponentBuildConfig.Hints — scheduling hints (e.g. expensive), not a build input.
52+
"ComponentBuildConfig.Hints": true,
4953

50-
// CheckConfig
54+
// CheckConfig.SkipReason — human documentation for why check is skipped, not a build input.
5155
"CheckConfig.SkipReason": true,
5256

5357
// ComponentBuildFailureConfig — entire struct excluded via parent, but individual
5458
// fields are also tagged so reflection on the struct alone is consistent.
55-
"ComponentBuildFailureConfig.Expected": true,
59+
// ComponentBuildFailureConfig.Expected — CI decision about expected failures.
60+
"ComponentBuildFailureConfig.Expected": true,
61+
// ComponentBuildFailureConfig.ExpectedReason — documentation for expected failure.
5662
"ComponentBuildFailureConfig.ExpectedReason": true,
5763

5864
// ComponentBuildHints — entire struct excluded via parent, fields also tagged.
65+
// ComponentBuildHints.Expensive — scheduling hint, does not affect build output.
5966
"ComponentBuildHints.Expensive": true,
6067

61-
// ComponentOverlay
68+
// ComponentOverlay.Description — human-readable documentation for the overlay.
6269
"ComponentOverlay.Description": true,
6370

64-
// SourceFileReference
71+
// SourceFileReference.Component — back-reference to parent, not a build input.
6572
"SourceFileReference.Component": true,
73+
74+
// DistroReference.Snapshot — snapshot timestamp is not a build input; the resolved
75+
// upstream commit hash (captured separately via SourceIdentity) is what matters.
76+
// Excluding this prevents a snapshot bump from marking all upstream components as changed.
77+
"DistroReference.Snapshot": true,
6678
}
6779

6880
// Collect all actual exclusions found via reflection.

internal/providers/sourceproviders/fedorasourceprovider.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,19 +274,33 @@ func (g *FedoraSourcesProviderImpl) checkoutTargetCommit(
274274
}
275275

276276
// ResolveSourceIdentity implements [SourceIdentityProvider] by resolving the upstream
277-
// commit hash for the component. Uses the same resolution priority as [checkoutTargetCommit].
277+
// commit hash for the component. Resolution priority matches [checkoutTargetCommit]:
278+
// 1. Explicit upstream commit hash (pinned per-component) — returned directly.
279+
// 2. Otherwise — query HEAD of the dist-git branch via ls-remote.
280+
//
281+
// Snapshot-time resolution is not supported here because it requires a full clone
282+
// to walk git history. Snapshot changes are tracked separately via the config hash.
278283
func (g *FedoraSourcesProviderImpl) ResolveSourceIdentity(
279284
ctx context.Context,
280285
component components.Component,
281286
) (string, error) {
287+
// Case 1: Explicit upstream commit hash — no network call needed.
288+
if pinnedCommit := component.GetConfig().Spec.UpstreamCommit; pinnedCommit != "" {
289+
slog.Debug("Using pinned upstream commit for identity",
290+
"component", component.GetName(),
291+
"commit", pinnedCommit)
292+
293+
return pinnedCommit, nil
294+
}
295+
296+
// Case 2: Query HEAD of the dist-git branch.
282297
upstreamName := component.GetConfig().Spec.UpstreamName
283298
if upstreamName == "" {
284299
upstreamName = component.GetName()
285300
}
286301

287302
gitRepoURL := strings.ReplaceAll(g.distroGitBaseURI, "$pkg", upstreamName)
288303

289-
// Query HEAD of the dist-git branch with retry for transient failures.
290304
var commitHash string
291305

292306
retryErr := retry.Do(ctx, g.retryConfig, func() error {

internal/providers/sourceproviders/identityprovider_test.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,22 @@ func TestFedoraProvider_ResolveSourceIdentity(t *testing.T) {
139139
require.Error(t, resolveErr)
140140
assert.Contains(t, resolveErr.Error(), testPackageName)
141141
})
142+
143+
t.Run("returns pinned commit without network call", func(t *testing.T) {
144+
pinnedCommit := "deadbeef12345678"
145+
comp := newMockCompWithConfig(ctrl, testPackageName, &projectconfig.ComponentConfig{
146+
Name: testPackageName,
147+
Spec: projectconfig.SpecSource{
148+
SourceType: projectconfig.SpecSourceTypeUpstream,
149+
UpstreamCommit: pinnedCommit,
150+
},
151+
})
152+
153+
// No LsRemoteHead expectation — the pinned commit should be returned directly.
154+
identity, resolveErr := provider.ResolveSourceIdentity(t.Context(), comp)
155+
require.NoError(t, resolveErr)
156+
assert.Equal(t, pinnedCommit, identity)
157+
})
142158
}
143159

144160
// --- RPMContentsProviderImpl.ResolveSourceIdentity tests ---
@@ -184,12 +200,19 @@ func TestRPMProvider_ResolveSourceIdentity(t *testing.T) {
184200

185201
// newMockComp creates a mock component with the given name and an empty upstream config.
186202
func newMockComp(ctrl *gomock.Controller, name string) *components_testutils.MockComponent {
187-
comp := components_testutils.NewMockComponent(ctrl)
188-
comp.EXPECT().GetName().AnyTimes().Return(name)
189-
comp.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
203+
return newMockCompWithConfig(ctrl, name, &projectconfig.ComponentConfig{
190204
Name: name,
191205
Spec: projectconfig.SpecSource{},
192206
})
207+
}
208+
209+
// newMockCompWithConfig creates a mock component with the given name and a custom config.
210+
func newMockCompWithConfig(
211+
ctrl *gomock.Controller, name string, config *projectconfig.ComponentConfig,
212+
) *components_testutils.MockComponent {
213+
comp := components_testutils.NewMockComponent(ctrl)
214+
comp.EXPECT().GetName().AnyTimes().Return(name)
215+
comp.EXPECT().GetConfig().AnyTimes().Return(config)
193216

194217
return comp
195218
}

0 commit comments

Comments
 (0)