From e6642f729c2e8de0cde395daf880f32f630dbedc Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Mon, 11 May 2026 22:25:53 -0700 Subject: [PATCH 1/5] feat(render): add --allow-dirty flag for uncommitted lock state Add --allow-dirty to build, render, and prepare-sources commands to control how uncommitted lock file changes are handled during synthetic history generation. When --allow-dirty is set, uncommitted lock data is used as a fallback when no committed lock exists at HEAD, and fingerprint mismatches produce dirty synthetic entries. When unset, uncommitted state produces errors with hints suggesting --allow-dirty. - Add allowDirty + hints plumbing through buildSyntheticCommits - Extract handleMissingHeadLock and handleDirtyState helpers - Add Hints() to SourcePreparer interface for caller-facing suggestions - Always compute fingerprints (local I/O only, no network) - Add WithPreScript option to scenario ProjectTest - Scenario build test commits locks before building - Regenerate CLI docs --- .../reference/cli/azldev_component_build.md | 1 + .../cli/azldev_component_prepare-sources.md | 1 + .../reference/cli/azldev_component_render.md | 1 + internal/app/azldev/cmds/component/build.go | 16 +- .../azldev/cmds/component/preparesources.go | 19 +- internal/app/azldev/cmds/component/render.go | 34 ++- .../app/azldev/core/sources/sourceprep.go | 53 ++-- .../app/azldev/core/sources/synthistory.go | 117 +++++++- .../core/sources/synthistory_internal_test.go | 256 ++++++++++++++++++ scenario/component_build_test.go | 2 + scenario/internal/projecttest/projecttest.go | 13 + 11 files changed, 473 insertions(+), 40 deletions(-) create mode 100644 internal/app/azldev/core/sources/synthistory_internal_test.go diff --git a/docs/user/reference/cli/azldev_component_build.md b/docs/user/reference/cli/azldev_component_build.md index 2086f683..00a3586d 100644 --- a/docs/user/reference/cli/azldev_component_build.md +++ b/docs/user/reference/cli/azldev_component_build.md @@ -48,6 +48,7 @@ azldev component build [flags] ``` -a, --all-components Include all components + -d, --allow-dirty include uncommitted lock file changes in synthetic history -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -k, --continue-on-error Continue building when some components fail diff --git a/docs/user/reference/cli/azldev_component_prepare-sources.md b/docs/user/reference/cli/azldev_component_prepare-sources.md index 9e22b5c5..a684197f 100644 --- a/docs/user/reference/cli/azldev_component_prepare-sources.md +++ b/docs/user/reference/cli/azldev_component_prepare-sources.md @@ -33,6 +33,7 @@ azldev component prepare-sources [flags] ``` -a, --all-components Include all components + -d, --allow-dirty include uncommitted lock file changes in synthetic history --allow-no-hashes compute missing hashes by downloading source files from their origin -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name diff --git a/docs/user/reference/cli/azldev_component_render.md b/docs/user/reference/cli/azldev_component_render.md index 1ded12de..6f79b089 100644 --- a/docs/user/reference/cli/azldev_component_render.md +++ b/docs/user/reference/cli/azldev_component_render.md @@ -54,6 +54,7 @@ azldev component render [flags] ``` -a, --all-components Include all components + -d, --allow-dirty include uncommitted lock changes in synthetic history --check-only render to a staging area and compare against the existing on-disk output without modifying the output directory. Exits 0 when nothing would change and 1 when any component would drift. With -a + --clean-stale, also fails on orphan rendered-spec directories. Intended for CI gates. --clean-stale prune rendered-spec directories that no longer correspond to a configured component (only with -a; requires -f with -o). Top-level non-component siblings are preserved. -p, --component stringArray Component name pattern diff --git a/internal/app/azldev/cmds/component/build.go b/internal/app/azldev/cmds/component/build.go index 63824f20..d12212f6 100644 --- a/internal/app/azldev/cmds/component/build.go +++ b/internal/app/azldev/cmds/component/build.go @@ -39,6 +39,8 @@ type ComponentBuildOptions struct { // MockConfigOpts is an optional set of key-value config options that will be passed through // to mock as --config-opts key=value arguments. MockConfigOpts map[string]string + + AllowDirty bool } // RPMResult encapsulates a single binary RPM produced by a component build, @@ -146,6 +148,8 @@ builds can consume.`, "Path to local repository to include during build and publish built RPMs to") cmd.Flags().StringToStringVar(&options.MockConfigOpts, "mock-config-opt", nil, "Pass a configuration option through to mock (key=value, can be specified multiple times)") + cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, + "include uncommitted lock file changes in synthetic history") // Mark flags as mutually exclusive. cmd.MarkFlagsMutuallyExclusive("srpm-only", "local-repo-with-publish") @@ -255,8 +259,11 @@ func BuildComponent( if !options.WithoutGitRepo { preparerOpts = append(preparerOpts, sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), - sources.WithDirtyDetection(), ) + + if options.AllowDirty { + preparerOpts = append(preparerOpts, sources.WithDirtyDetection()) + } } sourcePreparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, preparerOpts...) @@ -278,6 +285,13 @@ func BuildComponent( options.SourcePackageOnly, options.NoCheck, options.LocalRepoWithPublishPath, ) + + // Drain hints from the preparer — the builder calls PrepareSources + // internally, so hints are populated after buildComponentUsingBuilder returns. + for _, hint := range sourcePreparer.Hints() { + env.AddFixSuggestion(hint) + } + if err != nil { return results, err } diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index 1a40b2f1..83880445 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -24,6 +24,7 @@ type PrepareSourcesOptions struct { WithoutGitRepo bool Force bool AllowNoHashes bool + AllowDirty bool } func prepareOnAppInit(_ *azldev.App, sourceCmd *cobra.Command) { @@ -73,6 +74,8 @@ Only one component may be selected at a time.`, cmd.Flags().BoolVar(&options.Force, "force", false, "delete and recreate the output directory if it already exists") cmd.Flags().BoolVar(&options.AllowNoHashes, "allow-no-hashes", false, "compute missing hashes by downloading source files from their origin") + cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, + "include uncommitted lock file changes in synthetic history") return cmd } @@ -130,8 +133,11 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er if !options.WithoutGitRepo && !options.SkipOverlays { preparerOpts = append(preparerOpts, sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), - sources.WithDirtyDetection(), ) + + if options.AllowDirty { + preparerOpts = append(preparerOpts, sources.WithDirtyDetection()) + } } if options.AllowNoHashes { @@ -143,9 +149,14 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er return fmt.Errorf("failed to create source preparer:\n%w", err) } - err = preparer.PrepareSources(env, component, options.OutputDir, !options.SkipOverlays) - if err != nil { - return fmt.Errorf("failed to prepare sources for component %q:\n%w", component.GetName(), err) + prepErr := preparer.PrepareSources(env, component, options.OutputDir, !options.SkipOverlays) + + for _, hint := range preparer.Hints() { + env.AddFixSuggestion(hint) + } + + if prepErr != nil { + return fmt.Errorf("failed to prepare sources for component %q:\n%w", component.GetName(), prepErr) } return nil diff --git a/internal/app/azldev/cmds/component/render.go b/internal/app/azldev/cmds/component/render.go index 2f2cc54f..78504ce3 100644 --- a/internal/app/azldev/cmds/component/render.go +++ b/internal/app/azldev/cmds/component/render.go @@ -36,6 +36,7 @@ type RenderOptions struct { Force bool CleanStale bool CheckOnly bool + AllowDirty bool } func renderOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { @@ -43,7 +44,7 @@ func renderOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { } // NewRenderCmd constructs a [cobra.Command] for the "component render" CLI subcommand. -func NewRenderCmd() *cobra.Command { +func NewRenderCmd() *cobra.Command { //nolint:funlen // flag registrations push count slightly over var options RenderOptions var cmd *cobra.Command @@ -120,12 +121,20 @@ valid with -a.`, "and 1 when any component would drift. With -a + --clean-stale, also fails "+ "on orphan rendered-spec directories. Intended for CI gates.") + cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, + "include uncommitted lock changes in synthetic history") + // --check-only is a read-only diff against on-disk state; --fail-on-error // is the loud-failure-per-run knob. Combining them is semantically // muddled (CI would fail on stale failures even when on-disk markers // already record them) and forcing a choice keeps the contract crisp. cmd.MarkFlagsMutuallyExclusive("fail-on-error", "check-only") + // --check-only validates committed state for CI gates; --allow-dirty + // injects uncommitted state. Combining them would validate against + // working-tree state instead of committed state, defeating the purpose. + cmd.MarkFlagsMutuallyExclusive("check-only", "allow-dirty") + return cmd } @@ -204,7 +213,7 @@ func RenderComponents(env *azldev.Env, options *RenderOptions) ([]*RenderResult, results := make([]*RenderResult, len(componentList)) // ── Phase 1: Parallel source preparation ── - prepared := parallelPrepare(env, componentList, stagingDir, options.OutputDir, results) + prepared := parallelPrepare(env, componentList, stagingDir, options.OutputDir, results, options.AllowDirty) // ── Phase 2: Batch mock processing ── mockResultMap := batchMockProcess(env, mockProcessor, stagingDir, prepared) @@ -391,6 +400,7 @@ func parallelPrepare( stagingDir string, outputDir string, results []*RenderResult, + allowDirty bool, ) []*preparedComponent { progressEvent := env.StartEvent("Preparing component sources", "count", len(comps)) defer progressEvent.End() @@ -408,7 +418,8 @@ func parallelPrepare( func(_ context.Context, comp components.Component) prepResult { // workerEnv (captured) is the effective context for this call chain; // the parmap-supplied ctx is identical and unused here. - return prepareOneComponent(workerEnv, comp, stagingDir, outputDir) //nolint:contextcheck // env carries the ctx + //nolint:contextcheck // env carries the ctx + return prepareOneComponent(workerEnv, comp, stagingDir, outputDir, allowDirty) }, ) @@ -454,6 +465,7 @@ func prepareOneComponent( comp components.Component, stagingDir string, outputDir string, + allowDirty bool, ) prepResult { componentName := comp.GetName() @@ -468,7 +480,7 @@ func prepareOneComponent( }} } - prep, err := prepareComponentSources(env, comp, stagingDir) + prep, err := prepareComponentSources(env, comp, stagingDir, allowDirty) if err != nil { slog.Error("Failed to prepare component sources", "component", componentName, "error", err) @@ -493,6 +505,7 @@ func prepareComponentSources( env *azldev.Env, comp components.Component, stagingDir string, + allowDirty bool, ) (*preparedComponent, error) { componentName := comp.GetName() @@ -525,16 +538,25 @@ func prepareComponentSources( // sidecar files are needed for rendering. preparerOpts := []sources.PreparerOption{ sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), - sources.WithDirtyDetection(), sources.WithSkipLookaside(), } + if allowDirty { + preparerOpts = append(preparerOpts, sources.WithDirtyDetection()) + } + preparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, preparerOpts...) if err != nil { return nil, fmt.Errorf("creating source preparer for %#q:\n%w", componentName, err) } - if prepErr := preparer.PrepareSources(env, comp, componentDir, true /*applyOverlays*/); prepErr != nil { + prepErr := preparer.PrepareSources(env, comp, componentDir, true /*applyOverlays*/) + + for _, hint := range preparer.Hints() { + env.AddFixSuggestion(hint) + } + + if prepErr != nil { return nil, fmt.Errorf("preparing sources for %#q:\n%w", componentName, prepErr) } diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 617dcbd8..a21a5fdc 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -54,6 +54,11 @@ type SourcePreparer interface { // The component's sources are fetched once into a subdirectory of baseDir, then copied to a second // subdirectory where overlays are applied in-place. The diff between the two subdirectories is returned. DiffSources(ctx context.Context, component components.Component, baseDir string) (*dirdiff.DiffResult, error) + + // Hints returns user-facing suggestions collected during the most recent + // [PrepareSources] call (e.g., "use --allow-dirty"). Callers should pass + // these to [azldev.Env.AddFixSuggestion] for display. + Hints() []string } // PreparerOption is a functional option for configuring a [SourcePreparer]. @@ -83,15 +88,11 @@ func WithGitRepo( } } -// WithDirtyDetection returns a [PreparerOption] that enables uncommitted-change -// detection during synthetic history generation. When set, the current input -// fingerprint is compared against the committed lock file; if they differ, a -// "dirty" synthetic commit is appended to represent the uncommitted changes. -// Without this option, only committed fingerprint changes produce synthetic commits. -// -// This should be enabled for commands that operate on the working tree state -// (build, render, prepare-sources) and left disabled for commands that should -// only reflect committed state. +// WithDirtyDetection returns a [PreparerOption] that allows uncommitted lock +// state to be included in synthetic history generation. When set, uncommitted +// lock files and fingerprint mismatches produce dirty synthetic commits. +// Without this option, uncommitted changes cause an error with a hint to use +// '--allow-dirty'. Gated by the '--allow-dirty' ('-d') CLI flag. func WithDirtyDetection() PreparerOption { return func(p *sourcePreparerImpl) { p.dirtyDetection = true @@ -160,6 +161,10 @@ type sourcePreparerImpl struct { // synthetic history generation. Set via [WithDirtyDetection]. dirtyDetection bool + // hints collects user-facing suggestions generated during [PrepareSources]. + // Callers can retrieve them via [Hints] after PrepareSources returns. + hints []string + // releaseVer is the per-component resolved distro release version, not the // project default. Set via [WithGitRepo]. releaseVer string @@ -211,10 +216,18 @@ func NewPreparer( return impl, nil } +// Hints returns user-facing suggestions collected during PrepareSources. +func (p *sourcePreparerImpl) Hints() []string { + return p.hints +} + // PrepareSources implements the [SourcePreparer] interface. func (p *sourcePreparerImpl) PrepareSources( ctx context.Context, component components.Component, outputDir string, applyOverlays bool, ) error { + // Reset hints from any prior call (preparer may be reused). + p.hints = p.hints[:0] + // Use the source manager to fetch source files (archives, patches, etc.) // Skip this step when skipLookaside is set — source tarballs are not needed // for rendering and are the most expensive download. @@ -407,23 +420,23 @@ func (p *sourcePreparerImpl) trySyntheticHistory( config := component.GetConfig() componentName := component.GetName() - // Compute the current fingerprint for uncommitted-change detection. - // Only computed when dirty detection is enabled (e.g., build, render). - // An empty fingerprint skips dirty detection in buildSyntheticCommits. - var currentFingerprint string - - if p.dirtyDetection { - var fpErr error - - currentFingerprint, fpErr = computeCurrentFingerprint(p.fs, config, p.releaseVer) - if fpErr != nil { + // Always compute the current fingerprint — it's local I/O only (no + // network). When dirty detection is enabled (--allow-dirty), mismatches + // produce synthetic dirty entries. When disabled, mismatches generate + // hints suggesting --allow-dirty. + currentFingerprint, fpErr := computeCurrentFingerprint(p.fs, config, p.releaseVer) + if fpErr != nil { + if p.dirtyDetection { return fmt.Errorf("dirty detection failed for component %#q:\n%w", componentName, fpErr) } + + slog.Debug("Cannot compute current fingerprint for dirty detection", + "component", componentName, "error", fpErr) } changes, importCommit, err := buildSyntheticCommits( ctx, p.cmdFactory, config, componentName, p.lockReader.LockDir(), - currentFingerprint, + currentFingerprint, p.dirtyDetection, &p.hints, ) if err != nil { return fmt.Errorf("failed to build synthetic commits:\n%w", err) diff --git a/internal/app/azldev/core/sources/synthistory.go b/internal/app/azldev/core/sources/synthistory.go index 9d66f152..c345ddd9 100644 --- a/internal/app/azldev/core/sources/synthistory.go +++ b/internal/app/azldev/core/sources/synthistory.go @@ -451,9 +451,12 @@ func updateHead(repo *gogit.Repository, commitHash plumbing.Hash) error { // current fingerprint matches the committed one. // // When currentFingerprint is non-empty it is compared against the fingerprint -// stored in the HEAD commit's lock file. If they differ a "dirty" entry is -// appended so that uncommitted config/overlay changes are represented in the -// synthetic history. +// stored in the HEAD commit's lock file to detect uncommitted changes. +// +// When allowDirty is true, uncommitted lock state is used as a fallback when +// no committed lock exists at HEAD, and fingerprint mismatches produce dirty +// synthetic entries. When false, only committed state is considered and any +// detected dirty state is appended to hints for the caller to surface. // // The lockDir is the absolute path to the lock file directory. It is converted // to a repo-relative path internally once the git repository root is known. @@ -464,6 +467,8 @@ func buildSyntheticCommits( componentName string, lockDir string, currentFingerprint string, + allowDirty bool, + hints *[]string, ) (changes []FingerprintChange, importCommit string, err error) { projectRepo, projectRepoDir, err := openProjectRepo(config, componentName) if err != nil { @@ -487,14 +492,18 @@ func buildSyntheticCommits( } // Read the lock file at HEAD. If the file is missing (not yet committed), - // synthetic history is skipped. + // fall back to the on-disk lock data from the resolver. This handles the + // first-time workflow: 'component update' creates the lock on disk, then + // 'component render' runs before the lock is committed. Without this + // fallback, the component would get no synthetic history (autorelease=1) + // even though the on-disk lock has all the data needed. headLock, err := readLockFileAtHEAD(projectRepo, lockFileRelPath) if err != nil { return nil, "", err } if headLock == nil { - return nil, "", nil + return handleMissingHeadLock(config, componentName, lockFileRelPath, allowDirty, hints) } importCommit = headLock.ImportCommit @@ -518,11 +527,15 @@ func buildSyntheticCommits( // Check for uncommitted ("dirty") changes by comparing the caller-provided // current fingerprint against the fingerprint stored in the HEAD lock file. - if dirty := BuildDirtyChange(currentFingerprint, headLock, config.EffectiveUpstreamCommit()); dirty != nil { - slog.Info("Current fingerprint differs from HEAD lock file; adding dirty entry", - "lockFile", lockFileRelPath) + dirtyEntry, dirtyErr := handleDirtyState( + currentFingerprint, headLock, config, componentName, lockFileRelPath, allowDirty, hints, + ) + if dirtyErr != nil { + return nil, "", dirtyErr + } - fpChanges = append(fpChanges, *dirty) + if dirtyEntry != nil { + fpChanges = append(fpChanges, *dirtyEntry) } if len(fpChanges) == 0 { @@ -535,6 +548,37 @@ func buildSyntheticCommits( return fpChanges, importCommit, nil } +// handleDirtyState checks for uncommitted changes and either returns a dirty +// [FingerprintChange] entry (when allowDirty is true) or an error with a hint +// (when false). Returns (nil, nil) when no dirty state is detected. +func handleDirtyState( + currentFingerprint string, + headLock *lockfile.ComponentLock, + config *projectconfig.ComponentConfig, + componentName, lockFileRelPath string, + allowDirty bool, + hints *[]string, +) (*FingerprintChange, error) { + dirty := BuildDirtyChange(currentFingerprint, headLock, config.EffectiveUpstreamCommit()) + if dirty == nil { + return nil, nil //nolint:nilnil // nil,nil signals "not dirty" to caller. + } + + if allowDirty { + slog.Info("Current fingerprint differs from HEAD lock file; adding dirty entry", + "lockFile", lockFileRelPath) + + return dirty, nil + } + + *hints = append(*hints, + "use '--allow-dirty' ('-d') to proceed with uncommitted changes") + + return nil, fmt.Errorf( + "component %#q has uncommitted changes (lock fingerprint differs from HEAD)", + componentName) +} + // BuildDirtyChange returns a [FingerprintChange] representing uncommitted // config/overlay changes. Returns nil when currentFingerprint is empty or // matches the HEAD lock file's fingerprint. @@ -647,6 +691,61 @@ func readLockFileAtHEAD( return nil, nil //nolint:nilnil // nil,nil signals "not found, skip" to caller. } +// handleMissingHeadLock handles the case where no committed lock file exists +// at HEAD. When allowDirty is true and on-disk lock data is available, creates +// an initial import entry. When false and dirty state is detected, returns an +// error with a hint. Returns (nil, "", nil) when there's nothing to do. +func handleMissingHeadLock( + config *projectconfig.ComponentConfig, + componentName, lockFileRelPath string, + allowDirty bool, + hints *[]string, +) ([]FingerprintChange, string, error) { + if allowDirty { + return initialImportFromDisk(config, lockFileRelPath) + } + + // Uncommitted lock exists but --allow-dirty not set — error. + if config.Locked != nil && config.Locked.InputFingerprint != "" { + *hints = append(*hints, + "use '--allow-dirty' ('-d') to proceed with uncommitted lock files") + + return nil, "", fmt.Errorf( + "component %#q has an uncommitted lock file", + componentName) + } + + return nil, "", nil +} + +// initialImportFromDisk creates a single synthetic "initial import" entry from +// the on-disk lock data when no committed lock exists at HEAD. This handles the +// first-time workflow: 'component update' creates the lock on disk, then +// 'component render' runs before the lock is committed. +// Returns (nil, "", nil) when the on-disk lock has no fingerprint. +func initialImportFromDisk( + config *projectconfig.ComponentConfig, + lockFileRelPath string, +) ([]FingerprintChange, string, error) { + if config.Locked == nil || config.Locked.InputFingerprint == "" { + return nil, "", nil + } + + slog.Warn("No committed lock; using on-disk lock for initial synthetic history", + "lockFile", lockFileRelPath) + + return []FingerprintChange{{ + CommitMetadata: CommitMetadata{ + Hash: "initial", + Author: "azldev", + AuthorEmail: "azldev@local", + Timestamp: time.Now().Unix(), + Message: "Initial component import (uncommitted)", + }, + UpstreamCommit: config.EffectiveUpstreamCommit(), + }}, config.Locked.ImportCommit, nil +} + // collectUpstreamCommits returns commits in the repository in chronological // order (oldest first), bounded by importCommit (inclusive start) and // upstreamCommit (inclusive end). Only first-parent links are followed so that diff --git a/internal/app/azldev/core/sources/synthistory_internal_test.go b/internal/app/azldev/core/sources/synthistory_internal_test.go new file mode 100644 index 00000000..4a2adf39 --- /dev/null +++ b/internal/app/azldev/core/sources/synthistory_internal_test.go @@ -0,0 +1,256 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "testing" + "time" + + memfs "github.com/go-git/go-billy/v5/memfs" + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/storage/memory" + "github.com/microsoft/azure-linux-dev-tools/internal/lockfile" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// initRepoWithLockFile creates an in-memory git repo with a committed lock file. +// Returns the repo and its worktree root. +func initRepoWithLockFile(t *testing.T, lockRelPath, lockContent string) *gogit.Repository { + t.Helper() + + memFS := memfs.New() + storer := memory.NewStorage() + + repo, err := gogit.Init(storer, memFS) + require.NoError(t, err) + + // Write the lock file. + file, err := memFS.Create(lockRelPath) + require.NoError(t, err) + + _, err = file.Write([]byte(lockContent)) + require.NoError(t, err) + require.NoError(t, file.Close()) + + // Stage and commit. + worktree, err := repo.Worktree() + require.NoError(t, err) + + _, err = worktree.Add(lockRelPath) + require.NoError(t, err) + + _, err = worktree.Commit("Add lock file", &gogit.CommitOptions{ + Author: &object.Signature{ + Name: "test", + Email: "test@test.com", + When: time.Unix(1000000, 0), + }, + }) + require.NoError(t, err) + + return repo +} + +func TestReadLockFileAtHEAD_Found(t *testing.T) { + lockContent := `version = 1 +upstream-commit = "abc123" +input-fingerprint = "sha256:test-fp" +` + repo := initRepoWithLockFile(t, "locks/curl.lock", lockContent) + + lock, err := readLockFileAtHEAD(repo, "locks/curl.lock") + require.NoError(t, err) + require.NotNil(t, lock) + assert.Equal(t, "abc123", lock.UpstreamCommit) + assert.Equal(t, "sha256:test-fp", lock.InputFingerprint) +} + +func TestReadLockFileAtHEAD_Missing(t *testing.T) { + // Create repo with a different file — lock is missing. + memFS := memfs.New() + storer := memory.NewStorage() + + repo, err := gogit.Init(storer, memFS) + require.NoError(t, err) + + file, err := memFS.Create("README.md") + require.NoError(t, err) + + _, err = file.Write([]byte("hello")) + require.NoError(t, err) + require.NoError(t, file.Close()) + + worktree, err := repo.Worktree() + require.NoError(t, err) + + _, err = worktree.Add("README.md") + require.NoError(t, err) + + _, err = worktree.Commit("Initial", &gogit.CommitOptions{ + Author: &object.Signature{Name: "test", Email: "t@t", When: time.Unix(1, 0)}, + }) + require.NoError(t, err) + + lock, err := readLockFileAtHEAD(repo, "locks/curl.lock") + require.NoError(t, err) + assert.Nil(t, lock, "missing lock should return nil without error") +} + +func TestInitialImportFromDisk_WithLockedData(t *testing.T) { + config := projectConfigWithLock("test-comp", &lockfile.ComponentLock{ + UpstreamCommit: "abc123", + ImportCommit: "import456", + InputFingerprint: "sha256:test-fp", + }) + + changes, importCommit, err := initialImportFromDisk( + &config, + "locks/test-comp.lock", + ) + require.NoError(t, err) + require.Len(t, changes, 1) + assert.Equal(t, "initial", changes[0].Hash) + assert.Equal(t, "abc123", changes[0].UpstreamCommit) + assert.Equal(t, "import456", importCommit) +} + +func TestInitialImportFromDisk_NoLockedData(t *testing.T) { + config := projectConfigWithLock("test-comp", nil) + + changes, importCommit, err := initialImportFromDisk( + &config, + "locks/test-comp.lock", + ) + require.NoError(t, err) + assert.Nil(t, changes) + assert.Empty(t, importCommit) +} + +// projectConfigWithLock creates a minimal ComponentConfig with optional lock data. +func projectConfigWithLock(name string, lock *lockfile.ComponentLock) projectconfig.ComponentConfig { + config := projectconfig.ComponentConfig{Name: name} + if lock != nil { + config.Locked = &projectconfig.ComponentLockData{ + UpstreamCommit: lock.UpstreamCommit, + ImportCommit: lock.ImportCommit, + InputFingerprint: lock.InputFingerprint, + } + } + + return config +} + +// --- handleMissingHeadLock tests --- + +func TestHandleMissingHeadLock_AllowDirty_WithLock(t *testing.T) { + config := projectConfigWithLock("curl", &lockfile.ComponentLock{ + UpstreamCommit: "abc123", + ImportCommit: "import456", + InputFingerprint: "sha256:test-fp", + }) + + var hints []string + + changes, importCommit, err := handleMissingHeadLock( + &config, "curl", "locks/curl.lock", true, &hints, + ) + require.NoError(t, err) + require.Len(t, changes, 1) + assert.Equal(t, "initial", changes[0].Hash) + assert.Equal(t, "abc123", changes[0].UpstreamCommit) + assert.Equal(t, "import456", importCommit) + assert.Empty(t, hints, "no hints when allow-dirty is true") +} + +func TestHandleMissingHeadLock_NoDirty_WithLock_Errors(t *testing.T) { + config := projectConfigWithLock("curl", &lockfile.ComponentLock{ + UpstreamCommit: "abc123", + InputFingerprint: "sha256:test-fp", + }) + + var hints []string + + _, _, err := handleMissingHeadLock( + &config, "curl", "locks/curl.lock", false, &hints, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "uncommitted lock file") + require.Len(t, hints, 1) + assert.Contains(t, hints[0], "--allow-dirty") +} + +func TestHandleMissingHeadLock_NoDirty_NoLock_Skips(t *testing.T) { + config := projectConfigWithLock("curl", nil) + + var hints []string + + changes, importCommit, err := handleMissingHeadLock( + &config, "curl", "locks/curl.lock", false, &hints, + ) + require.NoError(t, err) + assert.Nil(t, changes) + assert.Empty(t, importCommit) + assert.Empty(t, hints, "no hints when no lock data exists") +} + +// --- handleDirtyState tests --- + +func TestHandleDirtyState_AllowDirty_ReturnsDirtyEntry(t *testing.T) { + headLock := &lockfile.ComponentLock{ + UpstreamCommit: "committed-abc", + InputFingerprint: "sha256:old-fp", + } + config := projectConfigWithLock("curl", &lockfile.ComponentLock{ + UpstreamCommit: "new-commit", + }) + + var hints []string + + entry, err := handleDirtyState( + "sha256:new-fp", headLock, &config, "curl", "locks/curl.lock", true, &hints, + ) + require.NoError(t, err) + require.NotNil(t, entry) + assert.Equal(t, "dirty", entry.Hash) + assert.Empty(t, hints, "no hints when allow-dirty is true") +} + +func TestHandleDirtyState_NoDirty_Errors(t *testing.T) { + headLock := &lockfile.ComponentLock{ + UpstreamCommit: "committed-abc", + InputFingerprint: "sha256:old-fp", + } + config := projectConfigWithLock("curl", nil) + + var hints []string + + entry, err := handleDirtyState( + "sha256:new-fp", headLock, &config, "curl", "locks/curl.lock", false, &hints, + ) + require.Error(t, err) + assert.Nil(t, entry) + assert.Contains(t, err.Error(), "uncommitted changes") + require.Len(t, hints, 1) + assert.Contains(t, hints[0], "--allow-dirty") +} + +func TestHandleDirtyState_NoMismatch_NoError(t *testing.T) { + headLock := &lockfile.ComponentLock{ + UpstreamCommit: "committed-abc", + InputFingerprint: "sha256:same-fp", + } + config := projectConfigWithLock("curl", nil) + + var hints []string + + entry, err := handleDirtyState( + "sha256:same-fp", headLock, &config, "curl", "locks/curl.lock", false, &hints, + ) + require.NoError(t, err) + assert.Nil(t, entry, "matching fingerprints should not produce dirty entry") + assert.Empty(t, hints) +} diff --git a/scenario/component_build_test.go b/scenario/component_build_test.go index 59466ea6..153580e9 100644 --- a/scenario/component_build_test.go +++ b/scenario/component_build_test.go @@ -128,9 +128,11 @@ func TestBuildingUpstreamComponent(t *testing.T) { // Run the build with test default configs copied into the container. // component update populates lock files first (required by lock validation). + // Lock files must be committed before build so synthetic history can read them. results := buildtest.NewBuildTest(project, testComponentName, projecttest.WithTestDefaultConfigs(), projecttest.WithPreCommand("component", "update", "-a"), + projecttest.WithPreScript("(cd project && git add -A && git commit -m 'update locks')"), ).Run(t) // Make sure we got 1 SRPM. diff --git a/scenario/internal/projecttest/projecttest.go b/scenario/internal/projecttest/projecttest.go index c61bba51..a27e7b8f 100644 --- a/scenario/internal/projecttest/projecttest.go +++ b/scenario/internal/projecttest/projecttest.go @@ -50,6 +50,7 @@ type ProjectTest struct { project TestProject commandArgs []string preCommands [][]string + preScripts []string useTestDefaultConfigs bool } @@ -73,6 +74,14 @@ func WithPreCommand(args ...string) ProjectTestOption { } } +// WithPreScript adds an arbitrary shell script line to run before the main test +// command. Unlike [WithPreCommand], the line is not prefixed with anything. +func WithPreScript(script string) ProjectTestOption { + return func(p *ProjectTest) { + p.preScripts = append(p.preScripts, script) + } +} + // NewProjectTest creates a new [ProjectTest] with the specified project and command arguments. func NewProjectTest(project TestProject, commandArgs []string, options ...ProjectTestOption) *ProjectTest { params := &ProjectTest{ @@ -109,6 +118,10 @@ func (p *ProjectTest) RunInContainer(t *testing.T) *ProjectTestResults { preCommandLines += "\nazldev -C project -v " + shellquote.Join(pre...) } + for _, script := range p.preScripts { + preCommandLines += "\n" + script + } + testScript := fmt.Sprintf(` set -ex From 19c7aa00c2c6ac3a31a2fbdb529704671461086b Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 13 May 2026 11:33:48 -0700 Subject: [PATCH 2/5] fixup! feat(render): add --allow-dirty flag for uncommitted lock state --- internal/app/azldev/cmds/component/build.go | 2 +- internal/app/azldev/cmds/component/preparesources.go | 2 +- internal/app/azldev/cmds/component/render.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/app/azldev/cmds/component/build.go b/internal/app/azldev/cmds/component/build.go index d12212f6..e0d1cfdf 100644 --- a/internal/app/azldev/cmds/component/build.go +++ b/internal/app/azldev/cmds/component/build.go @@ -149,7 +149,7 @@ builds can consume.`, cmd.Flags().StringToStringVar(&options.MockConfigOpts, "mock-config-opt", nil, "Pass a configuration option through to mock (key=value, can be specified multiple times)") cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, - "include uncommitted lock file changes in synthetic history") + "include uncommitted changes in synthetic history") // Mark flags as mutually exclusive. cmd.MarkFlagsMutuallyExclusive("srpm-only", "local-repo-with-publish") diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index 83880445..b3de9778 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -75,7 +75,7 @@ Only one component may be selected at a time.`, cmd.Flags().BoolVar(&options.AllowNoHashes, "allow-no-hashes", false, "compute missing hashes by downloading source files from their origin") cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, - "include uncommitted lock file changes in synthetic history") + "include uncommitted changes in synthetic history") return cmd } diff --git a/internal/app/azldev/cmds/component/render.go b/internal/app/azldev/cmds/component/render.go index 78504ce3..eb4f7504 100644 --- a/internal/app/azldev/cmds/component/render.go +++ b/internal/app/azldev/cmds/component/render.go @@ -122,7 +122,7 @@ valid with -a.`, "on orphan rendered-spec directories. Intended for CI gates.") cmd.Flags().BoolVarP(&options.AllowDirty, "allow-dirty", "d", false, - "include uncommitted lock changes in synthetic history") + "include uncommitted changes in synthetic history") // --check-only is a read-only diff against on-disk state; --fail-on-error // is the loud-failure-per-run knob. Combining them is semantically From 5f5bff808188aa755d4a3ff176d5ca442d91d052 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 13 May 2026 11:50:07 -0700 Subject: [PATCH 3/5] fixup! feat(render): add --allow-dirty flag for uncommitted lock state --- internal/app/azldev/cmds/component/preparesources.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index b3de9778..3a6f3dbe 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -80,6 +80,7 @@ Only one component may be selected at a time.`, return cmd } +//nolint:cyclop // slightly over due to flag/option setup func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) error { var comps *components.ComponentSet From f3abf10ee81bc4dc6b93e08a88c54a2b60ff163f Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 13 May 2026 14:56:24 -0700 Subject: [PATCH 4/5] fixup! feat(render): add --allow-dirty flag for uncommitted lock state --- docs/user/reference/cli/azldev_component_build.md | 2 +- docs/user/reference/cli/azldev_component_prepare-sources.md | 2 +- docs/user/reference/cli/azldev_component_render.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user/reference/cli/azldev_component_build.md b/docs/user/reference/cli/azldev_component_build.md index 00a3586d..8591863a 100644 --- a/docs/user/reference/cli/azldev_component_build.md +++ b/docs/user/reference/cli/azldev_component_build.md @@ -48,7 +48,7 @@ azldev component build [flags] ``` -a, --all-components Include all components - -d, --allow-dirty include uncommitted lock file changes in synthetic history + -d, --allow-dirty include uncommitted changes in synthetic history -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -k, --continue-on-error Continue building when some components fail diff --git a/docs/user/reference/cli/azldev_component_prepare-sources.md b/docs/user/reference/cli/azldev_component_prepare-sources.md index a684197f..8a3e5d4c 100644 --- a/docs/user/reference/cli/azldev_component_prepare-sources.md +++ b/docs/user/reference/cli/azldev_component_prepare-sources.md @@ -33,7 +33,7 @@ azldev component prepare-sources [flags] ``` -a, --all-components Include all components - -d, --allow-dirty include uncommitted lock file changes in synthetic history + -d, --allow-dirty include uncommitted changes in synthetic history --allow-no-hashes compute missing hashes by downloading source files from their origin -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name diff --git a/docs/user/reference/cli/azldev_component_render.md b/docs/user/reference/cli/azldev_component_render.md index 6f79b089..0f0f8e47 100644 --- a/docs/user/reference/cli/azldev_component_render.md +++ b/docs/user/reference/cli/azldev_component_render.md @@ -54,7 +54,7 @@ azldev component render [flags] ``` -a, --all-components Include all components - -d, --allow-dirty include uncommitted lock changes in synthetic history + -d, --allow-dirty include uncommitted changes in synthetic history --check-only render to a staging area and compare against the existing on-disk output without modifying the output directory. Exits 0 when nothing would change and 1 when any component would drift. With -a + --clean-stale, also fails on orphan rendered-spec directories. Intended for CI gates. --clean-stale prune rendered-spec directories that no longer correspond to a configured component (only with -a; requires -f with -o). Top-level non-component siblings are preserved. -p, --component stringArray Component name pattern From 54b8bf1d7870e1ad1b9b43160527c995d173db63 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 13 May 2026 15:01:44 -0700 Subject: [PATCH 5/5] fixup! feat(render): add --allow-dirty flag for uncommitted lock state --- internal/app/azldev/core/sources/sourceprep.go | 2 +- internal/app/azldev/core/sources/synthistory.go | 16 ++++++++++------ .../core/sources/synthistory_internal_test.go | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index a21a5fdc..e18ed752 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -218,7 +218,7 @@ func NewPreparer( // Hints returns user-facing suggestions collected during PrepareSources. func (p *sourcePreparerImpl) Hints() []string { - return p.hints + return slices.Clone(p.hints) } // PrepareSources implements the [SourcePreparer] interface. diff --git a/internal/app/azldev/core/sources/synthistory.go b/internal/app/azldev/core/sources/synthistory.go index c345ddd9..52956b2b 100644 --- a/internal/app/azldev/core/sources/synthistory.go +++ b/internal/app/azldev/core/sources/synthistory.go @@ -455,8 +455,8 @@ func updateHead(repo *gogit.Repository, commitHash plumbing.Hash) error { // // When allowDirty is true, uncommitted lock state is used as a fallback when // no committed lock exists at HEAD, and fingerprint mismatches produce dirty -// synthetic entries. When false, only committed state is considered and any -// detected dirty state is appended to hints for the caller to surface. +// synthetic entries. When false, dirty state causes a hard error with hints +// suggesting '--allow-dirty' for the caller to surface. // // The lockDir is the absolute path to the lock file directory. It is converted // to a repo-relative path internally once the git repository root is known. @@ -571,8 +571,10 @@ func handleDirtyState( return dirty, nil } - *hints = append(*hints, - "use '--allow-dirty' ('-d') to proceed with uncommitted changes") + if hints != nil { + *hints = append(*hints, + "use '--allow-dirty' ('-d') to proceed with uncommitted changes") + } return nil, fmt.Errorf( "component %#q has uncommitted changes (lock fingerprint differs from HEAD)", @@ -707,8 +709,10 @@ func handleMissingHeadLock( // Uncommitted lock exists but --allow-dirty not set — error. if config.Locked != nil && config.Locked.InputFingerprint != "" { - *hints = append(*hints, - "use '--allow-dirty' ('-d') to proceed with uncommitted lock files") + if hints != nil { + *hints = append(*hints, + "use '--allow-dirty' ('-d') to proceed with uncommitted lock files") + } return nil, "", fmt.Errorf( "component %#q has an uncommitted lock file", diff --git a/internal/app/azldev/core/sources/synthistory_internal_test.go b/internal/app/azldev/core/sources/synthistory_internal_test.go index 4a2adf39..dda566e9 100644 --- a/internal/app/azldev/core/sources/synthistory_internal_test.go +++ b/internal/app/azldev/core/sources/synthistory_internal_test.go @@ -18,7 +18,7 @@ import ( ) // initRepoWithLockFile creates an in-memory git repo with a committed lock file. -// Returns the repo and its worktree root. +// Returns the repo. func initRepoWithLockFile(t *testing.T, lockRelPath, lockContent string) *gogit.Repository { t.Helper()