diff --git a/docs/user/reference/cli/azldev_component_build.md b/docs/user/reference/cli/azldev_component_build.md index 2086f683..16a30f7f 100644 --- a/docs/user/reference/cli/azldev_component_build.md +++ b/docs/user/reference/cli/azldev_component_build.md @@ -57,7 +57,6 @@ azldev component build [flags] --mock-config-opt stringToString Pass a configuration option through to mock (key=value, can be specified multiple times) (default []) --no-check Skip package %check tests --preserve-buildenv policy Preserve build environment {on-failure, always, never} (default on-failure) - --skip-lock-validation skip lock file consistency checks -s, --spec-path stringArray Spec path --srpm-only Build SRPM (source RPM) *only* --without-git Skip creating a dist-git repository with synthetic commit history diff --git a/docs/user/reference/cli/azldev_component_diff-sources.md b/docs/user/reference/cli/azldev_component_diff-sources.md index afd471ac..34969640 100644 --- a/docs/user/reference/cli/azldev_component_diff-sources.md +++ b/docs/user/reference/cli/azldev_component_diff-sources.md @@ -22,7 +22,6 @@ azldev component diff-sources [flags] -g, --component-group stringArray Component group name -h, --help help for diff-sources --output-file string write the diff output to a file instead of stdout - --skip-lock-validation skip lock file consistency checks -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_list.md b/docs/user/reference/cli/azldev_component_list.md index 2900def0..ab8dc5d9 100644 --- a/docs/user/reference/cli/azldev_component_list.md +++ b/docs/user/reference/cli/azldev_component_list.md @@ -38,6 +38,7 @@ azldev component list [flags] -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -h, --help help for list + --skip-lock-population skip lock file population (default: true; set to false to read lock data) (default true) -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_prepare-sources.md b/docs/user/reference/cli/azldev_component_prepare-sources.md index 590286e7..7055caa9 100644 --- a/docs/user/reference/cli/azldev_component_prepare-sources.md +++ b/docs/user/reference/cli/azldev_component_prepare-sources.md @@ -39,7 +39,6 @@ azldev component prepare-sources [flags] --force delete and recreate the output directory if it already exists -h, --help help for prepare-sources -o, --output-dir string output directory - --skip-lock-validation skip lock file consistency checks --skip-overlays skip applying overlays to prepared sources --skip-sources skip downloading fetched sources when preparing the package (useful to extract dist-git metadata when source files are not needed) -s, --spec-path stringArray Spec path diff --git a/docs/user/reference/cli/azldev_component_query.md b/docs/user/reference/cli/azldev_component_query.md index 03a49fe1..26bd9750 100644 --- a/docs/user/reference/cli/azldev_component_query.md +++ b/docs/user/reference/cli/azldev_component_query.md @@ -34,7 +34,6 @@ azldev component query [flags] -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -h, --help help for query - --skip-lock-validation skip lock file consistency checks -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_render.md b/docs/user/reference/cli/azldev_component_render.md index 1ded12de..297ed8f3 100644 --- a/docs/user/reference/cli/azldev_component_render.md +++ b/docs/user/reference/cli/azldev_component_render.md @@ -62,7 +62,6 @@ azldev component render [flags] -f, --force allow overwriting existing rendered component directories -h, --help help for render -o, --output-dir string output directory for rendered specs (overrides rendered-specs-dir from config) - --skip-lock-validation skip lock file consistency checks -s, --spec-path stringArray Spec path ``` diff --git a/internal/app/azldev/cmds/component/changed.go b/internal/app/azldev/cmds/component/changed.go index 63a1b896..40128896 100644 --- a/internal/app/azldev/cmds/component/changed.go +++ b/internal/app/azldev/cmds/component/changed.go @@ -119,11 +119,13 @@ const ( func ChangedComponents( env *azldev.Env, options *ChangedComponentOptions, ) ([]ChangedResult, error) { - // Changed compares lock files between git refs — skip validation since - // the current working-tree locks may legitimately be stale. - options.ComponentFilter.SkipLockValidation = true + // Changed compares lock files between git refs — skip both validation and population + // since it reads locks directly from git commits (ReadAllAtCommit). + options.ComponentFilter.LockMode = components.LockModeSkipBoth resolver := components.NewResolver(env) + // Skip lock population since changed reads locks directly from git commits + resolver.SkipLockPopulation = true comps, err := resolver.FindComponents(&options.ComponentFilter) if err != nil { diff --git a/internal/app/azldev/cmds/component/list.go b/internal/app/azldev/cmds/component/list.go index ac348c81..ba8e3236 100644 --- a/internal/app/azldev/cmds/component/list.go +++ b/internal/app/azldev/cmds/component/list.go @@ -17,6 +17,9 @@ import ( type ListComponentOptions struct { // Standard filter for selecting components. ComponentFilter components.ComponentFilter + // SkipLockPopulation controls whether lock files are read during resolution. + // Only applicable when component list is explicitly requested by the user. + SkipLockPopulation bool } func listOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { @@ -57,26 +60,32 @@ Component name patterns support glob syntax (*, ?, []).`, components.AddComponentFilterOptionsToCommand(cmd, &options.ComponentFilter) - // List always skips lock validation (read-only), so the flag is - // meaningless here. Hide it to avoid confusion. - _ = cmd.Flags().MarkHidden("skip-lock-validation") + // Add skip-lock-population flag for list command + cmd.Flags().BoolVar(&options.SkipLockPopulation, "skip-lock-population", + true, + "skip lock file population (default: true; set to false to read lock data)") return cmd } // ListComponentConfigs lists components in the env, in accordance with options. -// Lock validation is always skipped regardless of the caller's SkipLockValidation -// value — list is read-only. +// Lock validation is always skipped since list is read-only. func ListComponentConfigs( env *azldev.Env, options *ListComponentOptions, ) (results []projectconfig.ComponentConfig, err error) { var comps *components.ComponentSet - // List is read-only — skip lock validation so it works even when - // locks are missing or stale. - options.ComponentFilter.SkipLockValidation = true + // List is read-only — always skip lock validation. + // Determine lock mode based on user preference for population. + if options.SkipLockPopulation { + options.ComponentFilter.LockMode = components.LockModeSkipBoth + } else { + options.ComponentFilter.LockMode = components.LockModeSkipValidationPopulate + } resolver := components.NewResolver(env) + // Set the resolver's SkipLockPopulation to match the filter's LockMode + resolver.SkipLockPopulation = options.SkipLockPopulation comps, err = resolver.FindComponents(&options.ComponentFilter) if err != nil { diff --git a/internal/app/azldev/cmds/component/update.go b/internal/app/azldev/cmds/component/update.go index 906162fa..ee177bbe 100644 --- a/internal/app/azldev/cmds/component/update.go +++ b/internal/app/azldev/cmds/component/update.go @@ -150,8 +150,7 @@ type UpdateResult struct { // UpdateComponents resolves source identities for all selected components and // writes the results to per-component lock files under locks/. -// Lock validation is always skipped regardless of the caller's SkipLockValidation -// value — update is the lock writer. +// Lock validation is always skipped — update is the lock writer. func UpdateComponents(env *azldev.Env, options *UpdateComponentOptions) ([]UpdateResult, error) { if options.Bump && options.CheckOnly { return nil, fmt.Errorf("%w: --bump and --check-only are mutually exclusive", azldev.ErrInvalidUsage) @@ -161,9 +160,10 @@ func UpdateComponents(env *azldev.Env, options *UpdateComponentOptions) ([]Updat // Suppress staleness warnings — we're about to refresh the locks ourselves, // so warning the user to "run component update" would be self-referential noise. resolver.SuppressLockWarnings = true - // Skip lock validation — update is the lock file writer, so missing or - // stale locks are expected and will be fixed by this command. - options.ComponentFilter.SkipLockValidation = true + // Skip lock validation but keep population — update is the lock file writer, + // so missing or stale locks are expected and will be fixed by this command. + // We still need to populate locks to compute freshness. + options.ComponentFilter.LockMode = components.LockModeSkipValidationPopulate // Enable freshness checking so the resolver computes FreshnessStatus for // each component. This lets resolveSourceIdentitiesParallel skip // re-resolution for components whose resolution inputs haven't changed. diff --git a/internal/app/azldev/core/components/filter.go b/internal/app/azldev/core/components/filter.go index 95c9b582..d1416ca8 100644 --- a/internal/app/azldev/core/components/filter.go +++ b/internal/app/azldev/core/components/filter.go @@ -10,6 +10,35 @@ import ( "github.com/spf13/cobra" ) +// LockMode defines how lock files should be handled during component resolution. +type LockMode int + +const ( + // LockModeValidateAndPopulate validates lock file consistency and populates lock data on components (default). + LockModeValidateAndPopulate LockMode = iota + // LockModeSkipValidationPopulate skips lock file validation but still populates lock data on components. + // Used by commands that build/render components and consume lock data as-is. + LockModeSkipValidationPopulate + // LockModeSkipBoth skips both lock file validation and population of lock data. + // Lock population can only be skipped if validation is also skipped. + // Used by read-only commands (list, changed) that don't consume lock data. + LockModeSkipBoth +) + +// String returns a string representation of the LockMode. +func (m LockMode) String() string { + switch m { + case LockModeValidateAndPopulate: + return "validateAndPopulate" + case LockModeSkipValidationPopulate: + return "skipValidationPopulate" + case LockModeSkipBoth: + return "skipBoth" + default: + return "unknown" + } +} + // Describes a filter that selects a subset of components known within the environment's // loaded configuration. type ComponentFilter struct { @@ -22,10 +51,10 @@ type ComponentFilter struct { SpecPaths []string // If true, then *all* known components are included in the result set. IncludeAllComponents bool - // SkipLockValidation disables lock file consistency checks for this - // filter's resolution. Commands that write lock files (update) or are - // read-only (list) set this to true. - SkipLockValidation bool + // LockMode defines how lock file validation and population should be handled during resolution. + // Default is LockModeValidateAndPopulate. + // Lock population can only be skipped if validation is also skipped (LockModeSkipBoth). + LockMode LockMode } // HasNoCriteria returns true if the filter has no criteria set, meaning that it will never @@ -53,9 +82,8 @@ func AddComponentFilterOptionsToCommand(cmd *cobra.Command, filter *ComponentFil cmd.Flags().StringArrayVarP(&filter.SpecPaths, "spec-path", "s", []string{}, "Spec path") _ = cmd.MarkFlagFilename("spec-path", ".spec") - cmd.Flags().BoolVar(&filter.SkipLockValidation, "skip-lock-validation", - false, - "skip lock file consistency checks") + // Note: skip-lock-validation flag is added per-command, not here, to allow customization + // (e.g., hide for commands that always skip, or pair with skip-lock-population for list) } // Function suitable for use as a [cobra.ValidArgsFunction] in a [cobra.Command]. Intended for use diff --git a/internal/app/azldev/core/components/resolver.go b/internal/app/azldev/core/components/resolver.go index 5001ac1b..33908c69 100644 --- a/internal/app/azldev/core/components/resolver.go +++ b/internal/app/azldev/core/components/resolver.go @@ -4,6 +4,7 @@ package components import ( + "context" "errors" "fmt" "log/slog" @@ -16,6 +17,7 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/fingerprint" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/parmap" ) // Well-known errors. @@ -41,6 +43,11 @@ type Resolver struct { // Only 'component update' enables this — it uses the freshness status to // decide which components need re-resolution and which can be skipped. CheckFreshness bool + // SkipLockPopulation skips reading lock files and attaching lock data to + // resolved components. When true, component Locked fields remain nil. + // Set this when the resolver's lock mode requires skipping population + // (e.g., when called from a command with LockModeSkipBoth). + SkipLockPopulation bool } // NewResolver constructs a new [Resolver] for the given environment. @@ -52,11 +59,20 @@ func NewResolver(env *azldev.Env) *Resolver { // Given a component filter, finds all components defined in the environment that match the filter. func (r *Resolver) FindComponents(filter *ComponentFilter) (components *ComponentSet, err error) { - // The filter's SkipLockValidation field is the primary control. When - // created via AddComponentFilterOptionsToCommand, its default is false - // (validation on). Commands that write locks (update) or are read-only - // (list) explicitly set it to true. - skipValidation := filter.SkipLockValidation + // Determine whether to skip lock validation based on the filter's LockMode. + skipValidation := filter.LockMode != LockModeValidateAndPopulate + // Also check the resolver's SkipLockPopulation field (set programmatically by callers) + skipPopulation := filter.LockMode == LockModeSkipBoth || r.SkipLockPopulation + + // Temporarily set r.SkipLockPopulation during this resolution to ensure + // FindAllComponents and other methods respect the skip requirement. + oldSkipLockPopulation := r.SkipLockPopulation + if skipPopulation { + r.SkipLockPopulation = true + } + defer func() { + r.SkipLockPopulation = oldSkipLockPopulation + }() // For usability's sake, detect if the caller/user forgot to specify *any* criteria. if filter.HasNoCriteria() { @@ -72,7 +88,13 @@ func (r *Resolver) FindComponents(filter *ComponentFilter) (components *Componen return allComps, findErr } - r.warnOnLockDrift(allComps) + if !skipPopulation { + r.populateLocksParallel(allComps) + } + + if !r.SuppressLockWarnings { + r.warnOnLockDrift(allComps) + } return allComps, r.validateLockFiles(allComps, true, skipValidation) } @@ -103,6 +125,10 @@ func (r *Resolver) FindComponents(filter *ComponentFilter) (components *Componen } } + if !skipPopulation { + r.populateLocksParallel(components) + } + r.warnOnLockDrift(components) return components, r.validateLockFiles(components, false, skipValidation) @@ -162,6 +188,10 @@ func (r *Resolver) FindAllComponents() (components *ComponentSet, err error) { components.Add(comp) } + if !r.SkipLockPopulation { + r.populateLocksParallel(components) + } + return components, nil } @@ -497,17 +527,23 @@ func (r *Resolver) createComponentFromConfig(componentConfig *projectconfig.Comp componentConfig.Release.Calculation = projectconfig.ReleaseCalculationAuto } - // Populate locked state onto the component config. This makes lock data - // available to all downstream consumers (render, build, prepare-sources, - // diff-sources) without each needing lock-file awareness. - r.populateFromLock(componentConfig) - return &resolvedComponent{ env: r.env, config: *componentConfig, }, nil } +func (r *Resolver) populateLocksParallel(set *ComponentSet) { + comps := set.Components() + + _ = parmap.Map(r.env, r.env.IOBoundConcurrency(), comps, nil, + func(_ context.Context, c Component) error { + r.populateFromLock(c.GetConfig()) + + return nil + }) +} + // populateFromLock reads lock file data and attaches it to the component config // as [projectconfig.ComponentLockData]. This centralizes lock-file consumption so // all downstream commands (render, build, prepare-sources, diff-sources) get diff --git a/internal/app/azldev/core/components/resolver_test.go b/internal/app/azldev/core/components/resolver_test.go index dd734667..6b0daae1 100644 --- a/internal/app/azldev/core/components/resolver_test.go +++ b/internal/app/azldev/core/components/resolver_test.go @@ -769,7 +769,7 @@ func TestFindComponents_ErrorWhenNoLockFile(t *testing.T) { assert.Contains(t, err.Error(), "lock file") } -// When SkipLockValidation is set, a missing lock file is tolerated — +// When LockMode is set to skip validation, a missing lock file is tolerated — // the component's Locked field is nil instead of causing an error. func TestFindComponents_LockedNilWhenValidationSkipped(t *testing.T) { env := testutils.NewTestEnv(t) @@ -784,7 +784,7 @@ func TestFindComponents_LockedNilWhenValidationSkipped(t *testing.T) { filter := &components.ComponentFilter{ IncludeAllComponents: true, - SkipLockValidation: true, + LockMode: components.LockModeSkipValidationPopulate, } resolved, err := components.NewResolver(env.Env).FindComponents(filter) @@ -820,7 +820,7 @@ func TestFindComponents_ExplicitPinPreserved(t *testing.T) { filter := &components.ComponentFilter{ IncludeAllComponents: true, - SkipLockValidation: true, + LockMode: components.LockModeSkipValidationPopulate, } resolved, err := components.NewResolver(env.Env).FindComponents(filter) @@ -921,7 +921,7 @@ func TestFindComponents_CorruptLockSurfaces(t *testing.T) { filter := &components.ComponentFilter{ IncludeAllComponents: true, - SkipLockValidation: true, + LockMode: components.LockModeSkipValidationPopulate, } resolved, err := components.NewResolver(env.Env).FindComponents(filter) @@ -955,7 +955,7 @@ func TestFindComponents_PinDiffersFromLock(t *testing.T) { filter := &components.ComponentFilter{ IncludeAllComponents: true, - SkipLockValidation: true, + LockMode: components.LockModeSkipValidationPopulate, } resolved, err := components.NewResolver(env.Env).FindComponents(filter) @@ -1011,3 +1011,41 @@ func TestComponentFilter_HasNoCriteria(t *testing.T) { assert.False(t, filter.HasNoCriteria()) }) } + +// When LockMode is set to LockModeSkipBoth, lock files are not read +// and Locked remains nil — even when a valid lock file exists on disk. +func TestFindComponents_SkipLockPopulation(t *testing.T) { + env := testutils.NewTestEnv(t) + + env.Config.Components["curl"] = projectconfig.ComponentConfig{ + Name: "curl", + Spec: projectconfig.SpecSource{ + SourceType: projectconfig.SpecSourceTypeUpstream, + UpstreamCommit: "config-pin-aaa", + }, + } + + // Write a valid lock file that would normally be read. + lock := lockfile.New() + lock.UpstreamCommit = "lock-commit-bbb" + + env.WriteLock(t, "curl", lock) + + filter := &components.ComponentFilter{ + IncludeAllComponents: true, + LockMode: components.LockModeSkipBoth, + } + + resolver := components.NewResolver(env.Env) + + resolved, err := resolver.FindComponents(filter) + require.NoError(t, err) + + comp, ok := resolved.TryGet("curl") + require.True(t, ok) + + assert.Nil(t, comp.GetConfig().Locked, + "Locked must be nil when LockModeSkipBoth is set, even with a valid lock on disk") + assert.Equal(t, "config-pin-aaa", comp.GetConfig().Spec.UpstreamCommit, + "spec pin must be preserved regardless") +} diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 9d45a2b4..4d75e301 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -314,8 +314,8 @@ func (c *ComponentConfig) MergeUpdatesFrom(other *ComponentConfig) error { // EffectiveUpstreamCommit returns the commit to use for upstream operations. // Prefers the locked commit (resolved reality) over the config pin (user intent). -// Falls back to Spec.UpstreamCommit for SkipLockValidation paths (update, list, -// changed) where Locked may be nil. Returns empty string when neither is set. +// Falls back to Spec.UpstreamCommit for LockModeSkipBoth paths (list, changed) +// where Locked may be nil. Returns empty string when neither is set. func (c *ComponentConfig) EffectiveUpstreamCommit() string { if c.Locked != nil && c.Locked.UpstreamCommit != "" { return c.Locked.UpstreamCommit