From f989e5bf77ae204c85537de44c98b88e9b4fe1a4 Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 3 Jun 2026 17:12:46 +0000 Subject: [PATCH 1/5] feat: add tarball overlays for source archive modification Add three new overlay types (tarball-file-remove, tarball-search-replace, tarball-patch) that modify files inside source tarballs during source preparation. Operations are performed in pure Go on the host. Includes: - internal/utils/tarball: reusable deterministic tar extract/repack library - Overlay type registration, validation, and fingerprinting - Source prep integration with sources file hash rehashing - User documentation and TOML examples --- docs/user/reference/config/overlays.md | 72 +++- .../azldev/cmds/component/preparesources.go | 28 +- .../app/azldev/core/sources/sourceprep.go | 154 ++++++-- .../azldev/core/sources/sourceprep_test.go | 2 +- .../azldev/core/sources/tarballoverlays.go | 358 ++++++++++++++++++ .../sources/tarballoverlays_internal_test.go | 137 +++++++ internal/projectconfig/overlay.go | 64 +++- internal/projectconfig/overlay_test.go | 156 ++++++++ 8 files changed, 921 insertions(+), 50 deletions(-) create mode 100644 internal/app/azldev/core/sources/tarballoverlays.go create mode 100644 internal/app/azldev/core/sources/tarballoverlays_internal_test.go diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 55061118..626b4957 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -47,21 +47,34 @@ successfully makes a replacement to at least one matching file. | `file-remove` | Removes a file | `file` | Glob pattern for files to remove | | `file-rename` | Renames a file within the same directory | `file`, `replacement` | Name of file to rename | +### Tarball Overlays + +These overlays modify files **inside** source tarballs. The tarball is extracted into a temporary directory, modifications are applied, and the tarball is repacked with the same compression format. Extraction and repacking are handled natively; patch application requires the `patch` command on the host. + +> **Note:** Tarball overlays are applied before spec and file overlays, so subsequent overlays see the modified tarball. The `tarball-patch` overlay type requires the `patch` command to be installed on the host. + +| Type | Description | Required Fields | +|------|-------------|-----------------| +| `tarball-file-remove` | Removes file(s) matching a glob pattern from inside a tarball | `tarball`, `file` | +| `tarball-search-replace` | Regex-based search and replace on file(s) inside a tarball | `tarball`, `file`, `regex` | +| `tarball-patch` | Applies a unified diff patch to the extracted tarball contents | `tarball`, `source` | + ## Field Reference | Field | TOML Key | Description | Used By | |-------|----------|-------------|---------| | Type | `type` | **Required.** The overlay type to apply | All overlays | | Description | `description` | Human-readable explanation documenting the need for the change; helps identify overlays in error messages | All (optional) | +| Tarball | `tarball` | The source tarball filename to modify (must be a basename, not a path) | `tarball-file-remove`, `tarball-search-replace`, `tarball-patch` | | Tag | `tag` | The spec tag name (e.g., `BuildRequires`, `Requires`, `Version`) | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` | -| Value | `value` | The tag value to set, or value to match for removal | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching) | +| Value | `value` | The tag value to set, or value to match for removal. For `tarball-patch`, sets the patch strip level (default: `1`, equivalent to `patch -p1`). | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching), `tarball-patch` (optional) | | Section | `section` | The spec section to target (e.g., `%build`, `%install`, `%files`, `%description`) | `spec-prepend-lines`, `spec-append-lines`, `spec-search-replace` (optional), `spec-remove-section` | | Package | `package` | The sub-package name for multi-package specs; omit to target the main package | All spec overlays (optional, except `spec-remove-subpackage` which **requires** it) | -| Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace` | -| Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename` | +| Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace`, `tarball-search-replace` | +| Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename`, `tarball-search-replace` | | Lines | `lines` | Array of text lines to insert | `spec-prepend-lines`, `spec-append-lines`, `file-prepend-lines` | -| File | `file` | The name of the non-spec file to modify or add | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | -| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add` | +| File | `file` | The name of the non-spec file to modify or add | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove`, `tarball-file-remove`, `tarball-search-replace` | +| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add`, `tarball-patch` | > **Note:** For `file-rename`, the `replacement` field is a **filename only** (not a path). The file is renamed within its current directory. @@ -274,6 +287,55 @@ description = "Remove CVE patches that are now upstream" > `PatchN` tags. Macro-based tag numbering (e.g., `Patch%{n}`) is not expanded and may > conflict with auto-assigned numbers. +### Removing a File from a Tarball + +The `tarball-file-remove` overlay deletes files matching a glob pattern from inside a source +tarball. The tarball is extracted, matching files are removed, and the tarball is repacked. + +```toml +[[components.mypackage.overlays]] +type = "tarball-file-remove" +tarball = "mypackage-1.0.tar.gz" +file = "vendor/**" +description = "Remove bundled vendor directory" +``` + +### Search and Replace Inside a Tarball + +```toml +[[components.mypackage.overlays]] +type = "tarball-search-replace" +tarball = "mypackage-1.0.tar.xz" +file = "configure.ac" +regex = "AC_CHECK_LIB\\(old_lib" +replacement = "AC_CHECK_LIB(new_lib" +description = "Update library reference in configure script" +``` + +### Applying a Patch to Tarball Contents + +The `tarball-patch` overlay applies a unified diff patch to the extracted tarball contents. +By default, it uses `patch -p1`. Use the `value` field to change the strip level. + +```toml +[[components.mypackage.overlays]] +type = "tarball-patch" +tarball = "mypackage-1.0.tar.gz" +source = "patches/fix-build.patch" +description = "Fix build issue in upstream source" +``` + +With a custom strip level: + +```toml +[[components.mypackage.overlays]] +type = "tarball-patch" +tarball = "mypackage-1.0.tar.gz" +source = "patches/fix-build.patch" +value = "0" +description = "Apply patch with -p0 strip level" +``` + ### Removing a Section The `spec-remove-section` overlay removes an entire section from the spec, including its diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index 2f0ffcaa..3dc7d687 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -138,13 +138,7 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er ) } - if options.AllowNoHashes { - preparerOpts = append(preparerOpts, sources.WithAllowNoHashes()) - } - - if options.SkipSources { - preparerOpts = append(preparerOpts, sources.WithSkipLookaside()) - } + preparerOpts = appendPrepareSourcesOptions(env, preparerOpts, options, distro) preparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, preparerOpts...) if err != nil { @@ -194,3 +188,23 @@ func CheckOutputDir(env *azldev.Env, options *PrepareSourcesOptions) error { "use --force to delete and recreate it", options.OutputDir) } + +// appendPrepareSourcesOptions appends conditional preparer options that control +// hashing and lookaside behavior. Extracted from +// [PrepareComponentSources] to keep cyclomatic complexity within limits. +func appendPrepareSourcesOptions( + _ *azldev.Env, + opts []sources.PreparerOption, + options *PrepareSourcesOptions, + _ sourceproviders.ResolvedDistro, +) []sources.PreparerOption { + if options.AllowNoHashes { + opts = append(opts, sources.WithAllowNoHashes()) + } + + if options.SkipSources { + opts = append(opts, sources.WithSkipLookaside()) + } + + return opts +} diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 617dcbd8..62649e22 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -246,8 +246,7 @@ func (p *sourcePreparerImpl) PrepareSources( } if applyOverlays { - err := p.applyOverlaysToSources(ctx, component, outputDir) - if err != nil { + if err := p.applyOverlaysToSources(ctx, component, outputDir); err != nil { return err } @@ -276,9 +275,6 @@ func (p *sourcePreparerImpl) PrepareSources( func (p *sourcePreparerImpl) applyOverlaysToSources( ctx context.Context, component components.Component, outputDir string, ) error { - // Emit computed macros to a macros file in the output directory. - // If the build configuration produces no macros, no file is written and - // macrosFileName will be empty. var macrosFileName string macrosFilePath, err := p.writeMacrosFile(component, outputDir) @@ -291,32 +287,27 @@ func (p *sourcePreparerImpl) applyOverlaysToSources( macrosFileName = filepath.Base(macrosFilePath) } - // Apply all overlays to prepared sources. if err := p.applyOverlays(ctx, component, outputDir, macrosFileName); err != nil { - return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) + return fmt.Errorf("failed to apply overlays for component %#q:\n%w", + component.GetName(), err) } return nil } // applyOverlays applies all overlays (user-defined and system-generated) to the -// component sources. Overlay application is decoupled from git history generation: -// overlays modify the working tree; synthetic history is recorded separately by -// [trySyntheticHistory]. +// component sources. func (p *sourcePreparerImpl) applyOverlays( - _ context.Context, component components.Component, sourcesDirPath, macrosFileName string, + ctx context.Context, component components.Component, sourcesDirPath, macrosFileName string, ) error { event := p.eventListener.StartEvent("Applying overlays", "component", component.GetName()) defer event.End() - // Resolve the spec path once for all overlay operations in this call. absSpecPath, err := p.resolveSpecPath(component, sourcesDirPath) if err != nil { return err } - // Collect all overlays in application order. This ensures every change is - // captured in the synthetic history, including build configuration changes. allOverlays, err := p.collectOverlays(component, macrosFileName) if err != nil { return fmt.Errorf("failed to collect overlays for component %#q:\n%w", component.GetName(), err) @@ -326,7 +317,13 @@ func (p *sourcePreparerImpl) applyOverlays( return nil } - // Apply all overlays to the working tree. + // Tarball overlays are applied first (they modify archived source files + // in-place), followed by spec and loose-file overlays. Each function + // self-filters to the overlay types it handles. + if err := p.applyTarballOverlayGroup(ctx, component, sourcesDirPath, allOverlays); err != nil { + return err + } + if err := p.applyOverlayList(allOverlays, sourcesDirPath, absSpecPath); err != nil { return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } @@ -334,6 +331,40 @@ func (p *sourcePreparerImpl) applyOverlays( return nil } +// applyTarballOverlayGroup applies tarball overlays. Skipped when source +// downloads were not performed. +func (p *sourcePreparerImpl) applyTarballOverlayGroup( + ctx context.Context, component components.Component, + sourcesDirPath string, tarballOverlays []projectconfig.ComponentOverlay, +) error { + if len(tarballOverlays) == 0 { + return nil + } + + if p.skipLookaside { + slog.Warn("Skipping tarball overlays because source downloads were skipped (--skip-sources)", + "component", component.GetName(), + "count", len(tarballOverlays)) + + return nil + } + + cmdFactory, ok := p.dryRunnable.(opctx.CmdFactory) + if !ok { + return errors.New( + "tarball overlays require a CmdFactory; the provided DryRunnable does not implement it") + } + + if err := applyTarballOverlays( + ctx, cmdFactory, p.fs, p.eventListener, sourcesDirPath, tarballOverlays, + ); err != nil { + return fmt.Errorf("failed to apply tarball overlays for component %#q:\n%w", + component.GetName(), err) + } + + return nil +} + // collectOverlays gathers all overlays for a component into a single ordered slice: // macros-load first, then user overlays, followed by check-skip and file-header overlays. func (p *sourcePreparerImpl) collectOverlays( @@ -634,9 +665,17 @@ func (p *sourcePreparerImpl) DiffSources( // enforced by [projectconfig.ConfigFile.Validate]). Setting `ReplaceUpstream` = true without // a matching upstream entry is also an error: the user expressed intent to replace something // that isn't there, which almost certainly indicates a stale config or filename typo. -func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, outputDir string) error { - sourceFiles := component.GetConfig().SourceFiles - if len(sourceFiles) == 0 { +func (p *sourcePreparerImpl) updateSourcesFile( + component components.Component, outputDir string, +) error { + config := component.GetConfig() + sourceFiles := config.SourceFiles + + // Derive tarball names from the component's overlays — no need to thread + // them through the overlay application chain. + modifiedTarballs := tarballNamesFromOverlays(config.Overlays) + + if len(sourceFiles) == 0 && len(modifiedTarballs) == 0 { return nil } @@ -647,7 +686,19 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return err } - mergedLines, err := p.buildSourceEntries(sourceFiles, existingContent, component.GetName(), outputDir) + // Parse once, then rehash modified tarballs and merge source-files entries + // on the parsed representation — single parse, single write. + existingLines, err := fedorasource.ReadSourcesFile(existingContent) + if err != nil { + return fmt.Errorf("failed to parse 'sources' file %#q:\n%w", sourcesFilePath, err) + } + + // Rehash tarballs that were modified by tarball overlays in-place. + if err := p.rehashModifiedEntries(existingLines, outputDir, modifiedTarballs); err != nil { + return err + } + + mergedLines, err := p.buildSourceEntries(sourceFiles, existingLines, component.GetName(), outputDir) if err != nil { return err } @@ -666,6 +717,47 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return nil } +// rehashModifiedEntries updates the Raw and Entry fields of parsed 'sources' lines +// for tarballs that were modified by tarball overlays. The hash is recomputed using +// the same hash type as the original entry. +func (p *sourcePreparerImpl) rehashModifiedEntries( + lines []fedorasource.SourcesFileLine, outputDir string, modifiedTarballs []string, +) error { + if len(modifiedTarballs) == 0 { + return nil + } + + modified := make(map[string]bool, len(modifiedTarballs)) + for _, name := range modifiedTarballs { + modified[name] = true + } + + for idx, line := range lines { + if line.Entry == nil || !modified[line.Entry.Filename] { + continue + } + + tarballPath := filepath.Join(outputDir, line.Entry.Filename) + + newHash, err := fileutils.ComputeFileHash(p.fs, line.Entry.HashType, tarballPath) + if err != nil { + return fmt.Errorf("rehashing modified tarball %#q:\n%w", line.Entry.Filename, err) + } + + slog.Debug("Rehashed modified tarball in 'sources' file", + "tarball", line.Entry.Filename, + "hashType", line.Entry.HashType, + "oldHash", line.Entry.Hash, + "newHash", newHash, + ) + + lines[idx].Raw = fedorasource.FormatSourcesEntry(line.Entry.Filename, line.Entry.HashType, newHash) + lines[idx].Entry.Hash = newHash + } + + return nil +} + // readSourcesFileIfExists reads the 'sources' file content if it exists, returning empty string if not. func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (string, error) { exists, err := fileutils.Exists(p.fs, sourcesFilePath) @@ -685,32 +777,24 @@ func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (st return string(data), nil } -// buildSourceEntries validates [projectconfig.SourceFileReference] entries and returns -// the merged set of lines ready to be written to the 'sources' file. Before returning, -// it logs an INFO-level event indicating that the 'sources' file will be updated, -// including the counts of newly added and replaced entries. +// buildSourceEntries merges user-declared [projectconfig.SourceFileReference] entries +// into the parsed 'sources' lines. Returns the final set of raw lines ready to be +// written to the 'sources' file. // // Output ordering and preservation: -// - Each line of [existingContent] is emitted verbatim, except for entry lines whose +// - Each existing line is emitted verbatim, except for entry lines whose // filename matches a replacement, which are swapped for the new formatted entry. -// Comments and blank lines from the original file are kept in their original positions. -// - Brand-new entries (no upstream filename collision) are appended after the upstream -// content in the order they appear in [sourceFiles]. +// Comments and blank lines are kept in their original positions. +// - Brand-new entries (no upstream filename collision) are appended after the +// existing content in the order they appear in [sourceFiles]. // // Collision rules and hash resolution are documented on [sourcePreparerImpl.processSourceRef]. func (p *sourcePreparerImpl) buildSourceEntries( sourceFiles []projectconfig.SourceFileReference, - existingContent string, + existingLines []fedorasource.SourcesFileLine, componentName string, outputDir string, ) (mergedLines []string, err error) { - existingLines, err := fedorasource.ReadSourcesFile(existingContent) - if err != nil { - return nil, fmt.Errorf( - "failed to parse existing 'sources' file at %#q:\n%w", - filepath.Join(outputDir, fedorasource.SourcesFileName), err) - } - // Index upstream entries by filename for O(1) collision lookup. The parser // (fedorasource.ReadSourcesFile) errors on duplicate filenames, so the // entries are guaranteed unique by the time we get here. diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index 45a9d286..d21aa52d 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -854,7 +854,7 @@ func TestPrepareSources_UpdatesSourcesFile(t *testing.T) { existingSourcesContent: "SHA512 (dup.tar.gz) = aaaa1111\nSHA512 (dup.tar.gz) = bbbb2222\n", expectError: true, errorContains: []string{ - "failed to parse existing 'sources' file", + "failed to parse 'sources' file", "duplicate filename", "dup.tar.gz", }, diff --git a/internal/app/azldev/core/sources/tarballoverlays.go b/internal/app/azldev/core/sources/tarballoverlays.go new file mode 100644 index 00000000..5f9d4fe8 --- /dev/null +++ b/internal/app/azldev/core/sources/tarballoverlays.go @@ -0,0 +1,358 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/bmatcuk/doublestar/v4" + "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/tarball" +) + +// applyTarballOverlays groups tarball overlays by target archive and processes +// them in order. Multiple overlays targeting the same tarball are batched into +// a single extract/modify/repack cycle. All operations (extract, modify, repack) +// are performed in pure Go on the host, except for patch application which +// shells out to the host's `patch` command. +func applyTarballOverlays( + ctx context.Context, + cmdFactory opctx.CmdFactory, + fs opctx.FS, + eventListener opctx.EventListener, + sourcesDirPath string, + overlays []projectconfig.ComponentOverlay, +) error { + groups := groupOverlaysByTarball(overlays) + + if len(groups) == 0 { + return nil + } + + event := eventListener.StartEvent("Applying tarball overlays", + "tarballs", len(groups), + "operations", len(overlays), + ) + defer event.End() + + for _, group := range groups { + if err := processTarball(ctx, cmdFactory, fs, sourcesDirPath, group); err != nil { + return fmt.Errorf("tarball overlay failed for %#q:\n%w", group.tarball, err) + } + } + + return nil +} + +// tarballGroup holds overlays targeting the same tarball, preserving order. +type tarballGroup struct { + tarball string + overlays []projectconfig.ComponentOverlay +} + +// groupOverlaysByTarball groups tarball overlays by their +// [projectconfig.ComponentOverlay.Tarball] field, preserving insertion order +// within each group and across groups. Non-tarball overlays are silently skipped. +func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) []tarballGroup { + orderMap := make(map[string]int) + + var groups []tarballGroup + + for _, overlay := range overlays { + if !overlay.ModifiesTarball() { + continue + } + + idx, exists := orderMap[overlay.Tarball] + if !exists { + idx = len(groups) + orderMap[overlay.Tarball] = idx + + groups = append(groups, tarballGroup{tarball: overlay.Tarball}) + } + + groups[idx].overlays = append(groups[idx].overlays, overlay) + } + + return groups +} + +// processTarball extracts a tarball to a temp directory, applies all overlays, +// and deterministically repacks it in-place with the original compression. +func processTarball( + ctx context.Context, + cmdFactory opctx.CmdFactory, + fs opctx.FS, + sourcesDirPath string, + group tarballGroup, +) error { + archivePath := filepath.Join(sourcesDirPath, group.tarball) + + // Create a temporary directory for extraction. + workDir, err := os.MkdirTemp("", "tarball-overlay-") + if err != nil { + return fmt.Errorf("creating temp directory:\n%w", err) + } + + defer func() { + if removeErr := os.RemoveAll(workDir); removeErr != nil { + slog.Warn("Failed to clean up tarball work directory", "error", removeErr) + } + }() + + // Detect compression and extract. + compression, err := tarball.DetectCompression(group.tarball) + if err != nil { + return fmt.Errorf("detecting compression for %#q:\n%w", group.tarball, err) + } + + if err := tarball.Extract(fs, archivePath, workDir, compression); err != nil { + return fmt.Errorf("extracting tarball:\n%w", err) + } + + // Determine the root of the extracted content. Most source tarballs have + // a single top-level directory (e.g., "pkg-1.0/"). + extractRoot, err := tarball.ResolveExtractRoot(workDir) + if err != nil { + return fmt.Errorf("resolving extract root:\n%w", err) + } + + // Apply each overlay operation in order. + for _, overlay := range group.overlays { + if err := applyTarballOperation(ctx, cmdFactory, fs, extractRoot, overlay); err != nil { + return fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) + } + } + + // Deterministically repack the tarball in-place. + if err := tarball.RepackDeterministic(fs, archivePath, workDir, compression); err != nil { + return fmt.Errorf("repacking tarball:\n%w", err) + } + + slog.Info("Tarball overlay applied", "tarball", group.tarball) + + return nil +} + +// applyTarballOperation dispatches a single overlay to the appropriate handler. +func applyTarballOperation( + ctx context.Context, + cmdFactory opctx.CmdFactory, + fs opctx.FS, + extractRoot string, + overlay projectconfig.ComponentOverlay, +) error { + //nolint:exhaustive // Only tarball overlay types are valid here; the default catches the rest. + switch overlay.Type { + case projectconfig.ComponentOverlayTarballFileRemove: + return tarballFileRemove(extractRoot, overlay.Filename) + + case projectconfig.ComponentOverlayTarballSearchReplace: + return tarballSearchReplace(extractRoot, overlay.Filename, overlay.Regex, overlay.Replacement) + + case projectconfig.ComponentOverlayTarballPatch: + stripLevel := 1 + + if overlay.Value != "" { + parsed, err := strconv.Atoi(overlay.Value) + if err != nil { + return fmt.Errorf("invalid strip level %#q:\n%w", overlay.Value, err) + } + + stripLevel = parsed + } + + return tarballApplyPatch(ctx, cmdFactory, fs, extractRoot, overlay.Source, stripLevel) + + default: + return fmt.Errorf("unsupported tarball overlay type %#q", overlay.Type) + } +} + +// tarballFileRemove removes files matching a glob pattern from the extracted tree. +func tarballFileRemove(extractRoot, pattern string) error { + matches, err := globFilesInDir(extractRoot, pattern) + if err != nil { + return err + } + + if len(matches) == 0 { + return fmt.Errorf("no files match pattern %#q:\n%w", pattern, ErrOverlayDidNotApply) + } + + for _, path := range matches { + if err := os.Remove(path); err != nil { + return fmt.Errorf("failed to remove %#q:\n%w", path, err) + } + } + + return nil +} + +// tarballSearchReplace applies regex search-and-replace to files matching a glob +// pattern inside the extracted tree. +func tarballSearchReplace(extractRoot, pattern, regex, replacement string) error { + matches, err := globFilesInDir(extractRoot, pattern) + if err != nil { + return err + } + + if len(matches) == 0 { + return fmt.Errorf("no files match pattern %#q:\n%w", pattern, ErrOverlayDidNotApply) + } + + compiled, err := regexp.Compile(regex) + if err != nil { + return fmt.Errorf("invalid regex %#q:\n%w", regex, err) + } + + anyReplaced := false + + for _, path := range matches { + content, readErr := os.ReadFile(path) + if readErr != nil { + return fmt.Errorf("reading %#q:\n%w", path, readErr) + } + + newContent := compiled.ReplaceAll(content, []byte(replacement)) + if string(newContent) != string(content) { + anyReplaced = true + + if writeErr := os.WriteFile(path, newContent, fileperms.PublicFile); writeErr != nil { + return fmt.Errorf("writing %#q:\n%w", path, writeErr) + } + } + } + + if !anyReplaced { + return fmt.Errorf("regex %#q did not match any content in files matching %#q:\n%w", + regex, pattern, ErrOverlayDidNotApply) + } + + return nil +} + +// tarballApplyPatch applies a unified diff patch to the extracted tree by +// shelling out to the host's `patch` command. +func tarballApplyPatch( + ctx context.Context, + cmdFactory opctx.CmdFactory, + fs opctx.FS, + extractRoot, patchSource string, + stripLevel int, +) error { + if !cmdFactory.CommandInSearchPath("patch") { + return errors.New("'patch' command not found in PATH; " + + "install the 'patch' package to use tarball-patch overlays") + } + + // Read the patch file content via the abstract FS (supports both real and test FSs). + patchData, err := fileutils.ReadFile(fs, patchSource) + if err != nil { + return fmt.Errorf("reading patch file %#q:\n%w", patchSource, err) + } + + // Write to a temp file on the real filesystem so the patch command can read it. + tmpPatch, err := os.CreateTemp("", "tarball-patch-*.patch") + if err != nil { + return fmt.Errorf("creating temp patch file:\n%w", err) + } + + defer os.Remove(tmpPatch.Name()) + + if _, err := tmpPatch.Write(patchData); err != nil { + tmpPatch.Close() + + return fmt.Errorf("writing temp patch file:\n%w", err) + } + + tmpPatch.Close() + + var stderr strings.Builder + + rawCmd := exec.CommandContext(ctx, "patch", + fmt.Sprintf("-p%d", stripLevel), + "-i", tmpPatch.Name(), + ) + rawCmd.Dir = extractRoot + rawCmd.Stderr = &stderr + + cmd, err := cmdFactory.Command(rawCmd) + if err != nil { + return fmt.Errorf("creating patch command:\n%w", err) + } + + if runErr := cmd.Run(ctx); runErr != nil { + return fmt.Errorf("patch failed:\n%s\n%w", stderr.String(), runErr) + } + + return nil +} + +// globFilesInDir finds files under root matching a glob pattern. +// Supports doublestar patterns (e.g., "**/*.md"). +func globFilesInDir(root, pattern string) ([]string, error) { + var matches []string + + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + + if entry.IsDir() { + return nil + } + + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + return fmt.Errorf("computing relative path for %#q:\n%w", path, relErr) + } + + matched, matchErr := doublestar.Match(pattern, rel) + if matchErr != nil { + return fmt.Errorf("invalid glob pattern %#q:\n%w", pattern, matchErr) + } + + if matched { + matches = append(matches, path) + } + + return nil + }) + if err != nil { + return nil, fmt.Errorf("walking directory for glob %#q:\n%w", pattern, err) + } + + return matches, nil +} + +// tarballNamesFromOverlays returns the unique tarball filenames targeted by +// tarball overlays in the given overlay list. Used by [updateSourcesFile] to +// determine which 'sources' entries need rehashing after overlay application. +func tarballNamesFromOverlays(overlays []projectconfig.ComponentOverlay) []string { + seen := make(map[string]bool) + + var names []string + + for _, overlay := range overlays { + if overlay.ModifiesTarball() && !seen[overlay.Tarball] { + seen[overlay.Tarball] = true + names = append(names, overlay.Tarball) + } + } + + return names +} diff --git a/internal/app/azldev/core/sources/tarballoverlays_internal_test.go b/internal/app/azldev/core/sources/tarballoverlays_internal_test.go new file mode 100644 index 00000000..ac5a666e --- /dev/null +++ b/internal/app/azldev/core/sources/tarballoverlays_internal_test.go @@ -0,0 +1,137 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "os" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGroupOverlaysByTarball(t *testing.T) { + t.Run("groups overlays by tarball name preserving order", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + { + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Tarball: "pkg-1.0.tar.gz", + Filename: "config.h", + Regex: "old", + Replacement: "new", + }, + { + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "other-2.0.tar.xz", + Filename: "docs/*.md", + }, + } + + groups := groupOverlaysByTarball(overlays) + + require.Len(t, groups, 2) + + assert.Equal(t, "pkg-1.0.tar.gz", groups[0].tarball) + require.Len(t, groups[0].overlays, 2) + assert.Equal(t, projectconfig.ComponentOverlayTarballFileRemove, groups[0].overlays[0].Type) + assert.Equal(t, projectconfig.ComponentOverlayTarballSearchReplace, groups[0].overlays[1].Type) + + assert.Equal(t, "other-2.0.tar.xz", groups[1].tarball) + require.Len(t, groups[1].overlays, 1) + }) + + t.Run("skips non-tarball overlays", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0"}, + {Type: projectconfig.ComponentOverlayTarballFileRemove, Tarball: "pkg.tar.gz", Filename: "f"}, + {Type: projectconfig.ComponentOverlayAddFile, Filename: "new.txt", Source: "src"}, + } + + groups := groupOverlaysByTarball(overlays) + + require.Len(t, groups, 1) + assert.Equal(t, "pkg.tar.gz", groups[0].tarball) + require.Len(t, groups[0].overlays, 1) + }) +} + +func TestGlobFilesInDir(t *testing.T) { + workDir := t.TempDir() + + require.NoError(t, os.MkdirAll(workDir+"/sub", 0o755)) + require.NoError(t, os.WriteFile(workDir+"/file.txt", nil, fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(workDir+"/sub/deep.txt", nil, fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(workDir+"/sub/other.md", nil, fileperms.PrivateFile)) + + t.Run("simple glob", func(t *testing.T) { + matches, err := globFilesInDir(workDir, "*.txt") + require.NoError(t, err) + require.Len(t, matches, 1) + }) + + t.Run("doublestar glob", func(t *testing.T) { + matches, err := globFilesInDir(workDir, "**/*.txt") + require.NoError(t, err) + require.Len(t, matches, 2) + }) + + t.Run("no matches", func(t *testing.T) { + matches, err := globFilesInDir(workDir, "*.rs") + require.NoError(t, err) + assert.Empty(t, matches) + }) +} + +func TestTarballFileRemove(t *testing.T) { + workDir := t.TempDir() + + require.NoError(t, os.WriteFile(workDir+"/keep.txt", []byte("keep"), fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(workDir+"/remove.conf", []byte("remove"), fileperms.PrivateFile)) + + err := tarballFileRemove(workDir, "*.conf") + require.NoError(t, err) + + assert.FileExists(t, workDir+"/keep.txt") + assert.NoFileExists(t, workDir+"/remove.conf") +} + +func TestTarballFileRemoveNoMatch(t *testing.T) { + workDir := t.TempDir() + + require.NoError(t, os.WriteFile(workDir+"/file.txt", nil, fileperms.PrivateFile)) + + err := tarballFileRemove(workDir, "*.conf") + require.Error(t, err) + assert.ErrorIs(t, err, ErrOverlayDidNotApply) +} + +func TestTarballSearchReplace(t *testing.T) { + workDir := t.TempDir() + + require.NoError(t, os.WriteFile(workDir+"/config.h", []byte("#define OLD_VALUE 1\n"), fileperms.PrivateFile)) + + err := tarballSearchReplace(workDir, "config.h", "OLD_VALUE", "NEW_VALUE") + require.NoError(t, err) + + content, readErr := os.ReadFile(workDir + "/config.h") + require.NoError(t, readErr) + assert.Equal(t, "#define NEW_VALUE 1\n", string(content)) +} + +func TestTarballSearchReplaceNoMatch(t *testing.T) { + workDir := t.TempDir() + + require.NoError(t, os.WriteFile(workDir+"/config.h", []byte("#define SOMETHING 1\n"), fileperms.PrivateFile)) + + err := tarballSearchReplace(workDir, "config.h", "NONEXISTENT", "new") + require.Error(t, err) + assert.ErrorIs(t, err, ErrOverlayDidNotApply) +} diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 7ec0b50e..1fa8fcc2 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -17,10 +17,13 @@ import ( // ComponentOverlay represents an overlay that may be applied to a component's spec and/or its sources. type ComponentOverlay struct { // The type of overlay to apply. - Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"` + Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,enum=tarball-file-remove,enum=tarball-search-replace,enum=tarball-patch,title=Overlay type,description=The type of overlay to apply"` // Human readable description of overlay; primarily present to document the need for the change. Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay" fingerprint:"-"` + // For overlays that target files inside a source tarball, identifies the tarball to modify. + // Must be a filename (not a path) matching a source archive in the component's sources directory. + Tarball string `toml:"tarball,omitempty" json:"tarball,omitempty" jsonschema:"title=Tarball,description=The source tarball to modify (e.g. pkg-1.0.tar.gz)"` // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). Filename string `toml:"file,omitempty" json:"file,omitempty" jsonschema:"title=Filename,description=The name of the non-spec file to which this overlay applies, or a glob pattern matching multiple files"` @@ -119,6 +122,14 @@ func (c *ComponentOverlay) ModifiesSpec() bool { c.Type == ComponentOverlayRemovePatch } +// ModifiesTarball returns true if the overlay modifies files inside a source tarball. +// These overlays require a mock chroot for extraction and repacking. +func (c *ComponentOverlay) ModifiesTarball() bool { + return c.Type == ComponentOverlayTarballFileRemove || + c.Type == ComponentOverlayTarballSearchReplace || + c.Type == ComponentOverlayTarballPatch +} + // ModifiesNonSpecFiles returns true if the overlay modifies non-spec files. This includes // hybrid overlays that modify both spec and source files (e.g., patch overlays), since // those also require non-spec modifications. @@ -182,12 +193,21 @@ const ( ComponentOverlayRemoveFile ComponentOverlayType = "file-remove" // ComponentOverlayRenameFile is an overlay that renames a non-spec file. ComponentOverlayRenameFile ComponentOverlayType = "file-rename" + // ComponentOverlayTarballFileRemove is an overlay that removes file(s) from inside a source tarball. + // The tarball is extracted, matching files are deleted, and the tarball is repacked. + ComponentOverlayTarballFileRemove ComponentOverlayType = "tarball-file-remove" + // ComponentOverlayTarballSearchReplace is an overlay that performs regex search-and-replace + // on file(s) inside a source tarball. + ComponentOverlayTarballSearchReplace ComponentOverlayType = "tarball-search-replace" + // ComponentOverlayTarballPatch is an overlay that applies a unified diff patch to the + // extracted contents of a source tarball. + ComponentOverlayTarballPatch ComponentOverlayType = "tarball-patch" ) // Validate checks that required fields are set based on the overlay type. This catches // configuration errors at load time rather than at apply time. // -//nolint:cyclop,gocognit,gocyclo,funlen // complexity is inherent to the number of overlay types. +//nolint:cyclop,gocognit,gocyclo,funlen,maintidx // complexity is inherent to the number of overlay types. func (c *ComponentOverlay) Validate() error { desc := c.Description if desc == "" { @@ -329,6 +349,46 @@ func (c *ComponentOverlay) Validate() error { if err := validateGlobPattern(c.Filename, desc); err != nil { return err } + case ComponentOverlayTarballFileRemove: + if err := requireFileBasename("tarball", c.Tarball); err != nil { + return err + } + + if err := requireRelativePath("file", c.Filename); err != nil { + return err + } + + if err := validateGlobPattern(c.Filename, desc); err != nil { + return err + } + case ComponentOverlayTarballSearchReplace: + if err := requireFileBasename("tarball", c.Tarball); err != nil { + return err + } + + if err := requireRelativePath("file", c.Filename); err != nil { + return err + } + + if err := validateGlobPattern(c.Filename, desc); err != nil { + return err + } + + if c.Regex == "" { + return missingField("regex") + } + + if err := validateRegex(c.Regex, desc); err != nil { + return err + } + case ComponentOverlayTarballPatch: + if err := requireFileBasename("tarball", c.Tarball); err != nil { + return err + } + + if c.Source == "" { + return missingField("source") + } default: return fmt.Errorf("unknown overlay type %#q: %#q", c.Type, desc) } diff --git a/internal/projectconfig/overlay_test.go b/internal/projectconfig/overlay_test.go index 84ccf200..7b75f0e2 100644 --- a/internal/projectconfig/overlay_test.go +++ b/internal/projectconfig/overlay_test.go @@ -412,6 +412,147 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: true, errorContains: "section", }, + // tarball-file-remove tests + { + name: "tarball-file-remove valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + errorExpected: false, + }, + { + name: "tarball-file-remove valid with glob", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "pkg-1.0.tar.gz", + Filename: "docs/**/*.md", + }, + errorExpected: false, + }, + { + name: "tarball-file-remove missing tarball", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballFileRemove, + Filename: "unwanted.conf", + }, + errorExpected: true, + errorContains: "tarball", + }, + { + name: "tarball-file-remove missing file", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "pkg-1.0.tar.gz", + }, + errorExpected: true, + errorContains: "file", + }, + { + name: "tarball-file-remove rejects tarball path", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballFileRemove, + Tarball: "subdir/pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + errorExpected: true, + errorContains: "tarball", + }, + // tarball-search-replace tests + { + name: "tarball-search-replace valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Tarball: "pkg-1.0.tar.gz", + Filename: "config.h", + Regex: "old_value", + Replacement: "new_value", + }, + errorExpected: false, + }, + { + name: "tarball-search-replace missing tarball", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Filename: "config.h", + Regex: "old_value", + Replacement: "new_value", + }, + errorExpected: true, + errorContains: "tarball", + }, + { + name: "tarball-search-replace missing file", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Tarball: "pkg-1.0.tar.gz", + Regex: "old_value", + Replacement: "new_value", + }, + errorExpected: true, + errorContains: "file", + }, + { + name: "tarball-search-replace missing regex", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Tarball: "pkg-1.0.tar.gz", + Filename: "config.h", + }, + errorExpected: true, + errorContains: "regex", + }, + { + name: "tarball-search-replace invalid regex", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballSearchReplace, + Tarball: "pkg-1.0.tar.gz", + Filename: "config.h", + Regex: "[invalid", + Replacement: "new_value", + }, + errorExpected: true, + errorContains: "regex", + }, + // tarball-patch tests + { + name: "tarball-patch valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballPatch, + Tarball: "pkg-1.0.tar.gz", + Source: "patches/fix.patch", + }, + errorExpected: false, + }, + { + name: "tarball-patch valid with strip level", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballPatch, + Tarball: "pkg-1.0.tar.gz", + Source: "patches/fix.patch", + Value: "2", + }, + errorExpected: false, + }, + { + name: "tarball-patch missing tarball", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballPatch, + Source: "patches/fix.patch", + }, + errorExpected: true, + errorContains: "tarball", + }, + { + name: "tarball-patch missing source", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayTarballPatch, + Tarball: "pkg-1.0.tar.gz", + }, + errorExpected: true, + errorContains: "source", + }, } for _, testCase := range testCases { @@ -455,6 +596,12 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { projectconfig.ComponentOverlayAddFile, } + tarballOverlayTypes := []projectconfig.ComponentOverlayType{ + projectconfig.ComponentOverlayTarballFileRemove, + projectconfig.ComponentOverlayTarballSearchReplace, + projectconfig.ComponentOverlayTarballPatch, + } + for _, overlayType := range specOverlayTypes { t.Run(string(overlayType)+"_is_spec_overlay", func(t *testing.T) { overlay := projectconfig.ComponentOverlay{Type: overlayType} @@ -468,4 +615,13 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlayType) }) } + + for _, overlayType := range tarballOverlayTypes { + t.Run(string(overlayType)+"_is_tarball_overlay", func(t *testing.T) { + overlay := projectconfig.ComponentOverlay{Type: overlayType} + assert.True(t, overlay.ModifiesTarball(), "expected %s to be a tarball overlay", overlayType) + assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlayType) + assert.False(t, overlay.ModifiesNonSpecFiles(), "expected %s to not be a non-spec overlay", overlayType) + }) + } } From 23e6dc1b733201bad6db34393d13799d4d62b1da Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Thu, 4 Jun 2026 21:42:05 +0000 Subject: [PATCH 2/5] refactor: collapse tarball overlays into archive-scoped file overlays --- docs/user/reference/config/overlays.md | 46 +++- .../app/azldev/core/sources/sourceprep.go | 9 +- .../azldev/core/sources/tarballoverlays.go | 246 ++++++++---------- .../sources/tarballoverlays_internal_test.go | 222 ++++++++++++---- internal/projectconfig/overlay.go | 103 ++++---- internal/projectconfig/overlay_test.go | 121 ++++++--- internal/utils/archive/archive.go | 27 ++ schemas/azldev.schema.json | 13 +- 8 files changed, 496 insertions(+), 291 deletions(-) diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 626b4957..e445b665 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -47,16 +47,29 @@ successfully makes a replacement to at least one matching file. | `file-remove` | Removes a file | `file` | Glob pattern for files to remove | | `file-rename` | Renames a file within the same directory | `file`, `replacement` | Name of file to rename | -### Tarball Overlays +> **Tip:** `file-remove` and `file-search-replace` can also operate inside a source tarball by +> setting the `tarball` field — see [Archive (Tarball) Overlays](#archive-tarball-overlays). -These overlays modify files **inside** source tarballs. The tarball is extracted into a temporary directory, modifications are applied, and the tarball is repacked with the same compression format. Extraction and repacking are handled natively; patch application requires the `patch` command on the host. +### Archive (Tarball) Overlays -> **Note:** Tarball overlays are applied before spec and file overlays, so subsequent overlays see the modified tarball. The `tarball-patch` overlay type requires the `patch` command to be installed on the host. +File overlays can operate on files **inside** a source tarball instead of loose files in the +sources tree. Set the `tarball` field on a `file-remove` or `file-search-replace` overlay to +scope it to that archive; the dedicated `tarball-patch` type applies a unified diff to extracted +archive contents. The tarball is extracted into a temporary directory, the modifications are +applied with the same machinery as loose-file overlays, and the tarball is repacked with its +original compression format. Extraction and repacking are handled natively; `tarball-patch` +additionally requires the `patch` command on the host. + +> **Note:** Archive overlays are batched per tarball — all overlays targeting the same archive +> share a single extract/modify/repack cycle — and the `sources` file is rehashed afterward to +> reflect the repacked archive. They are processed independently of spec and loose-file overlays. + +> **Extraction root:** Paths in archive overlays (`file`, and the paths a patch targets) are interpreted relative to the archive's extraction root. By default the root is inferred: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. Set `tarball-root` to override this — the equivalent of rpmbuild's `%setup -n` — when an archive's top-level directory does not follow that convention. | Type | Description | Required Fields | |------|-------------|-----------------| -| `tarball-file-remove` | Removes file(s) matching a glob pattern from inside a tarball | `tarball`, `file` | -| `tarball-search-replace` | Regex-based search and replace on file(s) inside a tarball | `tarball`, `file`, `regex` | +| `file-remove` + `tarball` | Removes file(s) matching a glob pattern from inside a tarball | `tarball`, `file` | +| `file-search-replace` + `tarball` | Regex-based search and replace on file(s) inside a tarball | `tarball`, `file`, `regex` | | `tarball-patch` | Applies a unified diff patch to the extracted tarball contents | `tarball`, `source` | ## Field Reference @@ -65,15 +78,16 @@ These overlays modify files **inside** source tarballs. The tarball is extracted |-------|----------|-------------|---------| | Type | `type` | **Required.** The overlay type to apply | All overlays | | Description | `description` | Human-readable explanation documenting the need for the change; helps identify overlays in error messages | All (optional) | -| Tarball | `tarball` | The source tarball filename to modify (must be a basename, not a path) | `tarball-file-remove`, `tarball-search-replace`, `tarball-patch` | +| Tarball | `tarball` | The source tarball filename to scope an overlay to (must be a basename, not a path). When set, the overlay operates on files inside that archive. | `tarball-patch` (required); `file-remove`, `file-search-replace` (optional) | +| Tarball root | `tarball-root` | Top-level directory inside the tarball to treat as the extraction root (mirrors `%setup -n`); inferred when unset. Must be a local relative path (no `..` or absolute paths). When multiple overlays target the same tarball, any that set this must agree. | `tarball-patch`, and archive-scoped `file-remove` / `file-search-replace` (optional) | | Tag | `tag` | The spec tag name (e.g., `BuildRequires`, `Requires`, `Version`) | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` | | Value | `value` | The tag value to set, or value to match for removal. For `tarball-patch`, sets the patch strip level (default: `1`, equivalent to `patch -p1`). | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching), `tarball-patch` (optional) | | Section | `section` | The spec section to target (e.g., `%build`, `%install`, `%files`, `%description`) | `spec-prepend-lines`, `spec-append-lines`, `spec-search-replace` (optional), `spec-remove-section` | | Package | `package` | The sub-package name for multi-package specs; omit to target the main package | All spec overlays (optional, except `spec-remove-subpackage` which **requires** it) | -| Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace`, `tarball-search-replace` | -| Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename`, `tarball-search-replace` | +| Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace` | +| Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename` | | Lines | `lines` | Array of text lines to insert | `spec-prepend-lines`, `spec-append-lines`, `file-prepend-lines` | -| File | `file` | The name of the non-spec file to modify or add | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove`, `tarball-file-remove`, `tarball-search-replace` | +| File | `file` | The name of the non-spec file to modify or add, or a glob pattern. For archive-scoped overlays, it is matched against the tarball's extracted contents. | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | | Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add`, `tarball-patch` | > **Note:** For `file-rename`, the `replacement` field is a **filename only** (not a path). The file is renamed within its current directory. @@ -289,22 +303,28 @@ description = "Remove CVE patches that are now upstream" ### Removing a File from a Tarball -The `tarball-file-remove` overlay deletes files matching a glob pattern from inside a source -tarball. The tarball is extracted, matching files are removed, and the tarball is repacked. +Set the `tarball` field on a `file-remove` overlay to delete files matching a glob pattern from +inside a source tarball. The tarball is extracted, matching files are removed, and the tarball is +repacked. ```toml [[components.mypackage.overlays]] -type = "tarball-file-remove" +type = "file-remove" tarball = "mypackage-1.0.tar.gz" file = "vendor/**" description = "Remove bundled vendor directory" ``` +> **Tip:** Without the `tarball` field, the same `file-remove` overlay removes a loose file from +> the sources tree instead. The `tarball` field is the only thing that scopes it to an archive. + ### Search and Replace Inside a Tarball +Set the `tarball` field on a `file-search-replace` overlay to rewrite content inside an archive: + ```toml [[components.mypackage.overlays]] -type = "tarball-search-replace" +type = "file-search-replace" tarball = "mypackage-1.0.tar.xz" file = "configure.ac" regex = "AC_CHECK_LIB\\(old_lib" diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 62649e22..1bbf7cf6 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -356,7 +356,7 @@ func (p *sourcePreparerImpl) applyTarballOverlayGroup( } if err := applyTarballOverlays( - ctx, cmdFactory, p.fs, p.eventListener, sourcesDirPath, tarballOverlays, + ctx, cmdFactory, p.dryRunnable, p.fs, p.eventListener, sourcesDirPath, tarballOverlays, ); err != nil { return fmt.Errorf("failed to apply tarball overlays for component %#q:\n%w", component.GetName(), err) @@ -1169,10 +1169,17 @@ func (p *sourcePreparerImpl) resolveSpecPath( } // applyOverlayList applies a list of overlays to the component sources sequentially. +// Archive-scoped overlays (see [projectconfig.ComponentOverlay.ModifiesTarball]) are +// skipped here; they are handled separately by [applyTarballOverlays], which batches +// extraction and repacking per archive. func (p *sourcePreparerImpl) applyOverlayList( overlays []projectconfig.ComponentOverlay, sourcesDirPath, absSpecPath string, ) error { for _, overlay := range overlays { + if overlay.ModifiesTarball() { + continue + } + if err := ApplyOverlayToSources( p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath, ); err != nil { diff --git a/internal/app/azldev/core/sources/tarballoverlays.go b/internal/app/azldev/core/sources/tarballoverlays.go index 5f9d4fe8..bbf6d4d0 100644 --- a/internal/app/azldev/core/sources/tarballoverlays.go +++ b/internal/app/azldev/core/sources/tarballoverlays.go @@ -11,32 +11,35 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "strconv" "strings" - "github.com/bmatcuk/doublestar/v4" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/tarball" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" ) // applyTarballOverlays groups tarball overlays by target archive and processes // them in order. Multiple overlays targeting the same tarball are batched into -// a single extract/modify/repack cycle. All operations (extract, modify, repack) -// are performed in pure Go on the host, except for patch application which -// shells out to the host's `patch` command. +// a single extract/modify/repack cycle. File modifications inside the archive +// reuse the same machinery as loose-file overlays ([applyNonSpecOverlay]); patch +// application shells out to the host's `patch` command. func applyTarballOverlays( ctx context.Context, cmdFactory opctx.CmdFactory, + dryRunnable opctx.DryRunnable, fs opctx.FS, eventListener opctx.EventListener, sourcesDirPath string, overlays []projectconfig.ComponentOverlay, ) error { - groups := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByTarball(overlays) + if err != nil { + return err + } if len(groups) == 0 { return nil @@ -49,7 +52,7 @@ func applyTarballOverlays( defer event.End() for _, group := range groups { - if err := processTarball(ctx, cmdFactory, fs, sourcesDirPath, group); err != nil { + if err := processTarball(ctx, cmdFactory, dryRunnable, fs, sourcesDirPath, group); err != nil { return fmt.Errorf("tarball overlay failed for %#q:\n%w", group.tarball, err) } } @@ -60,13 +63,19 @@ func applyTarballOverlays( // tarballGroup holds overlays targeting the same tarball, preserving order. type tarballGroup struct { tarball string + root string overlays []projectconfig.ComponentOverlay } // groupOverlaysByTarball groups tarball overlays by their // [projectconfig.ComponentOverlay.Tarball] field, preserving insertion order // within each group and across groups. Non-tarball overlays are silently skipped. -func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) []tarballGroup { +// +// The optional [projectconfig.ComponentOverlay.TarballRoot] override (mirroring +// rpmbuild's `%setup -n`) is reconciled per tarball: all overlays targeting the +// same archive that set it must agree, otherwise the configuration is ambiguous +// and an error is returned. +func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) ([]tarballGroup, error) { orderMap := make(map[string]int) var groups []tarballGroup @@ -84,10 +93,21 @@ func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) []tarball groups = append(groups, tarballGroup{tarball: overlay.Tarball}) } + if overlay.TarballRoot != "" { + if groups[idx].root != "" && groups[idx].root != overlay.TarballRoot { + return nil, fmt.Errorf( + "conflicting %#q overrides for tarball %#q: %#q vs %#q", + "tarball-root", overlay.Tarball, groups[idx].root, overlay.TarballRoot, + ) + } + + groups[idx].root = overlay.TarballRoot + } + groups[idx].overlays = append(groups[idx].overlays, overlay) } - return groups + return groups, nil } // processTarball extracts a tarball to a temp directory, applies all overlays, @@ -95,50 +115,66 @@ func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) []tarball func processTarball( ctx context.Context, cmdFactory opctx.CmdFactory, + dryRunnable opctx.DryRunnable, fs opctx.FS, sourcesDirPath string, group tarballGroup, ) error { archivePath := filepath.Join(sourcesDirPath, group.tarball) - // Create a temporary directory for extraction. - workDir, err := os.MkdirTemp("", "tarball-overlay-") + // Create a temporary directory for extraction. The injected FS is real-filesystem + // backed in production, so the returned path is a genuine on-disk path usable by + // the [archive] package and the host `patch` command. + workDir, err := fileutils.MkdirTempInTempDir(fs, "tarball-overlay-") if err != nil { return fmt.Errorf("creating temp directory:\n%w", err) } defer func() { - if removeErr := os.RemoveAll(workDir); removeErr != nil { + if removeErr := fs.RemoveAll(workDir); removeErr != nil { slog.Warn("Failed to clean up tarball work directory", "error", removeErr) } }() - // Detect compression and extract. - compression, err := tarball.DetectCompression(group.tarball) - if err != nil { - return fmt.Errorf("detecting compression for %#q:\n%w", group.tarball, err) - } - - if err := tarball.Extract(fs, archivePath, workDir, compression); err != nil { + // Extract the archive; compression is inferred from the filename extension. + if err := archive.ExtractAuto(archivePath, workDir); err != nil { return fmt.Errorf("extracting tarball:\n%w", err) } // Determine the root of the extracted content. Most source tarballs have - // a single top-level directory (e.g., "pkg-1.0/"). - extractRoot, err := tarball.ResolveExtractRoot(workDir) + // a single top-level directory (e.g., "pkg-1.0/"); group.root overrides this + // inference when set (mirrors rpmbuild's `%setup -n`). + extractRoot, err := resolveExtractRoot(workDir, group.root) if err != nil { return fmt.Errorf("resolving extract root:\n%w", err) } + // Confine an FS to the extract root so file overlays reuse the same machinery + // as loose-file overlays. The extracted tree is always on the real filesystem + // (written by the [archive] package), so root it on an OS-backed FS regardless + // of the injected fs implementation. + extractFS, err := rootfs.New(extractRoot) + if err != nil { + return fmt.Errorf("confining FS to extract root:\n%w", err) + } + + defer func() { + if closeErr := extractFS.Close(); closeErr != nil { + slog.Warn("Failed to close extract-root FS", "error", closeErr) + } + }() + // Apply each overlay operation in order. for _, overlay := range group.overlays { - if err := applyTarballOperation(ctx, cmdFactory, fs, extractRoot, overlay); err != nil { + if err := applyTarballOperation( + ctx, cmdFactory, dryRunnable, fs, extractFS, extractRoot, overlay, + ); err != nil { return fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) } } - // Deterministically repack the tarball in-place. - if err := tarball.RepackDeterministic(fs, archivePath, workDir, compression); err != nil { + // Deterministically repack the tarball in-place, reusing the original compression. + if err := archive.CreateDeterministicArchiveAuto(archivePath, workDir); err != nil { return fmt.Errorf("repacking tarball:\n%w", err) } @@ -147,23 +183,19 @@ func processTarball( return nil } -// applyTarballOperation dispatches a single overlay to the appropriate handler. +// applyTarballOperation dispatches a single overlay against the extracted tree. +// The dedicated tarball-patch type shells out to the host `patch` command; all +// other (archive-scoped) types reuse [applyNonSpecOverlay], operating on the +// extract-root FS exactly as they would on the loose sources tree. func applyTarballOperation( ctx context.Context, cmdFactory opctx.CmdFactory, - fs opctx.FS, + dryRunnable opctx.DryRunnable, + sourceFS, extractFS opctx.FS, extractRoot string, overlay projectconfig.ComponentOverlay, ) error { - //nolint:exhaustive // Only tarball overlay types are valid here; the default catches the rest. - switch overlay.Type { - case projectconfig.ComponentOverlayTarballFileRemove: - return tarballFileRemove(extractRoot, overlay.Filename) - - case projectconfig.ComponentOverlayTarballSearchReplace: - return tarballSearchReplace(extractRoot, overlay.Filename, overlay.Regex, overlay.Replacement) - - case projectconfig.ComponentOverlayTarballPatch: + if overlay.Type == projectconfig.ComponentOverlayTarballPatch { stripLevel := 1 if overlay.Value != "" { @@ -175,74 +207,10 @@ func applyTarballOperation( stripLevel = parsed } - return tarballApplyPatch(ctx, cmdFactory, fs, extractRoot, overlay.Source, stripLevel) - - default: - return fmt.Errorf("unsupported tarball overlay type %#q", overlay.Type) - } -} - -// tarballFileRemove removes files matching a glob pattern from the extracted tree. -func tarballFileRemove(extractRoot, pattern string) error { - matches, err := globFilesInDir(extractRoot, pattern) - if err != nil { - return err - } - - if len(matches) == 0 { - return fmt.Errorf("no files match pattern %#q:\n%w", pattern, ErrOverlayDidNotApply) - } - - for _, path := range matches { - if err := os.Remove(path); err != nil { - return fmt.Errorf("failed to remove %#q:\n%w", path, err) - } - } - - return nil -} - -// tarballSearchReplace applies regex search-and-replace to files matching a glob -// pattern inside the extracted tree. -func tarballSearchReplace(extractRoot, pattern, regex, replacement string) error { - matches, err := globFilesInDir(extractRoot, pattern) - if err != nil { - return err - } - - if len(matches) == 0 { - return fmt.Errorf("no files match pattern %#q:\n%w", pattern, ErrOverlayDidNotApply) - } - - compiled, err := regexp.Compile(regex) - if err != nil { - return fmt.Errorf("invalid regex %#q:\n%w", regex, err) - } - - anyReplaced := false - - for _, path := range matches { - content, readErr := os.ReadFile(path) - if readErr != nil { - return fmt.Errorf("reading %#q:\n%w", path, readErr) - } - - newContent := compiled.ReplaceAll(content, []byte(replacement)) - if string(newContent) != string(content) { - anyReplaced = true - - if writeErr := os.WriteFile(path, newContent, fileperms.PublicFile); writeErr != nil { - return fmt.Errorf("writing %#q:\n%w", path, writeErr) - } - } - } - - if !anyReplaced { - return fmt.Errorf("regex %#q did not match any content in files matching %#q:\n%w", - regex, pattern, ErrOverlayDidNotApply) + return tarballApplyPatch(ctx, cmdFactory, sourceFS, extractRoot, overlay.Source, stripLevel) } - return nil + return applyNonSpecOverlay(dryRunnable, sourceFS, extractFS, overlay) } // tarballApplyPatch applies a unified diff patch to the extracted tree by @@ -265,27 +233,29 @@ func tarballApplyPatch( return fmt.Errorf("reading patch file %#q:\n%w", patchSource, err) } - // Write to a temp file on the real filesystem so the patch command can read it. - tmpPatch, err := os.CreateTemp("", "tarball-patch-*.patch") + // Stage the patch in a temp dir so the host `patch` command can read it. The + // injected FS is real-filesystem backed in production, so the path is on disk. + tmpDir, err := fileutils.MkdirTempInTempDir(fs, "tarball-patch-") if err != nil { - return fmt.Errorf("creating temp patch file:\n%w", err) + return fmt.Errorf("creating temp patch directory:\n%w", err) } - defer os.Remove(tmpPatch.Name()) - - if _, err := tmpPatch.Write(patchData); err != nil { - tmpPatch.Close() + defer func() { + if removeErr := fs.RemoveAll(tmpDir); removeErr != nil { + slog.Warn("Failed to clean up tarball patch directory", "error", removeErr) + } + }() + tmpPatchPath := filepath.Join(tmpDir, "overlay.patch") + if err := fileutils.WriteFile(fs, tmpPatchPath, patchData, fileperms.PublicFile); err != nil { return fmt.Errorf("writing temp patch file:\n%w", err) } - tmpPatch.Close() - var stderr strings.Builder rawCmd := exec.CommandContext(ctx, "patch", fmt.Sprintf("-p%d", stripLevel), - "-i", tmpPatch.Name(), + "-i", tmpPatchPath, ) rawCmd.Dir = extractRoot rawCmd.Stderr = &stderr @@ -302,41 +272,45 @@ func tarballApplyPatch( return nil } -// globFilesInDir finds files under root matching a glob pattern. -// Supports doublestar patterns (e.g., "**/*.md"). -func globFilesInDir(root, pattern string) ([]string, error) { - var matches []string - - err := filepath.WalkDir(root, func(path string, entry os.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr +// resolveExtractRoot returns the effective root of an extracted tarball. +// When rootOverride is set (the `%setup -n` equivalent), the named subdirectory +// of workDir is used; it must be a local path that exists as a directory. When +// rootOverride is empty, the root is inferred: if workDir contains exactly one +// entry and that entry is a directory (the common case for source tarballs like +// "pkg-1.0/"), that subdirectory is returned; otherwise workDir itself is +// returned. +func resolveExtractRoot(workDir, rootOverride string) (string, error) { + if rootOverride != "" { + // Defense in depth: validation already rejects non-local overrides, but + // re-check before joining so a malformed value can never escape workDir. + if !filepath.IsLocal(rootOverride) { + return "", fmt.Errorf("tarball root %#q is not a local path", rootOverride) } - if entry.IsDir() { - return nil - } + target := filepath.Join(workDir, rootOverride) - rel, relErr := filepath.Rel(root, path) - if relErr != nil { - return fmt.Errorf("computing relative path for %#q:\n%w", path, relErr) + info, err := os.Stat(target) + if err != nil { + return "", fmt.Errorf("tarball root %#q not found after extraction:\n%w", rootOverride, err) } - matched, matchErr := doublestar.Match(pattern, rel) - if matchErr != nil { - return fmt.Errorf("invalid glob pattern %#q:\n%w", pattern, matchErr) + if !info.IsDir() { + return "", fmt.Errorf("tarball root %#q is not a directory", rootOverride) } - if matched { - matches = append(matches, path) - } + return target, nil + } - return nil - }) + entries, err := os.ReadDir(workDir) if err != nil { - return nil, fmt.Errorf("walking directory for glob %#q:\n%w", pattern, err) + return "", fmt.Errorf("reading extracted directory:\n%w", err) + } + + if len(entries) == 1 && entries[0].IsDir() { + return filepath.Join(workDir, entries[0].Name()), nil } - return matches, nil + return workDir, nil } // tarballNamesFromOverlays returns the unique tarball filenames targeted by diff --git a/internal/app/azldev/core/sources/tarballoverlays_internal_test.go b/internal/app/azldev/core/sources/tarballoverlays_internal_test.go index ac5a666e..45acbcf8 100644 --- a/internal/app/azldev/core/sources/tarballoverlays_internal_test.go +++ b/internal/app/azldev/core/sources/tarballoverlays_internal_test.go @@ -4,11 +4,14 @@ package sources import ( + "context" "os" "testing" + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -17,121 +20,228 @@ func TestGroupOverlaysByTarball(t *testing.T) { t.Run("groups overlays by tarball name preserving order", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ { - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", Filename: "unwanted.conf", }, { - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Filename: "config.h", Regex: "old", Replacement: "new", }, { - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "other-2.0.tar.xz", Filename: "docs/*.md", }, } - groups := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByTarball(overlays) + require.NoError(t, err) require.Len(t, groups, 2) assert.Equal(t, "pkg-1.0.tar.gz", groups[0].tarball) require.Len(t, groups[0].overlays, 2) - assert.Equal(t, projectconfig.ComponentOverlayTarballFileRemove, groups[0].overlays[0].Type) - assert.Equal(t, projectconfig.ComponentOverlayTarballSearchReplace, groups[0].overlays[1].Type) + assert.Equal(t, projectconfig.ComponentOverlayRemoveFile, groups[0].overlays[0].Type) + assert.Equal(t, projectconfig.ComponentOverlaySearchAndReplaceInFile, groups[0].overlays[1].Type) assert.Equal(t, "other-2.0.tar.xz", groups[1].tarball) require.Len(t, groups[1].overlays, 1) }) - t.Run("skips non-tarball overlays", func(t *testing.T) { + t.Run("skips overlays that are not archive-scoped", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ {Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0"}, - {Type: projectconfig.ComponentOverlayTarballFileRemove, Tarball: "pkg.tar.gz", Filename: "f"}, + {Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg.tar.gz", Filename: "f"}, + // Plain (non-archive) file overlay: no Tarball set, so it must be skipped. + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "loose.txt"}, {Type: projectconfig.ComponentOverlayAddFile, Filename: "new.txt", Source: "src"}, } - groups := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByTarball(overlays) + require.NoError(t, err) require.Len(t, groups, 1) assert.Equal(t, "pkg.tar.gz", groups[0].tarball) require.Len(t, groups[0].overlays, 1) }) -} -func TestGlobFilesInDir(t *testing.T) { - workDir := t.TempDir() + t.Run("reconciles matching tarball-root overrides", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg-1.0.tar.gz", + TarballRoot: "custom-root", + Filename: "a.conf", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg-1.0.tar.gz", + Filename: "b.conf", + }, + } - require.NoError(t, os.MkdirAll(workDir+"/sub", 0o755)) - require.NoError(t, os.WriteFile(workDir+"/file.txt", nil, fileperms.PrivateFile)) - require.NoError(t, os.WriteFile(workDir+"/sub/deep.txt", nil, fileperms.PrivateFile)) - require.NoError(t, os.WriteFile(workDir+"/sub/other.md", nil, fileperms.PrivateFile)) + groups, err := groupOverlaysByTarball(overlays) + require.NoError(t, err) + + require.Len(t, groups, 1) + assert.Equal(t, "custom-root", groups[0].root) + }) - t.Run("simple glob", func(t *testing.T) { - matches, err := globFilesInDir(workDir, "*.txt") + t.Run("errors on conflicting tarball-root overrides", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg-1.0.tar.gz", + TarballRoot: "root-a", + Filename: "a.conf", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg-1.0.tar.gz", + TarballRoot: "root-b", + Filename: "b.conf", + }, + } + + _, err := groupOverlaysByTarball(overlays) + require.Error(t, err) + assert.Contains(t, err.Error(), "conflicting") + }) +} + +func TestResolveExtractRoot(t *testing.T) { + t.Run("infers single top-level directory", func(t *testing.T) { + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(workDir+"/pkg-1.0", 0o755)) + + root, err := resolveExtractRoot(workDir, "") require.NoError(t, err) - require.Len(t, matches, 1) + assert.Equal(t, workDir+"/pkg-1.0", root) }) - t.Run("doublestar glob", func(t *testing.T) { - matches, err := globFilesInDir(workDir, "**/*.txt") + t.Run("falls back to workDir for multiple top-level entries", func(t *testing.T) { + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(workDir+"/dirA", 0o755)) + require.NoError(t, os.WriteFile(workDir+"/loose.txt", nil, fileperms.PrivateFile)) + + root, err := resolveExtractRoot(workDir, "") require.NoError(t, err) - require.Len(t, matches, 2) + assert.Equal(t, workDir, root) }) - t.Run("no matches", func(t *testing.T) { - matches, err := globFilesInDir(workDir, "*.rs") + t.Run("override selects named subdirectory", func(t *testing.T) { + workDir := t.TempDir() + // Two top-level dirs so the heuristic would not pick one. + require.NoError(t, os.MkdirAll(workDir+"/dirA", 0o755)) + require.NoError(t, os.MkdirAll(workDir+"/dirB", 0o755)) + + root, err := resolveExtractRoot(workDir, "dirB") require.NoError(t, err) - assert.Empty(t, matches) + assert.Equal(t, workDir+"/dirB", root) }) -} -func TestTarballFileRemove(t *testing.T) { - workDir := t.TempDir() + t.Run("override missing directory errors", func(t *testing.T) { + workDir := t.TempDir() - require.NoError(t, os.WriteFile(workDir+"/keep.txt", []byte("keep"), fileperms.PrivateFile)) - require.NoError(t, os.WriteFile(workDir+"/remove.conf", []byte("remove"), fileperms.PrivateFile)) + _, err := resolveExtractRoot(workDir, "does-not-exist") + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") + }) - err := tarballFileRemove(workDir, "*.conf") - require.NoError(t, err) + t.Run("override pointing at a file errors", func(t *testing.T) { + workDir := t.TempDir() + require.NoError(t, os.WriteFile(workDir+"/afile", nil, fileperms.PrivateFile)) - assert.FileExists(t, workDir+"/keep.txt") - assert.NoFileExists(t, workDir+"/remove.conf") + _, err := resolveExtractRoot(workDir, "afile") + require.Error(t, err) + assert.Contains(t, err.Error(), "not a directory") + }) + + t.Run("non-local override is rejected", func(t *testing.T) { + workDir := t.TempDir() + + _, err := resolveExtractRoot(workDir, "../escape") + require.Error(t, err) + assert.Contains(t, err.Error(), "not a local path") + }) } -func TestTarballFileRemoveNoMatch(t *testing.T) { - workDir := t.TempDir() +// TestApplyTarballOperation_FileOverlays verifies that archive-scoped file +// overlays are routed through the shared [applyNonSpecOverlay] machinery against +// the extract-root FS (i.e., the same code path as loose-file overlays). +func TestApplyTarballOperation_FileOverlays(t *testing.T) { + ctx := testctx.NewCtx() - require.NoError(t, os.WriteFile(workDir+"/file.txt", nil, fileperms.PrivateFile)) + t.Run("file-remove deletes matching files in the extracted tree", func(t *testing.T) { + extractRoot := t.TempDir() + require.NoError(t, os.WriteFile(extractRoot+"/keep.txt", []byte("keep"), fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(extractRoot+"/remove.conf", []byte("x"), fileperms.PrivateFile)) - err := tarballFileRemove(workDir, "*.conf") - require.Error(t, err) - assert.ErrorIs(t, err, ErrOverlayDidNotApply) -} + extractFS, err := rootfs.New(extractRoot) + require.NoError(t, err) -func TestTarballSearchReplace(t *testing.T) { - workDir := t.TempDir() + defer extractFS.Close() - require.NoError(t, os.WriteFile(workDir+"/config.h", []byte("#define OLD_VALUE 1\n"), fileperms.PrivateFile)) + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg.tar.gz", + Filename: "*.conf", + } - err := tarballSearchReplace(workDir, "config.h", "OLD_VALUE", "NEW_VALUE") - require.NoError(t, err) + err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) + require.NoError(t, err) - content, readErr := os.ReadFile(workDir + "/config.h") - require.NoError(t, readErr) - assert.Equal(t, "#define NEW_VALUE 1\n", string(content)) -} + assert.FileExists(t, extractRoot+"/keep.txt") + assert.NoFileExists(t, extractRoot+"/remove.conf") + }) + + t.Run("file-search-replace rewrites matching content in the extracted tree", func(t *testing.T) { + extractRoot := t.TempDir() + require.NoError(t, os.WriteFile( + extractRoot+"/config.h", []byte("#define OLD_VALUE 1\n"), fileperms.PrivateFile)) -func TestTarballSearchReplaceNoMatch(t *testing.T) { - workDir := t.TempDir() + extractFS, err := rootfs.New(extractRoot) + require.NoError(t, err) + + defer extractFS.Close() + + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Tarball: "pkg.tar.gz", + Filename: "config.h", + Regex: "OLD_VALUE", + Replacement: "NEW_VALUE", + } + + err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) + require.NoError(t, err) + + content, readErr := os.ReadFile(extractRoot + "/config.h") + require.NoError(t, readErr) + assert.Equal(t, "#define NEW_VALUE 1\n", string(content)) + }) + + t.Run("file-remove with no match errors like a loose-file overlay", func(t *testing.T) { + extractRoot := t.TempDir() + require.NoError(t, os.WriteFile(extractRoot+"/file.txt", nil, fileperms.PrivateFile)) + + extractFS, err := rootfs.New(extractRoot) + require.NoError(t, err) - require.NoError(t, os.WriteFile(workDir+"/config.h", []byte("#define SOMETHING 1\n"), fileperms.PrivateFile)) + defer extractFS.Close() - err := tarballSearchReplace(workDir, "config.h", "NONEXISTENT", "new") - require.Error(t, err) - assert.ErrorIs(t, err, ErrOverlayDidNotApply) + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg.tar.gz", + Filename: "*.conf", + } + + err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) + require.Error(t, err) + assert.Contains(t, err.Error(), "did not match any files") + }) } diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 1fa8fcc2..5b20d894 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -17,13 +17,21 @@ import ( // ComponentOverlay represents an overlay that may be applied to a component's spec and/or its sources. type ComponentOverlay struct { // The type of overlay to apply. - Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,enum=tarball-file-remove,enum=tarball-search-replace,enum=tarball-patch,title=Overlay type,description=The type of overlay to apply"` + Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,enum=tarball-patch,title=Overlay type,description=The type of overlay to apply"` // Human readable description of overlay; primarily present to document the need for the change. Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay" fingerprint:"-"` // For overlays that target files inside a source tarball, identifies the tarball to modify. // Must be a filename (not a path) matching a source archive in the component's sources directory. + // Required for tarball-patch; optional for file-remove and file-search-replace, which operate + // inside the named archive instead of the loose sources tree when it is set. Tarball string `toml:"tarball,omitempty" json:"tarball,omitempty" jsonschema:"title=Tarball,description=The source tarball to modify (e.g. pkg-1.0.tar.gz)"` + // For overlays that target files inside a source tarball, optionally overrides the top-level + // directory to treat as the extraction root, mirroring rpmbuild's `%setup -n`. When unset, the + // root is inferred: if the archive unpacks to a single top-level directory (the conventional + // `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. + // Set this when an archive's top-level directory does not follow that convention. + TarballRoot string `toml:"tarball-root,omitempty" json:"tarballRoot,omitempty" jsonschema:"title=Tarball root,description=Top-level directory inside the tarball to treat as the extraction root (mirrors %setup -n); inferred when unset"` // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). Filename string `toml:"file,omitempty" json:"file,omitempty" jsonschema:"title=Filename,description=The name of the non-spec file to which this overlay applies, or a glob pattern matching multiple files"` @@ -123,17 +131,27 @@ func (c *ComponentOverlay) ModifiesSpec() bool { } // ModifiesTarball returns true if the overlay modifies files inside a source tarball. -// These overlays require a mock chroot for extraction and repacking. +// These overlays require extraction and repacking of the archive. The dedicated +// tarball-patch type is always archive-scoped; the file-remove and file-search-replace +// types become archive-scoped when their [ComponentOverlay.Tarball] field is set. func (c *ComponentOverlay) ModifiesTarball() bool { - return c.Type == ComponentOverlayTarballFileRemove || - c.Type == ComponentOverlayTarballSearchReplace || - c.Type == ComponentOverlayTarballPatch + if c.Type == ComponentOverlayTarballPatch { + return true + } + + return c.Tarball != "" && + (c.Type == ComponentOverlayRemoveFile || c.Type == ComponentOverlaySearchAndReplaceInFile) } // ModifiesNonSpecFiles returns true if the overlay modifies non-spec files. This includes // hybrid overlays that modify both spec and source files (e.g., patch overlays), since -// those also require non-spec modifications. +// those also require non-spec modifications. Archive-scoped overlays (see [ModifiesTarball]) +// are excluded: they operate on files inside a tarball, not loose files in the sources tree. func (c *ComponentOverlay) ModifiesNonSpecFiles() bool { + if c.ModifiesTarball() { + return false + } + return c.Type == ComponentOverlayPrependLinesToFile || c.Type == ComponentOverlaySearchAndReplaceInFile || c.Type == ComponentOverlayAddFile || @@ -186,19 +204,17 @@ const ( // ComponentOverlayPrependLinesToFile is an overlay that prepends lines to a non-spec file. ComponentOverlayPrependLinesToFile ComponentOverlayType = "file-prepend-lines" // ComponentOverlaySearchAndReplaceInFile is an overlay that replaces text in a non-spec file. + // When its [ComponentOverlay.Tarball] field is set, it operates on file(s) inside that source + // tarball instead of loose files in the sources tree. ComponentOverlaySearchAndReplaceInFile ComponentOverlayType = "file-search-replace" // ComponentOverlayAddFile is an overlay that adds a non-spec file. ComponentOverlayAddFile ComponentOverlayType = "file-add" - // ComponentOverlayRemoveFile is an overlay that removes a non-spec file. + // ComponentOverlayRemoveFile is an overlay that removes a non-spec file. When its + // [ComponentOverlay.Tarball] field is set, it removes file(s) from inside that source + // tarball instead of loose files in the sources tree. ComponentOverlayRemoveFile ComponentOverlayType = "file-remove" // ComponentOverlayRenameFile is an overlay that renames a non-spec file. ComponentOverlayRenameFile ComponentOverlayType = "file-rename" - // ComponentOverlayTarballFileRemove is an overlay that removes file(s) from inside a source tarball. - // The tarball is extracted, matching files are deleted, and the tarball is repacked. - ComponentOverlayTarballFileRemove ComponentOverlayType = "tarball-file-remove" - // ComponentOverlayTarballSearchReplace is an overlay that performs regex search-and-replace - // on file(s) inside a source tarball. - ComponentOverlayTarballSearchReplace ComponentOverlayType = "tarball-search-replace" // ComponentOverlayTarballPatch is an overlay that applies a unified diff patch to the // extracted contents of a source tarball. ComponentOverlayTarballPatch ComponentOverlayType = "tarball-patch" @@ -252,6 +268,35 @@ func (c *ComponentOverlay) Validate() error { return nil } + // The tarball field scopes an overlay to operate inside a source archive. It is only + // accepted on the archive-capable types and must be a bare filename (not a path). + if c.Tarball != "" { + //nolint:exhaustive // Only archive-capable types accept the tarball field; the default rejects the rest. + switch c.Type { + case ComponentOverlayRemoveFile, ComponentOverlaySearchAndReplaceInFile, ComponentOverlayTarballPatch: + if err := requireFileBasename("tarball", c.Tarball); err != nil { + return err + } + default: + return unexpectedField("tarball") + } + } + + // The tarball-root override is only meaningful for archive-scoped overlays, and must be a + // local relative path so it cannot escape the extraction directory. + if c.TarballRoot != "" { + if !c.ModifiesTarball() { + return unexpectedField("tarball-root") + } + + if !filepath.IsLocal(c.TarballRoot) { + return fmt.Errorf( + "overlay type %#q requires %#q to be a local relative path (no %#q or absolute paths); found %#q", + c.Type, "tarball-root", "..", c.TarballRoot, + ) + } + } + switch c.Type { case ComponentOverlayAddSpecTag, ComponentOverlayInsertSpecTag, ComponentOverlaySetSpecTag, ComponentOverlayUpdateSpecTag: @@ -349,38 +394,6 @@ func (c *ComponentOverlay) Validate() error { if err := validateGlobPattern(c.Filename, desc); err != nil { return err } - case ComponentOverlayTarballFileRemove: - if err := requireFileBasename("tarball", c.Tarball); err != nil { - return err - } - - if err := requireRelativePath("file", c.Filename); err != nil { - return err - } - - if err := validateGlobPattern(c.Filename, desc); err != nil { - return err - } - case ComponentOverlayTarballSearchReplace: - if err := requireFileBasename("tarball", c.Tarball); err != nil { - return err - } - - if err := requireRelativePath("file", c.Filename); err != nil { - return err - } - - if err := validateGlobPattern(c.Filename, desc); err != nil { - return err - } - - if c.Regex == "" { - return missingField("regex") - } - - if err := validateRegex(c.Regex, desc); err != nil { - return err - } case ComponentOverlayTarballPatch: if err := requireFileBasename("tarball", c.Tarball); err != nil { return err diff --git a/internal/projectconfig/overlay_test.go b/internal/projectconfig/overlay_test.go index 7b75f0e2..1a760a7a 100644 --- a/internal/projectconfig/overlay_test.go +++ b/internal/projectconfig/overlay_test.go @@ -412,80 +412,121 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: true, errorContains: "section", }, - // tarball-file-remove tests + // archive-scoped file-remove tests (tarball modifier) { - name: "tarball-file-remove valid", + name: "file-remove with tarball valid (archive-scoped)", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", Filename: "unwanted.conf", }, errorExpected: false, }, { - name: "tarball-file-remove valid with glob", + name: "file-remove with tarball glob valid", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", Filename: "docs/**/*.md", }, errorExpected: false, }, { - name: "tarball-file-remove missing tarball", + name: "file-remove without tarball is a plain loose-file remove", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Filename: "unwanted.conf", }, - errorExpected: true, - errorContains: "tarball", + errorExpected: false, }, { - name: "tarball-file-remove missing file", + name: "file-remove with tarball missing file", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", }, errorExpected: true, errorContains: "file", }, { - name: "tarball-file-remove rejects tarball path", + name: "file-remove rejects tarball path", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballFileRemove, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "subdir/pkg-1.0.tar.gz", Filename: "unwanted.conf", }, errorExpected: true, errorContains: "tarball", }, - // tarball-search-replace tests { - name: "tarball-search-replace valid", + name: "tarball rejected on overlay type that cannot be archive-scoped", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayAddFile, + Tarball: "pkg-1.0.tar.gz", + Filename: "new.conf", + Source: "files/new.conf", + }, + errorExpected: true, + errorContains: "tarball", + }, + { + name: "file-remove with tarball-root override valid", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", - Filename: "config.h", - Regex: "old_value", - Replacement: "new_value", + TarballRoot: "src-root", + Filename: "unwanted.conf", }, errorExpected: false, }, { - name: "tarball-search-replace missing tarball", + name: "file-remove rejects non-local tarball-root", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Tarball: "pkg-1.0.tar.gz", + TarballRoot: "../escape", + Filename: "unwanted.conf", + }, + errorExpected: true, + errorContains: "tarball-root", + }, + { + name: "tarball-root rejected when tarball unset", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlayRemoveFile, + TarballRoot: "src-root", + Filename: "unwanted.conf", + }, + errorExpected: true, + errorContains: "tarball-root", + }, + { + name: "tarball-root rejected on non-archive overlay", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlaySetSpecTag, + Tag: "Version", + Value: "1.0.0", + TarballRoot: "src-root", + }, + errorExpected: true, + errorContains: "tarball-root", + }, + // archive-scoped file-search-replace tests (tarball modifier) + { + name: "file-search-replace with tarball valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Tarball: "pkg-1.0.tar.gz", Filename: "config.h", Regex: "old_value", Replacement: "new_value", }, - errorExpected: true, - errorContains: "tarball", + errorExpected: false, }, { - name: "tarball-search-replace missing file", + name: "file-search-replace with tarball missing file", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Regex: "old_value", Replacement: "new_value", @@ -494,9 +535,9 @@ func TestComponentOverlay_Validate(t *testing.T) { errorContains: "file", }, { - name: "tarball-search-replace missing regex", + name: "file-search-replace with tarball missing regex", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Filename: "config.h", }, @@ -504,9 +545,9 @@ func TestComponentOverlay_Validate(t *testing.T) { errorContains: "regex", }, { - name: "tarball-search-replace invalid regex", + name: "file-search-replace with tarball invalid regex", overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballSearchReplace, + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Filename: "config.h", Regex: "[invalid", @@ -596,10 +637,12 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { projectconfig.ComponentOverlayAddFile, } - tarballOverlayTypes := []projectconfig.ComponentOverlayType{ - projectconfig.ComponentOverlayTarballFileRemove, - projectconfig.ComponentOverlayTarballSearchReplace, - projectconfig.ComponentOverlayTarballPatch, + // Archive-scoped overlays: tarball-patch is always archive-scoped, while file-remove + // and file-search-replace become archive-scoped only when their Tarball field is set. + tarballOverlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayTarballPatch, Tarball: "pkg-1.0.tar.gz", Source: "p.patch"}, + {Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", Filename: "f"}, + {Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Filename: "f"}, } for _, overlayType := range specOverlayTypes { @@ -616,12 +659,12 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { }) } - for _, overlayType := range tarballOverlayTypes { - t.Run(string(overlayType)+"_is_tarball_overlay", func(t *testing.T) { - overlay := projectconfig.ComponentOverlay{Type: overlayType} - assert.True(t, overlay.ModifiesTarball(), "expected %s to be a tarball overlay", overlayType) - assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlayType) - assert.False(t, overlay.ModifiesNonSpecFiles(), "expected %s to not be a non-spec overlay", overlayType) + for _, overlay := range tarballOverlays { + t.Run(string(overlay.Type)+"_is_archive_scoped", func(t *testing.T) { + assert.True(t, overlay.ModifiesTarball(), "expected %s to be an archive-scoped overlay", overlay.Type) + assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlay.Type) + assert.False(t, overlay.ModifiesNonSpecFiles(), + "expected %s to not be a loose non-spec overlay", overlay.Type) }) } } diff --git a/internal/utils/archive/archive.go b/internal/utils/archive/archive.go index 65be9a18..a50dff61 100644 --- a/internal/utils/archive/archive.go +++ b/internal/utils/archive/archive.go @@ -79,6 +79,20 @@ func DetectCompression(filename string) (Compression, error) { } } +// ExtractAuto is a convenience wrapper that infers the compression from +// archivePath's extension via [DetectCompression] and then calls [Extract]. +// Most callers should prefer this over the explicit-compression [Extract], +// which exists for cases where the compression cannot be derived from the +// filename. +func ExtractAuto(archivePath, destDir string) error { + comp, err := DetectCompression(archivePath) + if err != nil { + return fmt.Errorf("detecting compression for %#q:\n%w", archivePath, err) + } + + return Extract(archivePath, destDir, comp) +} + // Extract reads a tar archive, decompresses it, and extracts all entries into // destDir. Supported entry types are regular files, directories, and symlinks; // other entry types are skipped. Entry paths are confined to destDir via @@ -174,6 +188,19 @@ func (r readerCloser) Close() error { return nil } +// CreateDeterministicArchiveAuto is a convenience wrapper that infers the +// compression from archivePath's extension via [DetectCompression] and then +// calls [CreateDeterministicArchive]. Most callers should prefer this over the +// explicit-compression [CreateDeterministicArchive]. +func CreateDeterministicArchiveAuto(archivePath, sourceDir string) error { + comp, err := DetectCompression(archivePath) + if err != nil { + return fmt.Errorf("detecting compression for %#q:\n%w", archivePath, err) + } + + return CreateDeterministicArchive(archivePath, sourceDir, comp) +} + // CreateDeterministicArchive creates a new tar archive from the contents of sourceDir // and writes it to archivePath on the OS filesystem, replacing any existing file. // diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index 7aa38d7e..a8048802 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -221,7 +221,8 @@ "file-search-replace", "file-add", "file-remove", - "file-rename" + "file-rename", + "tarball-patch" ], "title": "Overlay type", "description": "The type of overlay to apply" @@ -231,6 +232,16 @@ "title": "Description", "description": "Human readable description of overlay" }, + "tarball": { + "type": "string", + "title": "Tarball", + "description": "The source tarball to modify (e.g. pkg-1.0.tar.gz)" + }, + "tarball-root": { + "type": "string", + "title": "Tarball root", + "description": "Top-level directory inside the tarball to treat as the extraction root (mirrors %setup -n); inferred when unset" + }, "file": { "type": "string", "title": "Filename", From 3a1883c530da03d479491e3058114b24262ff588 Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Mon, 8 Jun 2026 23:30:57 +0000 Subject: [PATCH 3/5] refactor: rename to archive, limit archive overlays to file-remove/search-replace --- docs/user/reference/config/overlays.md | 79 ++--- .../azldev/core/sources/archiveoverlays.go | 231 ++++++++++++ ...st.go => archiveoverlays_internal_test.go} | 103 ++---- .../app/azldev/core/sources/sourceprep.go | 78 ++-- .../azldev/core/sources/tarballoverlays.go | 332 ------------------ internal/projectconfig/overlay.go | 85 ++--- internal/projectconfig/overlay_test.go | 145 ++------ ...ainer_config_generate-schema_stdout_1.snap | 10 + ...shots_config_generate-schema_stdout_1.snap | 10 + schemas/azldev.schema.json | 15 +- 10 files changed, 426 insertions(+), 662 deletions(-) create mode 100644 internal/app/azldev/core/sources/archiveoverlays.go rename internal/app/azldev/core/sources/{tarballoverlays_internal_test.go => archiveoverlays_internal_test.go} (62%) delete mode 100644 internal/app/azldev/core/sources/tarballoverlays.go diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index e445b665..2fad5f7a 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -47,30 +47,27 @@ successfully makes a replacement to at least one matching file. | `file-remove` | Removes a file | `file` | Glob pattern for files to remove | | `file-rename` | Renames a file within the same directory | `file`, `replacement` | Name of file to rename | -> **Tip:** `file-remove` and `file-search-replace` can also operate inside a source tarball by -> setting the `tarball` field — see [Archive (Tarball) Overlays](#archive-tarball-overlays). +> **Tip:** `file-remove` and `file-search-replace` can also operate inside a source archive by +> setting the `archive` field — see [Archive Overlays](#archive-overlays). -### Archive (Tarball) Overlays +### Archive Overlays -File overlays can operate on files **inside** a source tarball instead of loose files in the -sources tree. Set the `tarball` field on a `file-remove` or `file-search-replace` overlay to -scope it to that archive; the dedicated `tarball-patch` type applies a unified diff to extracted -archive contents. The tarball is extracted into a temporary directory, the modifications are -applied with the same machinery as loose-file overlays, and the tarball is repacked with its -original compression format. Extraction and repacking are handled natively; `tarball-patch` -additionally requires the `patch` command on the host. +A `file-remove` or `file-search-replace` overlay can modify files **inside** a source archive +instead of loose files in the sources tree. Set the `archive` field to scope it to that archive. +The archive is extracted into a temporary directory, the matching files are modified with the +same machinery as loose-file overlays, and the archive is repacked with its original compression +format. Extraction and repacking are handled natively. -> **Note:** Archive overlays are batched per tarball — all overlays targeting the same archive +> **Note:** Archive overlays are batched per archive — all overlays targeting the same archive > share a single extract/modify/repack cycle — and the `sources` file is rehashed afterward to > reflect the repacked archive. They are processed independently of spec and loose-file overlays. -> **Extraction root:** Paths in archive overlays (`file`, and the paths a patch targets) are interpreted relative to the archive's extraction root. By default the root is inferred: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. Set `tarball-root` to override this — the equivalent of rpmbuild's `%setup -n` — when an archive's top-level directory does not follow that convention. +> **Extraction root:** The `file` glob in an archive overlay is interpreted relative to the archive's extraction root. By default the root is inferred: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. Set `archive-root` to override this — the equivalent of rpmbuild's `%setup -n` — when an archive's top-level directory does not follow that convention. | Type | Description | Required Fields | |------|-------------|-----------------| -| `file-remove` + `tarball` | Removes file(s) matching a glob pattern from inside a tarball | `tarball`, `file` | -| `file-search-replace` + `tarball` | Regex-based search and replace on file(s) inside a tarball | `tarball`, `file`, `regex` | -| `tarball-patch` | Applies a unified diff patch to the extracted tarball contents | `tarball`, `source` | +| `file-remove` + `archive` | Removes file(s) matching a glob pattern from inside an archive | `archive`, `file` | +| `file-search-replace` + `archive` | Regex-based search and replace on file(s) inside an archive | `archive`, `file`, `regex` | ## Field Reference @@ -78,17 +75,17 @@ additionally requires the `patch` command on the host. |-------|----------|-------------|---------| | Type | `type` | **Required.** The overlay type to apply | All overlays | | Description | `description` | Human-readable explanation documenting the need for the change; helps identify overlays in error messages | All (optional) | -| Tarball | `tarball` | The source tarball filename to scope an overlay to (must be a basename, not a path). When set, the overlay operates on files inside that archive. | `tarball-patch` (required); `file-remove`, `file-search-replace` (optional) | -| Tarball root | `tarball-root` | Top-level directory inside the tarball to treat as the extraction root (mirrors `%setup -n`); inferred when unset. Must be a local relative path (no `..` or absolute paths). When multiple overlays target the same tarball, any that set this must agree. | `tarball-patch`, and archive-scoped `file-remove` / `file-search-replace` (optional) | +| Archive | `archive` | The source archive filename to scope an overlay to (must be a basename, not a path). When set, the overlay operates on files inside that archive. | `file-remove`, `file-search-replace` (optional) | +| Archive root | `archive-root` | Top-level directory inside the archive to treat as the extraction root (mirrors `%setup -n`); inferred when unset. Must be a local relative path (no `..` or absolute paths). When multiple overlays target the same archive, any that set this must agree. | archive-scoped `file-remove` / `file-search-replace` (optional) | | Tag | `tag` | The spec tag name (e.g., `BuildRequires`, `Requires`, `Version`) | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` | -| Value | `value` | The tag value to set, or value to match for removal. For `tarball-patch`, sets the patch strip level (default: `1`, equivalent to `patch -p1`). | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching), `tarball-patch` (optional) | +| Value | `value` | The tag value to set, or value to match for removal. | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching) | | Section | `section` | The spec section to target (e.g., `%build`, `%install`, `%files`, `%description`) | `spec-prepend-lines`, `spec-append-lines`, `spec-search-replace` (optional), `spec-remove-section` | | Package | `package` | The sub-package name for multi-package specs; omit to target the main package | All spec overlays (optional, except `spec-remove-subpackage` which **requires** it) | | Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace` | | Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename` | | Lines | `lines` | Array of text lines to insert | `spec-prepend-lines`, `spec-append-lines`, `file-prepend-lines` | -| File | `file` | The name of the non-spec file to modify or add, or a glob pattern. For archive-scoped overlays, it is matched against the tarball's extracted contents. | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | -| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add`, `tarball-patch` | +| File | `file` | The name of the non-spec file to modify or add, or a glob pattern. For an archive-scoped overlay, it is matched against the archive's extracted contents. | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | +| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add` | > **Note:** For `file-rename`, the `replacement` field is a **filename only** (not a path). The file is renamed within its current directory. @@ -301,61 +298,37 @@ description = "Remove CVE patches that are now upstream" > `PatchN` tags. Macro-based tag numbering (e.g., `Patch%{n}`) is not expanded and may > conflict with auto-assigned numbers. -### Removing a File from a Tarball +### Removing a File from an Archive -Set the `tarball` field on a `file-remove` overlay to delete files matching a glob pattern from -inside a source tarball. The tarball is extracted, matching files are removed, and the tarball is +Set the `archive` field on a `file-remove` overlay to delete files matching a glob pattern from +inside a source archive. The archive is extracted, matching files are removed, and the archive is repacked. ```toml [[components.mypackage.overlays]] type = "file-remove" -tarball = "mypackage-1.0.tar.gz" +archive = "mypackage-1.0.tar.gz" file = "vendor/**" description = "Remove bundled vendor directory" ``` -> **Tip:** Without the `tarball` field, the same `file-remove` overlay removes a loose file from -> the sources tree instead. The `tarball` field is the only thing that scopes it to an archive. +> **Tip:** Without the `archive` field, the same `file-remove` overlay removes a loose file from +> the sources tree instead. The `archive` field is the only thing that scopes it to an archive. -### Search and Replace Inside a Tarball +### Search and Replace Inside an Archive -Set the `tarball` field on a `file-search-replace` overlay to rewrite content inside an archive: +Set the `archive` field on a `file-search-replace` overlay to rewrite content inside an archive: ```toml [[components.mypackage.overlays]] type = "file-search-replace" -tarball = "mypackage-1.0.tar.xz" +archive = "mypackage-1.0.tar.xz" file = "configure.ac" regex = "AC_CHECK_LIB\\(old_lib" replacement = "AC_CHECK_LIB(new_lib" description = "Update library reference in configure script" ``` -### Applying a Patch to Tarball Contents - -The `tarball-patch` overlay applies a unified diff patch to the extracted tarball contents. -By default, it uses `patch -p1`. Use the `value` field to change the strip level. - -```toml -[[components.mypackage.overlays]] -type = "tarball-patch" -tarball = "mypackage-1.0.tar.gz" -source = "patches/fix-build.patch" -description = "Fix build issue in upstream source" -``` - -With a custom strip level: - -```toml -[[components.mypackage.overlays]] -type = "tarball-patch" -tarball = "mypackage-1.0.tar.gz" -source = "patches/fix-build.patch" -value = "0" -description = "Apply patch with -p0 strip level" -``` - ### Removing a Section The `spec-remove-section` overlay removes an entire section from the spec, including its diff --git a/internal/app/azldev/core/sources/archiveoverlays.go b/internal/app/azldev/core/sources/archiveoverlays.go new file mode 100644 index 00000000..c1907e00 --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays.go @@ -0,0 +1,231 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" +) + +// applyArchiveOverlays groups archive overlays by target archive and processes +// them in order. Multiple overlays targeting the same archive are batched into +// a single extract/modify/repack cycle. File removals inside the archive reuse +// the same machinery as loose-file overlays ([applyNonSpecOverlay]). +func applyArchiveOverlays( + dryRunnable opctx.DryRunnable, + fs opctx.FS, + eventListener opctx.EventListener, + sourcesDirPath string, + overlays []projectconfig.ComponentOverlay, +) error { + groups, err := groupOverlaysByArchive(overlays) + if err != nil { + return err + } + + if len(groups) == 0 { + return nil + } + + event := eventListener.StartEvent("Applying archive overlays", + "archives", len(groups), + "operations", len(overlays), + ) + defer event.End() + + for _, group := range groups { + if err := processArchive(dryRunnable, fs, sourcesDirPath, group); err != nil { + return fmt.Errorf("archive overlay failed for %#q:\n%w", group.archive, err) + } + } + + return nil +} + +// archiveGroup holds overlays targeting the same archive, preserving order. +type archiveGroup struct { + archive string + root string + overlays []projectconfig.ComponentOverlay +} + +// groupOverlaysByArchive groups archive overlays by their +// [projectconfig.ComponentOverlay.Archive] field, preserving insertion order +// within each group and across groups. Non-archive overlays are silently skipped. +// +// The optional [projectconfig.ComponentOverlay.ArchiveRoot] override (mirroring +// rpmbuild's `%setup -n`) is reconciled per archive: all overlays targeting the +// same archive that set it must agree, otherwise the configuration is ambiguous +// and an error is returned. +func groupOverlaysByArchive(overlays []projectconfig.ComponentOverlay) ([]archiveGroup, error) { + orderMap := make(map[string]int) + + var groups []archiveGroup + + for _, overlay := range overlays { + if !overlay.ModifiesArchive() { + continue + } + + idx, exists := orderMap[overlay.Archive] + if !exists { + idx = len(groups) + orderMap[overlay.Archive] = idx + + groups = append(groups, archiveGroup{archive: overlay.Archive}) + } + + if overlay.ArchiveRoot != "" { + if groups[idx].root != "" && groups[idx].root != overlay.ArchiveRoot { + return nil, fmt.Errorf( + "conflicting %#q overrides for archive %#q: %#q vs %#q", + "archive-root", overlay.Archive, groups[idx].root, overlay.ArchiveRoot, + ) + } + + groups[idx].root = overlay.ArchiveRoot + } + + groups[idx].overlays = append(groups[idx].overlays, overlay) + } + + return groups, nil +} + +// processArchive extracts an archive to a temp directory, applies all overlays, +// and deterministically repacks it in-place with the original compression. +func processArchive( + dryRunnable opctx.DryRunnable, + fs opctx.FS, + sourcesDirPath string, + group archiveGroup, +) error { + archivePath := filepath.Join(sourcesDirPath, group.archive) + + // Create a temporary directory for extraction. The injected FS is real-filesystem + // backed in production, so the returned path is a genuine on-disk path usable by + // the [archive] package. + workDir, err := fileutils.MkdirTempInTempDir(fs, "archive-overlay-") + if err != nil { + return fmt.Errorf("creating temp directory:\n%w", err) + } + + defer func() { + if removeErr := fs.RemoveAll(workDir); removeErr != nil { + slog.Warn("Failed to clean up archive work directory", "error", removeErr) + } + }() + + // Extract the archive; compression is inferred from the filename extension. + if err := archive.ExtractAuto(archivePath, workDir); err != nil { + return fmt.Errorf("extracting archive:\n%w", err) + } + + // Determine the root of the extracted content. Most source archives have + // a single top-level directory (e.g., "pkg-1.0/"); group.root overrides this + // inference when set (mirrors rpmbuild's `%setup -n`). + extractRoot, err := resolveExtractRoot(workDir, group.root) + if err != nil { + return fmt.Errorf("resolving extract root:\n%w", err) + } + + // Confine an FS to the extract root so file overlays reuse the same machinery + // as loose-file overlays. The extracted tree is always on the real filesystem + // (written by the [archive] package), so root it on an OS-backed FS regardless + // of the injected fs implementation. + extractFS, err := rootfs.New(extractRoot) + if err != nil { + return fmt.Errorf("confining FS to extract root:\n%w", err) + } + + defer func() { + if closeErr := extractFS.Close(); closeErr != nil { + slog.Warn("Failed to close extract-root FS", "error", closeErr) + } + }() + + // Apply each overlay operation in order. Archive overlays are file removals + // scoped to the extracted tree, routed through the shared loose-file machinery. + for _, overlay := range group.overlays { + if err := applyNonSpecOverlay(dryRunnable, fs, extractFS, overlay); err != nil { + return fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) + } + } + + // Deterministically repack the archive in-place, reusing the original compression. + if err := archive.CreateDeterministicArchiveAuto(archivePath, workDir); err != nil { + return fmt.Errorf("repacking archive:\n%w", err) + } + + slog.Info("Archive overlay applied", "archive", group.archive) + + return nil +} + +// resolveExtractRoot returns the effective root of an extracted archive. +// When rootOverride is set (the `%setup -n` equivalent), the named subdirectory +// of workDir is used; it must be a local path that exists as a directory. When +// rootOverride is empty, the root is inferred: if workDir contains exactly one +// entry and that entry is a directory (the common case for source archives like +// "pkg-1.0/"), that subdirectory is returned; otherwise workDir itself is +// returned. +func resolveExtractRoot(workDir, rootOverride string) (string, error) { + if rootOverride != "" { + // Defense in depth: validation already rejects non-local overrides, but + // re-check before joining so a malformed value can never escape workDir. + if !filepath.IsLocal(rootOverride) { + return "", fmt.Errorf("archive root %#q is not a local path", rootOverride) + } + + target := filepath.Join(workDir, rootOverride) + + info, err := os.Stat(target) + if err != nil { + return "", fmt.Errorf("archive root %#q not found after extraction:\n%w", rootOverride, err) + } + + if !info.IsDir() { + return "", fmt.Errorf("archive root %#q is not a directory", rootOverride) + } + + return target, nil + } + + entries, err := os.ReadDir(workDir) + if err != nil { + return "", fmt.Errorf("reading extracted directory:\n%w", err) + } + + if len(entries) == 1 && entries[0].IsDir() { + return filepath.Join(workDir, entries[0].Name()), nil + } + + return workDir, nil +} + +// archiveNamesFromOverlays returns the unique archive filenames targeted by +// archive overlays in the given overlay list. Used by [updateSourcesFile] to +// determine which 'sources' entries need rehashing after overlay application. +func archiveNamesFromOverlays(overlays []projectconfig.ComponentOverlay) []string { + seen := make(map[string]bool) + + var names []string + + for _, overlay := range overlays { + if overlay.ModifiesArchive() && !seen[overlay.Archive] { + seen[overlay.Archive] = true + names = append(names, overlay.Archive) + } + } + + return names +} diff --git a/internal/app/azldev/core/sources/tarballoverlays_internal_test.go b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go similarity index 62% rename from internal/app/azldev/core/sources/tarballoverlays_internal_test.go rename to internal/app/azldev/core/sources/archiveoverlays_internal_test.go index 45acbcf8..1a188f8f 100644 --- a/internal/app/azldev/core/sources/tarballoverlays_internal_test.go +++ b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go @@ -4,7 +4,6 @@ package sources import ( - "context" "os" "testing" @@ -16,98 +15,96 @@ import ( "github.com/stretchr/testify/require" ) -func TestGroupOverlaysByTarball(t *testing.T) { - t.Run("groups overlays by tarball name preserving order", func(t *testing.T) { +func TestGroupOverlaysByArchive(t *testing.T) { + t.Run("groups overlays by archive name preserving order", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "unwanted.conf", }, { - Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg-1.0.tar.gz", - Filename: "config.h", - Regex: "old", - Replacement: "new", + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "config.h", }, { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "other-2.0.tar.xz", + Archive: "other-2.0.tar.xz", Filename: "docs/*.md", }, } - groups, err := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByArchive(overlays) require.NoError(t, err) require.Len(t, groups, 2) - assert.Equal(t, "pkg-1.0.tar.gz", groups[0].tarball) + assert.Equal(t, "pkg-1.0.tar.gz", groups[0].archive) require.Len(t, groups[0].overlays, 2) - assert.Equal(t, projectconfig.ComponentOverlayRemoveFile, groups[0].overlays[0].Type) - assert.Equal(t, projectconfig.ComponentOverlaySearchAndReplaceInFile, groups[0].overlays[1].Type) + assert.Equal(t, "unwanted.conf", groups[0].overlays[0].Filename) + assert.Equal(t, "config.h", groups[0].overlays[1].Filename) - assert.Equal(t, "other-2.0.tar.xz", groups[1].tarball) + assert.Equal(t, "other-2.0.tar.xz", groups[1].archive) require.Len(t, groups[1].overlays, 1) }) t.Run("skips overlays that are not archive-scoped", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ {Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0"}, - {Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg.tar.gz", Filename: "f"}, - // Plain (non-archive) file overlay: no Tarball set, so it must be skipped. + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg.tar.gz", Filename: "f"}, + // Plain (non-archive) file overlay: no Archive set, so it must be skipped. {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "loose.txt"}, {Type: projectconfig.ComponentOverlayAddFile, Filename: "new.txt", Source: "src"}, } - groups, err := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByArchive(overlays) require.NoError(t, err) require.Len(t, groups, 1) - assert.Equal(t, "pkg.tar.gz", groups[0].tarball) + assert.Equal(t, "pkg.tar.gz", groups[0].archive) require.Len(t, groups[0].overlays, 1) }) - t.Run("reconciles matching tarball-root overrides", func(t *testing.T) { + t.Run("reconciles matching archive-root overrides", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", - TarballRoot: "custom-root", + Archive: "pkg-1.0.tar.gz", + ArchiveRoot: "custom-root", Filename: "a.conf", }, { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "b.conf", }, } - groups, err := groupOverlaysByTarball(overlays) + groups, err := groupOverlaysByArchive(overlays) require.NoError(t, err) require.Len(t, groups, 1) assert.Equal(t, "custom-root", groups[0].root) }) - t.Run("errors on conflicting tarball-root overrides", func(t *testing.T) { + t.Run("errors on conflicting archive-root overrides", func(t *testing.T) { overlays := []projectconfig.ComponentOverlay{ { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", - TarballRoot: "root-a", + Archive: "pkg-1.0.tar.gz", + ArchiveRoot: "root-a", Filename: "a.conf", }, { Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", - TarballRoot: "root-b", + Archive: "pkg-1.0.tar.gz", + ArchiveRoot: "root-b", Filename: "b.conf", }, } - _, err := groupOverlaysByTarball(overlays) + _, err := groupOverlaysByArchive(overlays) require.Error(t, err) assert.Contains(t, err.Error(), "conflicting") }) @@ -170,13 +167,13 @@ func TestResolveExtractRoot(t *testing.T) { }) } -// TestApplyTarballOperation_FileOverlays verifies that archive-scoped file -// overlays are routed through the shared [applyNonSpecOverlay] machinery against -// the extract-root FS (i.e., the same code path as loose-file overlays). -func TestApplyTarballOperation_FileOverlays(t *testing.T) { +// TestArchiveFileRemove verifies that archive-scoped file-remove overlays are +// routed through the shared [applyNonSpecOverlay] machinery against the +// extract-root FS (i.e., the same code path that [processArchive] uses). +func TestArchiveFileRemove(t *testing.T) { ctx := testctx.NewCtx() - t.Run("file-remove deletes matching files in the extracted tree", func(t *testing.T) { + t.Run("deletes matching files in the extracted tree", func(t *testing.T) { extractRoot := t.TempDir() require.NoError(t, os.WriteFile(extractRoot+"/keep.txt", []byte("keep"), fileperms.PrivateFile)) require.NoError(t, os.WriteFile(extractRoot+"/remove.conf", []byte("x"), fileperms.PrivateFile)) @@ -188,44 +185,18 @@ func TestApplyTarballOperation_FileOverlays(t *testing.T) { overlay := projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg.tar.gz", + Archive: "pkg.tar.gz", Filename: "*.conf", } - err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) + err = applyNonSpecOverlay(ctx, ctx.FS(), extractFS, overlay) require.NoError(t, err) assert.FileExists(t, extractRoot+"/keep.txt") assert.NoFileExists(t, extractRoot+"/remove.conf") }) - t.Run("file-search-replace rewrites matching content in the extracted tree", func(t *testing.T) { - extractRoot := t.TempDir() - require.NoError(t, os.WriteFile( - extractRoot+"/config.h", []byte("#define OLD_VALUE 1\n"), fileperms.PrivateFile)) - - extractFS, err := rootfs.New(extractRoot) - require.NoError(t, err) - - defer extractFS.Close() - - overlay := projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg.tar.gz", - Filename: "config.h", - Regex: "OLD_VALUE", - Replacement: "NEW_VALUE", - } - - err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) - require.NoError(t, err) - - content, readErr := os.ReadFile(extractRoot + "/config.h") - require.NoError(t, readErr) - assert.Equal(t, "#define NEW_VALUE 1\n", string(content)) - }) - - t.Run("file-remove with no match errors like a loose-file overlay", func(t *testing.T) { + t.Run("with no match errors like a loose-file overlay", func(t *testing.T) { extractRoot := t.TempDir() require.NoError(t, os.WriteFile(extractRoot+"/file.txt", nil, fileperms.PrivateFile)) @@ -236,11 +207,11 @@ func TestApplyTarballOperation_FileOverlays(t *testing.T) { overlay := projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg.tar.gz", + Archive: "pkg.tar.gz", Filename: "*.conf", } - err = applyTarballOperation(context.Background(), nil, ctx, ctx.FS(), extractFS, extractRoot, overlay) + err = applyNonSpecOverlay(ctx, ctx.FS(), extractFS, overlay) require.Error(t, err) assert.Contains(t, err.Error(), "did not match any files") }) diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 1bbf7cf6..a759904b 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -246,7 +246,7 @@ func (p *sourcePreparerImpl) PrepareSources( } if applyOverlays { - if err := p.applyOverlaysToSources(ctx, component, outputDir); err != nil { + if err := p.applyOverlaysToSources(component, outputDir); err != nil { return err } @@ -273,7 +273,7 @@ func (p *sourcePreparerImpl) PrepareSources( // applyOverlaysToSources writes the macros file and then applies all overlays. func (p *sourcePreparerImpl) applyOverlaysToSources( - ctx context.Context, component components.Component, outputDir string, + component components.Component, outputDir string, ) error { var macrosFileName string @@ -287,7 +287,7 @@ func (p *sourcePreparerImpl) applyOverlaysToSources( macrosFileName = filepath.Base(macrosFilePath) } - if err := p.applyOverlays(ctx, component, outputDir, macrosFileName); err != nil { + if err := p.applyOverlays(component, outputDir, macrosFileName); err != nil { return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } @@ -298,7 +298,7 @@ func (p *sourcePreparerImpl) applyOverlaysToSources( // applyOverlays applies all overlays (user-defined and system-generated) to the // component sources. func (p *sourcePreparerImpl) applyOverlays( - ctx context.Context, component components.Component, sourcesDirPath, macrosFileName string, + component components.Component, sourcesDirPath, macrosFileName string, ) error { event := p.eventListener.StartEvent("Applying overlays", "component", component.GetName()) defer event.End() @@ -317,10 +317,10 @@ func (p *sourcePreparerImpl) applyOverlays( return nil } - // Tarball overlays are applied first (they modify archived source files + // Archive overlays are applied first (they modify archived source files // in-place), followed by spec and loose-file overlays. Each function // self-filters to the overlay types it handles. - if err := p.applyTarballOverlayGroup(ctx, component, sourcesDirPath, allOverlays); err != nil { + if err := p.applyArchiveOverlayGroup(component, sourcesDirPath, allOverlays); err != nil { return err } @@ -331,34 +331,28 @@ func (p *sourcePreparerImpl) applyOverlays( return nil } -// applyTarballOverlayGroup applies tarball overlays. Skipped when source +// applyArchiveOverlayGroup applies archive overlays. Skipped when source // downloads were not performed. -func (p *sourcePreparerImpl) applyTarballOverlayGroup( - ctx context.Context, component components.Component, - sourcesDirPath string, tarballOverlays []projectconfig.ComponentOverlay, +func (p *sourcePreparerImpl) applyArchiveOverlayGroup( + component components.Component, + sourcesDirPath string, archiveOverlays []projectconfig.ComponentOverlay, ) error { - if len(tarballOverlays) == 0 { + if len(archiveOverlays) == 0 { return nil } if p.skipLookaside { - slog.Warn("Skipping tarball overlays because source downloads were skipped (--skip-sources)", + slog.Warn("Skipping archive overlays because source downloads were skipped (--skip-sources)", "component", component.GetName(), - "count", len(tarballOverlays)) + "count", len(archiveOverlays)) return nil } - cmdFactory, ok := p.dryRunnable.(opctx.CmdFactory) - if !ok { - return errors.New( - "tarball overlays require a CmdFactory; the provided DryRunnable does not implement it") - } - - if err := applyTarballOverlays( - ctx, cmdFactory, p.dryRunnable, p.fs, p.eventListener, sourcesDirPath, tarballOverlays, + if err := applyArchiveOverlays( + p.dryRunnable, p.fs, p.eventListener, sourcesDirPath, archiveOverlays, ); err != nil { - return fmt.Errorf("failed to apply tarball overlays for component %#q:\n%w", + return fmt.Errorf("failed to apply archive overlays for component %#q:\n%w", component.GetName(), err) } @@ -636,7 +630,7 @@ func (p *sourcePreparerImpl) DiffSources( } // Apply overlays in-place to the copied directory only. - if err := p.applyOverlaysToSources(ctx, component, overlaidDir); err != nil { + if err := p.applyOverlaysToSources(component, overlaidDir); err != nil { return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } @@ -671,11 +665,11 @@ func (p *sourcePreparerImpl) updateSourcesFile( config := component.GetConfig() sourceFiles := config.SourceFiles - // Derive tarball names from the component's overlays — no need to thread + // Derive archive names from the component's overlays — no need to thread // them through the overlay application chain. - modifiedTarballs := tarballNamesFromOverlays(config.Overlays) + modifiedArchives := archiveNamesFromOverlays(config.Overlays) - if len(sourceFiles) == 0 && len(modifiedTarballs) == 0 { + if len(sourceFiles) == 0 && len(modifiedArchives) == 0 { return nil } @@ -686,15 +680,15 @@ func (p *sourcePreparerImpl) updateSourcesFile( return err } - // Parse once, then rehash modified tarballs and merge source-files entries + // Parse once, then rehash modified archives and merge source-files entries // on the parsed representation — single parse, single write. existingLines, err := fedorasource.ReadSourcesFile(existingContent) if err != nil { return fmt.Errorf("failed to parse 'sources' file %#q:\n%w", sourcesFilePath, err) } - // Rehash tarballs that were modified by tarball overlays in-place. - if err := p.rehashModifiedEntries(existingLines, outputDir, modifiedTarballs); err != nil { + // Rehash archives that were modified by archive overlays in-place. + if err := p.rehashModifiedEntries(existingLines, outputDir, modifiedArchives); err != nil { return err } @@ -718,17 +712,17 @@ func (p *sourcePreparerImpl) updateSourcesFile( } // rehashModifiedEntries updates the Raw and Entry fields of parsed 'sources' lines -// for tarballs that were modified by tarball overlays. The hash is recomputed using +// for archives that were modified by archive overlays. The hash is recomputed using // the same hash type as the original entry. func (p *sourcePreparerImpl) rehashModifiedEntries( - lines []fedorasource.SourcesFileLine, outputDir string, modifiedTarballs []string, + lines []fedorasource.SourcesFileLine, outputDir string, modifiedArchives []string, ) error { - if len(modifiedTarballs) == 0 { + if len(modifiedArchives) == 0 { return nil } - modified := make(map[string]bool, len(modifiedTarballs)) - for _, name := range modifiedTarballs { + modified := make(map[string]bool, len(modifiedArchives)) + for _, name := range modifiedArchives { modified[name] = true } @@ -737,15 +731,15 @@ func (p *sourcePreparerImpl) rehashModifiedEntries( continue } - tarballPath := filepath.Join(outputDir, line.Entry.Filename) + archivePath := filepath.Join(outputDir, line.Entry.Filename) - newHash, err := fileutils.ComputeFileHash(p.fs, line.Entry.HashType, tarballPath) + newHash, err := fileutils.ComputeFileHash(p.fs, line.Entry.HashType, archivePath) if err != nil { - return fmt.Errorf("rehashing modified tarball %#q:\n%w", line.Entry.Filename, err) + return fmt.Errorf("rehashing modified archive %#q:\n%w", line.Entry.Filename, err) } - slog.Debug("Rehashed modified tarball in 'sources' file", - "tarball", line.Entry.Filename, + slog.Debug("Rehashed modified archive in 'sources' file", + "archive", line.Entry.Filename, "hashType", line.Entry.HashType, "oldHash", line.Entry.Hash, "newHash", newHash, @@ -1169,14 +1163,14 @@ func (p *sourcePreparerImpl) resolveSpecPath( } // applyOverlayList applies a list of overlays to the component sources sequentially. -// Archive-scoped overlays (see [projectconfig.ComponentOverlay.ModifiesTarball]) are -// skipped here; they are handled separately by [applyTarballOverlays], which batches +// Archive-scoped overlays (see [projectconfig.ComponentOverlay.ModifiesArchive]) are +// skipped here; they are handled separately by [applyArchiveOverlays], which batches // extraction and repacking per archive. func (p *sourcePreparerImpl) applyOverlayList( overlays []projectconfig.ComponentOverlay, sourcesDirPath, absSpecPath string, ) error { for _, overlay := range overlays { - if overlay.ModifiesTarball() { + if overlay.ModifiesArchive() { continue } diff --git a/internal/app/azldev/core/sources/tarballoverlays.go b/internal/app/azldev/core/sources/tarballoverlays.go deleted file mode 100644 index bbf6d4d0..00000000 --- a/internal/app/azldev/core/sources/tarballoverlays.go +++ /dev/null @@ -1,332 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package sources - -import ( - "context" - "errors" - "fmt" - "log/slog" - "os" - "os/exec" - "path/filepath" - "strconv" - "strings" - - "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" - "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" -) - -// applyTarballOverlays groups tarball overlays by target archive and processes -// them in order. Multiple overlays targeting the same tarball are batched into -// a single extract/modify/repack cycle. File modifications inside the archive -// reuse the same machinery as loose-file overlays ([applyNonSpecOverlay]); patch -// application shells out to the host's `patch` command. -func applyTarballOverlays( - ctx context.Context, - cmdFactory opctx.CmdFactory, - dryRunnable opctx.DryRunnable, - fs opctx.FS, - eventListener opctx.EventListener, - sourcesDirPath string, - overlays []projectconfig.ComponentOverlay, -) error { - groups, err := groupOverlaysByTarball(overlays) - if err != nil { - return err - } - - if len(groups) == 0 { - return nil - } - - event := eventListener.StartEvent("Applying tarball overlays", - "tarballs", len(groups), - "operations", len(overlays), - ) - defer event.End() - - for _, group := range groups { - if err := processTarball(ctx, cmdFactory, dryRunnable, fs, sourcesDirPath, group); err != nil { - return fmt.Errorf("tarball overlay failed for %#q:\n%w", group.tarball, err) - } - } - - return nil -} - -// tarballGroup holds overlays targeting the same tarball, preserving order. -type tarballGroup struct { - tarball string - root string - overlays []projectconfig.ComponentOverlay -} - -// groupOverlaysByTarball groups tarball overlays by their -// [projectconfig.ComponentOverlay.Tarball] field, preserving insertion order -// within each group and across groups. Non-tarball overlays are silently skipped. -// -// The optional [projectconfig.ComponentOverlay.TarballRoot] override (mirroring -// rpmbuild's `%setup -n`) is reconciled per tarball: all overlays targeting the -// same archive that set it must agree, otherwise the configuration is ambiguous -// and an error is returned. -func groupOverlaysByTarball(overlays []projectconfig.ComponentOverlay) ([]tarballGroup, error) { - orderMap := make(map[string]int) - - var groups []tarballGroup - - for _, overlay := range overlays { - if !overlay.ModifiesTarball() { - continue - } - - idx, exists := orderMap[overlay.Tarball] - if !exists { - idx = len(groups) - orderMap[overlay.Tarball] = idx - - groups = append(groups, tarballGroup{tarball: overlay.Tarball}) - } - - if overlay.TarballRoot != "" { - if groups[idx].root != "" && groups[idx].root != overlay.TarballRoot { - return nil, fmt.Errorf( - "conflicting %#q overrides for tarball %#q: %#q vs %#q", - "tarball-root", overlay.Tarball, groups[idx].root, overlay.TarballRoot, - ) - } - - groups[idx].root = overlay.TarballRoot - } - - groups[idx].overlays = append(groups[idx].overlays, overlay) - } - - return groups, nil -} - -// processTarball extracts a tarball to a temp directory, applies all overlays, -// and deterministically repacks it in-place with the original compression. -func processTarball( - ctx context.Context, - cmdFactory opctx.CmdFactory, - dryRunnable opctx.DryRunnable, - fs opctx.FS, - sourcesDirPath string, - group tarballGroup, -) error { - archivePath := filepath.Join(sourcesDirPath, group.tarball) - - // Create a temporary directory for extraction. The injected FS is real-filesystem - // backed in production, so the returned path is a genuine on-disk path usable by - // the [archive] package and the host `patch` command. - workDir, err := fileutils.MkdirTempInTempDir(fs, "tarball-overlay-") - if err != nil { - return fmt.Errorf("creating temp directory:\n%w", err) - } - - defer func() { - if removeErr := fs.RemoveAll(workDir); removeErr != nil { - slog.Warn("Failed to clean up tarball work directory", "error", removeErr) - } - }() - - // Extract the archive; compression is inferred from the filename extension. - if err := archive.ExtractAuto(archivePath, workDir); err != nil { - return fmt.Errorf("extracting tarball:\n%w", err) - } - - // Determine the root of the extracted content. Most source tarballs have - // a single top-level directory (e.g., "pkg-1.0/"); group.root overrides this - // inference when set (mirrors rpmbuild's `%setup -n`). - extractRoot, err := resolveExtractRoot(workDir, group.root) - if err != nil { - return fmt.Errorf("resolving extract root:\n%w", err) - } - - // Confine an FS to the extract root so file overlays reuse the same machinery - // as loose-file overlays. The extracted tree is always on the real filesystem - // (written by the [archive] package), so root it on an OS-backed FS regardless - // of the injected fs implementation. - extractFS, err := rootfs.New(extractRoot) - if err != nil { - return fmt.Errorf("confining FS to extract root:\n%w", err) - } - - defer func() { - if closeErr := extractFS.Close(); closeErr != nil { - slog.Warn("Failed to close extract-root FS", "error", closeErr) - } - }() - - // Apply each overlay operation in order. - for _, overlay := range group.overlays { - if err := applyTarballOperation( - ctx, cmdFactory, dryRunnable, fs, extractFS, extractRoot, overlay, - ); err != nil { - return fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) - } - } - - // Deterministically repack the tarball in-place, reusing the original compression. - if err := archive.CreateDeterministicArchiveAuto(archivePath, workDir); err != nil { - return fmt.Errorf("repacking tarball:\n%w", err) - } - - slog.Info("Tarball overlay applied", "tarball", group.tarball) - - return nil -} - -// applyTarballOperation dispatches a single overlay against the extracted tree. -// The dedicated tarball-patch type shells out to the host `patch` command; all -// other (archive-scoped) types reuse [applyNonSpecOverlay], operating on the -// extract-root FS exactly as they would on the loose sources tree. -func applyTarballOperation( - ctx context.Context, - cmdFactory opctx.CmdFactory, - dryRunnable opctx.DryRunnable, - sourceFS, extractFS opctx.FS, - extractRoot string, - overlay projectconfig.ComponentOverlay, -) error { - if overlay.Type == projectconfig.ComponentOverlayTarballPatch { - stripLevel := 1 - - if overlay.Value != "" { - parsed, err := strconv.Atoi(overlay.Value) - if err != nil { - return fmt.Errorf("invalid strip level %#q:\n%w", overlay.Value, err) - } - - stripLevel = parsed - } - - return tarballApplyPatch(ctx, cmdFactory, sourceFS, extractRoot, overlay.Source, stripLevel) - } - - return applyNonSpecOverlay(dryRunnable, sourceFS, extractFS, overlay) -} - -// tarballApplyPatch applies a unified diff patch to the extracted tree by -// shelling out to the host's `patch` command. -func tarballApplyPatch( - ctx context.Context, - cmdFactory opctx.CmdFactory, - fs opctx.FS, - extractRoot, patchSource string, - stripLevel int, -) error { - if !cmdFactory.CommandInSearchPath("patch") { - return errors.New("'patch' command not found in PATH; " + - "install the 'patch' package to use tarball-patch overlays") - } - - // Read the patch file content via the abstract FS (supports both real and test FSs). - patchData, err := fileutils.ReadFile(fs, patchSource) - if err != nil { - return fmt.Errorf("reading patch file %#q:\n%w", patchSource, err) - } - - // Stage the patch in a temp dir so the host `patch` command can read it. The - // injected FS is real-filesystem backed in production, so the path is on disk. - tmpDir, err := fileutils.MkdirTempInTempDir(fs, "tarball-patch-") - if err != nil { - return fmt.Errorf("creating temp patch directory:\n%w", err) - } - - defer func() { - if removeErr := fs.RemoveAll(tmpDir); removeErr != nil { - slog.Warn("Failed to clean up tarball patch directory", "error", removeErr) - } - }() - - tmpPatchPath := filepath.Join(tmpDir, "overlay.patch") - if err := fileutils.WriteFile(fs, tmpPatchPath, patchData, fileperms.PublicFile); err != nil { - return fmt.Errorf("writing temp patch file:\n%w", err) - } - - var stderr strings.Builder - - rawCmd := exec.CommandContext(ctx, "patch", - fmt.Sprintf("-p%d", stripLevel), - "-i", tmpPatchPath, - ) - rawCmd.Dir = extractRoot - rawCmd.Stderr = &stderr - - cmd, err := cmdFactory.Command(rawCmd) - if err != nil { - return fmt.Errorf("creating patch command:\n%w", err) - } - - if runErr := cmd.Run(ctx); runErr != nil { - return fmt.Errorf("patch failed:\n%s\n%w", stderr.String(), runErr) - } - - return nil -} - -// resolveExtractRoot returns the effective root of an extracted tarball. -// When rootOverride is set (the `%setup -n` equivalent), the named subdirectory -// of workDir is used; it must be a local path that exists as a directory. When -// rootOverride is empty, the root is inferred: if workDir contains exactly one -// entry and that entry is a directory (the common case for source tarballs like -// "pkg-1.0/"), that subdirectory is returned; otherwise workDir itself is -// returned. -func resolveExtractRoot(workDir, rootOverride string) (string, error) { - if rootOverride != "" { - // Defense in depth: validation already rejects non-local overrides, but - // re-check before joining so a malformed value can never escape workDir. - if !filepath.IsLocal(rootOverride) { - return "", fmt.Errorf("tarball root %#q is not a local path", rootOverride) - } - - target := filepath.Join(workDir, rootOverride) - - info, err := os.Stat(target) - if err != nil { - return "", fmt.Errorf("tarball root %#q not found after extraction:\n%w", rootOverride, err) - } - - if !info.IsDir() { - return "", fmt.Errorf("tarball root %#q is not a directory", rootOverride) - } - - return target, nil - } - - entries, err := os.ReadDir(workDir) - if err != nil { - return "", fmt.Errorf("reading extracted directory:\n%w", err) - } - - if len(entries) == 1 && entries[0].IsDir() { - return filepath.Join(workDir, entries[0].Name()), nil - } - - return workDir, nil -} - -// tarballNamesFromOverlays returns the unique tarball filenames targeted by -// tarball overlays in the given overlay list. Used by [updateSourcesFile] to -// determine which 'sources' entries need rehashing after overlay application. -func tarballNamesFromOverlays(overlays []projectconfig.ComponentOverlay) []string { - seen := make(map[string]bool) - - var names []string - - for _, overlay := range overlays { - if overlay.ModifiesTarball() && !seen[overlay.Tarball] { - seen[overlay.Tarball] = true - names = append(names, overlay.Tarball) - } - } - - return names -} diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 5b20d894..57bc0926 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -17,21 +17,21 @@ import ( // ComponentOverlay represents an overlay that may be applied to a component's spec and/or its sources. type ComponentOverlay struct { // The type of overlay to apply. - Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,enum=tarball-patch,title=Overlay type,description=The type of overlay to apply"` + Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"` // Human readable description of overlay; primarily present to document the need for the change. Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay" fingerprint:"-"` - // For overlays that target files inside a source tarball, identifies the tarball to modify. + // For overlays that target files inside a source archive, identifies the archive to modify. // Must be a filename (not a path) matching a source archive in the component's sources directory. - // Required for tarball-patch; optional for file-remove and file-search-replace, which operate - // inside the named archive instead of the loose sources tree when it is set. - Tarball string `toml:"tarball,omitempty" json:"tarball,omitempty" jsonschema:"title=Tarball,description=The source tarball to modify (e.g. pkg-1.0.tar.gz)"` - // For overlays that target files inside a source tarball, optionally overrides the top-level + // Only file-remove supports archive scoping; when set, it removes file(s) from inside the named + // archive instead of the loose sources tree. + Archive string `toml:"archive,omitempty" json:"archive,omitempty" jsonschema:"title=Archive,description=The source archive to modify (e.g. pkg-1.0.tar.gz)"` + // For overlays that target files inside a source archive, optionally overrides the top-level // directory to treat as the extraction root, mirroring rpmbuild's `%setup -n`. When unset, the // root is inferred: if the archive unpacks to a single top-level directory (the conventional // `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. // Set this when an archive's top-level directory does not follow that convention. - TarballRoot string `toml:"tarball-root,omitempty" json:"tarballRoot,omitempty" jsonschema:"title=Tarball root,description=Top-level directory inside the tarball to treat as the extraction root (mirrors %setup -n); inferred when unset"` + ArchiveRoot string `toml:"archive-root,omitempty" json:"archiveRoot,omitempty" jsonschema:"title=Archive root,description=Top-level directory inside the archive to treat as the extraction root (mirrors %setup -n); inferred when unset"` // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). Filename string `toml:"file,omitempty" json:"file,omitempty" jsonschema:"title=Filename,description=The name of the non-spec file to which this overlay applies, or a glob pattern matching multiple files"` @@ -130,25 +130,21 @@ func (c *ComponentOverlay) ModifiesSpec() bool { c.Type == ComponentOverlayRemovePatch } -// ModifiesTarball returns true if the overlay modifies files inside a source tarball. -// These overlays require extraction and repacking of the archive. The dedicated -// tarball-patch type is always archive-scoped; the file-remove and file-search-replace -// types become archive-scoped when their [ComponentOverlay.Tarball] field is set. -func (c *ComponentOverlay) ModifiesTarball() bool { - if c.Type == ComponentOverlayTarballPatch { - return true - } - - return c.Tarball != "" && +// ModifiesArchive returns true if the overlay modifies files inside a source archive. +// These overlays require extraction and repacking of the archive. Only file-remove and +// file-search-replace support archive scoping, and only when their +// [ComponentOverlay.Archive] field is set. +func (c *ComponentOverlay) ModifiesArchive() bool { + return c.Archive != "" && (c.Type == ComponentOverlayRemoveFile || c.Type == ComponentOverlaySearchAndReplaceInFile) } // ModifiesNonSpecFiles returns true if the overlay modifies non-spec files. This includes // hybrid overlays that modify both spec and source files (e.g., patch overlays), since -// those also require non-spec modifications. Archive-scoped overlays (see [ModifiesTarball]) -// are excluded: they operate on files inside a tarball, not loose files in the sources tree. +// those also require non-spec modifications. Archive-scoped overlays (see [ModifiesArchive]) +// are excluded: they operate on files inside an archive, not loose files in the sources tree. func (c *ComponentOverlay) ModifiesNonSpecFiles() bool { - if c.ModifiesTarball() { + if c.ModifiesArchive() { return false } @@ -204,20 +200,15 @@ const ( // ComponentOverlayPrependLinesToFile is an overlay that prepends lines to a non-spec file. ComponentOverlayPrependLinesToFile ComponentOverlayType = "file-prepend-lines" // ComponentOverlaySearchAndReplaceInFile is an overlay that replaces text in a non-spec file. - // When its [ComponentOverlay.Tarball] field is set, it operates on file(s) inside that source - // tarball instead of loose files in the sources tree. ComponentOverlaySearchAndReplaceInFile ComponentOverlayType = "file-search-replace" // ComponentOverlayAddFile is an overlay that adds a non-spec file. ComponentOverlayAddFile ComponentOverlayType = "file-add" // ComponentOverlayRemoveFile is an overlay that removes a non-spec file. When its - // [ComponentOverlay.Tarball] field is set, it removes file(s) from inside that source - // tarball instead of loose files in the sources tree. + // [ComponentOverlay.Archive] field is set, it removes file(s) from inside that source + // archive instead of loose files in the sources tree. ComponentOverlayRemoveFile ComponentOverlayType = "file-remove" // ComponentOverlayRenameFile is an overlay that renames a non-spec file. ComponentOverlayRenameFile ComponentOverlayType = "file-rename" - // ComponentOverlayTarballPatch is an overlay that applies a unified diff patch to the - // extracted contents of a source tarball. - ComponentOverlayTarballPatch ComponentOverlayType = "tarball-patch" ) // Validate checks that required fields are set based on the overlay type. This catches @@ -268,31 +259,29 @@ func (c *ComponentOverlay) Validate() error { return nil } - // The tarball field scopes an overlay to operate inside a source archive. It is only - // accepted on the archive-capable types and must be a bare filename (not a path). - if c.Tarball != "" { - //nolint:exhaustive // Only archive-capable types accept the tarball field; the default rejects the rest. - switch c.Type { - case ComponentOverlayRemoveFile, ComponentOverlaySearchAndReplaceInFile, ComponentOverlayTarballPatch: - if err := requireFileBasename("tarball", c.Tarball); err != nil { - return err - } - default: - return unexpectedField("tarball") + // The archive field scopes an overlay to operate inside a source archive. It is only + // accepted on file-remove and file-search-replace, and must be a bare filename (not a path). + if c.Archive != "" { + if c.Type != ComponentOverlayRemoveFile && c.Type != ComponentOverlaySearchAndReplaceInFile { + return unexpectedField("archive") + } + + if err := requireFileBasename("archive", c.Archive); err != nil { + return err } } - // The tarball-root override is only meaningful for archive-scoped overlays, and must be a + // The archive-root override is only meaningful for archive-scoped overlays, and must be a // local relative path so it cannot escape the extraction directory. - if c.TarballRoot != "" { - if !c.ModifiesTarball() { - return unexpectedField("tarball-root") + if c.ArchiveRoot != "" { + if !c.ModifiesArchive() { + return unexpectedField("archive-root") } - if !filepath.IsLocal(c.TarballRoot) { + if !filepath.IsLocal(c.ArchiveRoot) { return fmt.Errorf( "overlay type %#q requires %#q to be a local relative path (no %#q or absolute paths); found %#q", - c.Type, "tarball-root", "..", c.TarballRoot, + c.Type, "archive-root", "..", c.ArchiveRoot, ) } } @@ -394,14 +383,6 @@ func (c *ComponentOverlay) Validate() error { if err := validateGlobPattern(c.Filename, desc); err != nil { return err } - case ComponentOverlayTarballPatch: - if err := requireFileBasename("tarball", c.Tarball); err != nil { - return err - } - - if c.Source == "" { - return missingField("source") - } default: return fmt.Errorf("unknown overlay type %#q: %#q", c.Type, desc) } diff --git a/internal/projectconfig/overlay_test.go b/internal/projectconfig/overlay_test.go index 1a760a7a..ece5a574 100644 --- a/internal/projectconfig/overlay_test.go +++ b/internal/projectconfig/overlay_test.go @@ -412,27 +412,27 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: true, errorContains: "section", }, - // archive-scoped file-remove tests (tarball modifier) + // archive-scoped file-remove tests (archive modifier) { - name: "file-remove with tarball valid (archive-scoped)", + name: "file-remove with archive valid (archive-scoped)", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "unwanted.conf", }, errorExpected: false, }, { - name: "file-remove with tarball glob valid", + name: "file-remove with archive glob valid", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "docs/**/*.md", }, errorExpected: false, }, { - name: "file-remove without tarball is a plain loose-file remove", + name: "file-remove without archive is a plain loose-file remove", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, Filename: "unwanted.conf", @@ -440,160 +440,89 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: false, }, { - name: "file-remove with tarball missing file", + name: "file-remove with archive missing file", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", }, errorExpected: true, errorContains: "file", }, { - name: "file-remove rejects tarball path", + name: "file-remove rejects archive path", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "subdir/pkg-1.0.tar.gz", + Archive: "subdir/pkg-1.0.tar.gz", Filename: "unwanted.conf", }, errorExpected: true, - errorContains: "tarball", + errorContains: "archive", }, { - name: "tarball rejected on overlay type that cannot be archive-scoped", + name: "archive rejected on overlay type that cannot be archive-scoped", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayAddFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "new.conf", Source: "files/new.conf", }, errorExpected: true, - errorContains: "tarball", + errorContains: "archive", }, { - name: "file-remove with tarball-root override valid", + name: "file-remove with archive-root override valid", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", - TarballRoot: "src-root", + Archive: "pkg-1.0.tar.gz", + ArchiveRoot: "src-root", Filename: "unwanted.conf", }, errorExpected: false, }, { - name: "file-remove rejects non-local tarball-root", + name: "file-remove rejects non-local archive-root", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - Tarball: "pkg-1.0.tar.gz", - TarballRoot: "../escape", + Archive: "pkg-1.0.tar.gz", + ArchiveRoot: "../escape", Filename: "unwanted.conf", }, errorExpected: true, - errorContains: "tarball-root", + errorContains: "archive-root", }, { - name: "tarball-root rejected when tarball unset", + name: "archive-root rejected when archive unset", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlayRemoveFile, - TarballRoot: "src-root", + ArchiveRoot: "src-root", Filename: "unwanted.conf", }, errorExpected: true, - errorContains: "tarball-root", + errorContains: "archive-root", }, { - name: "tarball-root rejected on non-archive overlay", + name: "archive-root rejected on non-archive overlay", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0.0", - TarballRoot: "src-root", + ArchiveRoot: "src-root", }, errorExpected: true, - errorContains: "tarball-root", + errorContains: "archive-root", }, - // archive-scoped file-search-replace tests (tarball modifier) + // file-search-replace supports archive scoping { - name: "file-search-replace with tarball valid", + name: "file-search-replace with archive valid (archive-scoped)", overlay: projectconfig.ComponentOverlay{ Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg-1.0.tar.gz", + Archive: "pkg-1.0.tar.gz", Filename: "config.h", Regex: "old_value", Replacement: "new_value", }, errorExpected: false, }, - { - name: "file-search-replace with tarball missing file", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg-1.0.tar.gz", - Regex: "old_value", - Replacement: "new_value", - }, - errorExpected: true, - errorContains: "file", - }, - { - name: "file-search-replace with tarball missing regex", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg-1.0.tar.gz", - Filename: "config.h", - }, - errorExpected: true, - errorContains: "regex", - }, - { - name: "file-search-replace with tarball invalid regex", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, - Tarball: "pkg-1.0.tar.gz", - Filename: "config.h", - Regex: "[invalid", - Replacement: "new_value", - }, - errorExpected: true, - errorContains: "regex", - }, - // tarball-patch tests - { - name: "tarball-patch valid", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballPatch, - Tarball: "pkg-1.0.tar.gz", - Source: "patches/fix.patch", - }, - errorExpected: false, - }, - { - name: "tarball-patch valid with strip level", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballPatch, - Tarball: "pkg-1.0.tar.gz", - Source: "patches/fix.patch", - Value: "2", - }, - errorExpected: false, - }, - { - name: "tarball-patch missing tarball", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballPatch, - Source: "patches/fix.patch", - }, - errorExpected: true, - errorContains: "tarball", - }, - { - name: "tarball-patch missing source", - overlay: projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayTarballPatch, - Tarball: "pkg-1.0.tar.gz", - }, - errorExpected: true, - errorContains: "source", - }, } for _, testCase := range testCases { @@ -637,12 +566,10 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { projectconfig.ComponentOverlayAddFile, } - // Archive-scoped overlays: tarball-patch is always archive-scoped, while file-remove - // and file-search-replace become archive-scoped only when their Tarball field is set. - tarballOverlays := []projectconfig.ComponentOverlay{ - {Type: projectconfig.ComponentOverlayTarballPatch, Tarball: "pkg-1.0.tar.gz", Source: "p.patch"}, - {Type: projectconfig.ComponentOverlayRemoveFile, Tarball: "pkg-1.0.tar.gz", Filename: "f"}, - {Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, Tarball: "pkg-1.0.tar.gz", Filename: "f"}, + // Archive-scoped overlays: only file-remove becomes archive-scoped, and only + // when its Archive field is set. + archiveOverlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg-1.0.tar.gz", Filename: "f"}, } for _, overlayType := range specOverlayTypes { @@ -659,9 +586,9 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { }) } - for _, overlay := range tarballOverlays { + for _, overlay := range archiveOverlays { t.Run(string(overlay.Type)+"_is_archive_scoped", func(t *testing.T) { - assert.True(t, overlay.ModifiesTarball(), "expected %s to be an archive-scoped overlay", overlay.Type) + assert.True(t, overlay.ModifiesArchive(), "expected %s to be an archive-scoped overlay", overlay.Type) assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlay.Type) assert.False(t, overlay.ModifiesNonSpecFiles(), "expected %s to not be a loose non-spec overlay", overlay.Type) diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index 7aa38d7e..9aa5a853 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -231,6 +231,16 @@ "title": "Description", "description": "Human readable description of overlay" }, + "archive": { + "type": "string", + "title": "Archive", + "description": "The source archive to modify (e.g. pkg-1.0.tar.gz)" + }, + "archive-root": { + "type": "string", + "title": "Archive root", + "description": "Top-level directory inside the archive to treat as the extraction root (mirrors %setup -n); inferred when unset" + }, "file": { "type": "string", "title": "Filename", diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index 7aa38d7e..9aa5a853 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -231,6 +231,16 @@ "title": "Description", "description": "Human readable description of overlay" }, + "archive": { + "type": "string", + "title": "Archive", + "description": "The source archive to modify (e.g. pkg-1.0.tar.gz)" + }, + "archive-root": { + "type": "string", + "title": "Archive root", + "description": "Top-level directory inside the archive to treat as the extraction root (mirrors %setup -n); inferred when unset" + }, "file": { "type": "string", "title": "Filename", diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index a8048802..9aa5a853 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -221,8 +221,7 @@ "file-search-replace", "file-add", "file-remove", - "file-rename", - "tarball-patch" + "file-rename" ], "title": "Overlay type", "description": "The type of overlay to apply" @@ -232,15 +231,15 @@ "title": "Description", "description": "Human readable description of overlay" }, - "tarball": { + "archive": { "type": "string", - "title": "Tarball", - "description": "The source tarball to modify (e.g. pkg-1.0.tar.gz)" + "title": "Archive", + "description": "The source archive to modify (e.g. pkg-1.0.tar.gz)" }, - "tarball-root": { + "archive-root": { "type": "string", - "title": "Tarball root", - "description": "Top-level directory inside the tarball to treat as the extraction root (mirrors %setup -n); inferred when unset" + "title": "Archive root", + "description": "Top-level directory inside the archive to treat as the extraction root (mirrors %setup -n); inferred when unset" }, "file": { "type": "string", From 96d9a9ce628ed1914bed484a70ac48549a4f727a Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 10 Jun 2026 20:40:51 +0000 Subject: [PATCH 4/5] Added additional tests --- docs/user/reference/config/overlays.md | 2 +- .../azldev/core/sources/archiveoverlays.go | 33 ++++-- .../app/azldev/core/sources/sourceprep.go | 14 ++- .../azldev/core/sources/sourceprep_test.go | 111 ++++++++++++++++++ internal/fingerprint/fingerprint_test.go | 39 ++++++ internal/projectconfig/fingerprint_test.go | 25 +++- internal/projectconfig/overlay.go | 14 +-- internal/projectconfig/overlay_test.go | 8 +- 8 files changed, 212 insertions(+), 34 deletions(-) diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 2fad5f7a..cc41528e 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -309,7 +309,7 @@ repacked. type = "file-remove" archive = "mypackage-1.0.tar.gz" file = "vendor/**" -description = "Remove bundled vendor directory" +description = "Remove all bundled vendor files" ``` > **Tip:** Without the `archive` field, the same `file-remove` overlay removes a loose file from diff --git a/internal/app/azldev/core/sources/archiveoverlays.go b/internal/app/azldev/core/sources/archiveoverlays.go index c1907e00..f54711ae 100644 --- a/internal/app/azldev/core/sources/archiveoverlays.go +++ b/internal/app/azldev/core/sources/archiveoverlays.go @@ -12,7 +12,6 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" - "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" ) @@ -22,7 +21,6 @@ import ( // the same machinery as loose-file overlays ([applyNonSpecOverlay]). func applyArchiveOverlays( dryRunnable opctx.DryRunnable, - fs opctx.FS, eventListener opctx.EventListener, sourcesDirPath string, overlays []projectconfig.ComponentOverlay, @@ -36,14 +34,19 @@ func applyArchiveOverlays( return nil } + operationCount := 0 + for _, group := range groups { + operationCount += len(group.overlays) + } + event := eventListener.StartEvent("Applying archive overlays", "archives", len(groups), - "operations", len(overlays), + "operations", operationCount, ) defer event.End() for _, group := range groups { - if err := processArchive(dryRunnable, fs, sourcesDirPath, group); err != nil { + if err := processArchive(dryRunnable, sourcesDirPath, group); err != nil { return fmt.Errorf("archive overlay failed for %#q:\n%w", group.archive, err) } } @@ -105,22 +108,24 @@ func groupOverlaysByArchive(overlays []projectconfig.ComponentOverlay) ([]archiv // and deterministically repacks it in-place with the original compression. func processArchive( dryRunnable opctx.DryRunnable, - fs opctx.FS, sourcesDirPath string, group archiveGroup, ) error { archivePath := filepath.Join(sourcesDirPath, group.archive) - // Create a temporary directory for extraction. The injected FS is real-filesystem - // backed in production, so the returned path is a genuine on-disk path usable by - // the [archive] package. - workDir, err := fileutils.MkdirTempInTempDir(fs, "archive-overlay-") + // Create a temporary directory for extraction directly on the real filesystem. + // The [archive] package operates exclusively through OS primitives ([os.Root], + // os.*), so the work directory must be a genuine on-disk path regardless of the + // injected FS implementation. Using os.MkdirTemp here (instead of the injected + // FS) makes that requirement explicit and keeps the path valid even when fs is + // an in-memory or otherwise non-OS-backed FS (e.g., in tests or alternate runners). + workDir, err := os.MkdirTemp("", "archive-overlay-") if err != nil { return fmt.Errorf("creating temp directory:\n%w", err) } defer func() { - if removeErr := fs.RemoveAll(workDir); removeErr != nil { + if removeErr := os.RemoveAll(workDir); removeErr != nil { slog.Warn("Failed to clean up archive work directory", "error", removeErr) } }() @@ -153,10 +158,12 @@ func processArchive( } }() - // Apply each overlay operation in order. Archive overlays are file removals - // scoped to the extracted tree, routed through the shared loose-file machinery. + // Apply each overlay operation in order. Archive overlays are restricted to + // file-remove / file-search-replace (see [projectconfig.ComponentOverlay.ModifiesArchive]), + // which operate solely on the destination tree, so the extract-root FS is passed as + // both the source and destination FS — there is no component-source FS to read from. for _, overlay := range group.overlays { - if err := applyNonSpecOverlay(dryRunnable, fs, extractFS, overlay); err != nil { + if err := applyNonSpecOverlay(dryRunnable, extractFS, extractFS, overlay); err != nil { return fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) } } diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index a759904b..526158aa 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -331,12 +331,18 @@ func (p *sourcePreparerImpl) applyOverlays( return nil } -// applyArchiveOverlayGroup applies archive overlays. Skipped when source -// downloads were not performed. +// applyArchiveOverlayGroup applies the archive-scoped overlays contained in the +// given overlay list. The list may hold overlays of any type; only those for +// which [projectconfig.ComponentOverlay.ModifiesArchive] reports true are +// processed here. Skipped when source downloads were not performed. func (p *sourcePreparerImpl) applyArchiveOverlayGroup( component components.Component, - sourcesDirPath string, archiveOverlays []projectconfig.ComponentOverlay, + sourcesDirPath string, overlays []projectconfig.ComponentOverlay, ) error { + archiveOverlays := lo.Filter(overlays, func(overlay projectconfig.ComponentOverlay, _ int) bool { + return overlay.ModifiesArchive() + }) + if len(archiveOverlays) == 0 { return nil } @@ -350,7 +356,7 @@ func (p *sourcePreparerImpl) applyArchiveOverlayGroup( } if err := applyArchiveOverlays( - p.dryRunnable, p.fs, p.eventListener, sourcesDirPath, archiveOverlays, + p.dryRunnable, p.eventListener, sourcesDirPath, archiveOverlays, ); err != nil { return fmt.Errorf("failed to apply archive overlays for component %#q:\n%w", component.GetName(), err) diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index d21aa52d..704bea69 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -5,6 +5,7 @@ package sources_test import ( "errors" + "os" "path/filepath" "strings" "testing" @@ -16,6 +17,7 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/sourceproviders_test" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" "github.com/stretchr/testify/assert" @@ -99,6 +101,115 @@ func TestPrepareSources_Success(t *testing.T) { assert.NotContains(t, string(specContents), "Source9999") } +// TestPrepareSources_ArchiveOverlayRehashesSourcesEntry is an end-to-end check +// of the key correctness behavior introduced with archive overlays: when an +// archive-scoped overlay mutates an archive's contents, the matching 'sources' +// entry must be re-hashed in place so the recorded digest reflects the repacked +// archive (while keeping the original hash *type*). +// +// This runs against the host filesystem with a real temp dir because archive +// overlays extract/repack through the [archive] package, which uses OS +// primitives ([os.Root], os.*) and therefore requires genuine on-disk paths — +// an in-memory FS would not be visible to extraction/repacking. This mirrors +// the existing archive internal tests, which likewise use t.TempDir(). +func TestPrepareSources_ArchiveOverlayRehashesSourcesEntry(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg-1.0.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + + archivePath := filepath.Join(outputDir, archiveName) + specPath := filepath.Join(outputDir, componentName+".spec") + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + + // Build a deterministic archive whose single top-level directory follows the + // conventional "%{name}-%{version}/" layout, containing a file we will remove + // and one we will keep. + stagingDir := t.TempDir() + pkgRoot := filepath.Join(stagingDir, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "keep.txt"), []byte("keep me"), fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchiveAuto(archivePath, stagingDir)) + + // Record the original hash of the archive and seed a 'sources' file with it. + // Use SHA256 (not the SHA512 default) so the test also proves the hash *type* + // is preserved rather than coincidentally matching a default. + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + originalEntry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, []byte(originalEntry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: "remove-me.txt", + }, + }, + }) + + // The archive and 'sources' file already exist on disk; the source manager + // only needs to provide the spec file (FetchFiles is a no-op download). + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + require.NoError(t, err) + + // The overlay must have actually mutated the archive on disk. + assert.FileExists(t, specPath) + + repackedHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + require.NotEqual(t, originalHash, repackedHash, + "precondition: removing a file from the archive should change its hash") + + // The 'sources' entry must have been rewritten to the repacked archive's hash, + // preserving the original SHA256 hash type. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + parsedLines, err := fedorasource.ReadSourcesFile(string(sourcesContent)) + require.NoError(t, err) + + var entry *fedorasource.SourcesFileEntry + + for i := range parsedLines { + if parsedLines[i].Entry != nil && parsedLines[i].Entry.Filename == archiveName { + entry = parsedLines[i].Entry + + break + } + } + + require.NotNil(t, entry, "rewritten 'sources' file should still contain an entry for %q", archiveName) + assert.Equal(t, fileutils.HashTypeSHA256, entry.HashType, "original hash type must be preserved") + assert.Equal(t, repackedHash, entry.Hash, "'sources' entry must record the repacked archive's hash") + assert.NotEqual(t, originalHash, entry.Hash, "'sources' entry hash must have been updated") +} + func TestPrepareSources_SourceManagerError(t *testing.T) { ctrl := gomock.NewController(t) component := components_testutils.NewMockComponent(ctrl) diff --git a/internal/fingerprint/fingerprint_test.go b/internal/fingerprint/fingerprint_test.go index 4a6cf1eb..3c748a46 100644 --- a/internal/fingerprint/fingerprint_test.go +++ b/internal/fingerprint/fingerprint_test.go @@ -251,6 +251,45 @@ func TestComputeIdentity_OverlaySourceFileChange(t *testing.T) { assert.NotEqual(t, fp1, fp2, "different overlay source content must produce different fingerprints") } +func TestComputeIdentity_OverlayArchiveScopingChangesFP(t *testing.T) { + // Archive scoping fields are hashed with `,omitempty`: an unset scope must + // not perturb the fingerprint (so the overwhelming majority of overlays stay + // idempotent against already-rendered specs), but a SET scope genuinely + // changes the build output (a file removed from inside an archive) and so + // must change the fingerprint. This test pins the second half of that + // contract — guarding against an over-correction that excludes the fields + // entirely (e.g. `fingerprint:"-"`), which would let an archive overlay slip + // through without triggering a rebuild. + ctx := newTestFS(t, map[string]string{ + "/specs/test.spec": "Name: testpkg\nVersion: 1.0", + }) + releaseVer := testReleaseVer + + base := baseComponent() + base.Overlays = []projectconfig.ComponentOverlay{ + {Type: "file-remove", Filename: "bundled.conf"}, + } + fpBase := computeFingerprint(t, ctx, base, releaseVer, 0) + + withArchive := baseComponent() + withArchive.Overlays = []projectconfig.ComponentOverlay{ + {Type: "file-remove", Filename: "bundled.conf", Archive: "pkg-1.0.tar.gz"}, + } + fpArchive := computeFingerprint(t, ctx, withArchive, releaseVer, 0) + + assert.NotEqual(t, fpBase, fpArchive, + "setting the archive scope must change the fingerprint") + + withRoot := baseComponent() + withRoot.Overlays = []projectconfig.ComponentOverlay{ + {Type: "file-remove", Filename: "bundled.conf", Archive: "pkg-1.0.tar.gz", ArchiveRoot: "custom-root"}, + } + fpRoot := computeFingerprint(t, ctx, withRoot, releaseVer, 0) + + assert.NotEqual(t, fpArchive, fpRoot, + "setting the archive-root override must change the fingerprint") +} + func TestComputeIdentity_PatchAddRenameChangesFP(t *testing.T) { // When patch-add omits 'file', the destination filename is derived from // filepath.Base(Source). Renaming the source file changes the rendered diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 3bd14eff..214bac56 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -5,6 +5,7 @@ package projectconfig_test import ( "reflect" + "strings" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" @@ -104,15 +105,29 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { tag := field.Tag.Get("fingerprint") - switch tag { + // hashstructure tags are `name,option,...`; the name part decides + // inclusion ("-" excludes, anything else includes) and the options + // tune how an included field is hashed. + name, options, _ := strings.Cut(tag, ",") + + switch name { case "": - // No tag — included by default (the safe default). + // Empty name — included by default (the safe default). The only + // option we permit is `omitempty`, which makes hashstructure skip + // the field when it holds its zero value (so an unset field never + // perturbs the hash) while still hashing it when set. Reject any + // other option as a likely typo. + if options != "" && options != "omitempty" { + assert.Failf(t, "invalid fingerprint tag", + "field %q has unrecognised fingerprint tag option %q — "+ + "only `omitempty` is supported on included fields", key, options) + } case "-": actualExclusions[key] = true default: - // hashstructure only recognises "" (include) and "-" (exclude). - // Any other value is silently treated as included, which is - // almost certainly a typo. + // hashstructure only recognises "" (include) and "-" (exclude) + // for the name part. Any other value is silently treated as + // included, which is almost certainly a typo. assert.Failf(t, "invalid fingerprint tag", "field %q has unrecognised fingerprint tag value %q — "+ "only `fingerprint:\"-\"` (exclude) is valid; "+ diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 57bc0926..4e52380e 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -21,16 +21,12 @@ type ComponentOverlay struct { // Human readable description of overlay; primarily present to document the need for the change. Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay" fingerprint:"-"` - // For overlays that target files inside a source archive, identifies the archive to modify. - // Must be a filename (not a path) matching a source archive in the component's sources directory. - // Only file-remove supports archive scoping; when set, it removes file(s) from inside the named - // archive instead of the loose sources tree. + // Scopes the overlay to files inside this source archive (a bare filename, not a path). + // Only file-remove and file-search-replace honor it; when set, the overlay operates inside + // the named archive instead of the loose sources tree. Archive string `toml:"archive,omitempty" json:"archive,omitempty" jsonschema:"title=Archive,description=The source archive to modify (e.g. pkg-1.0.tar.gz)"` - // For overlays that target files inside a source archive, optionally overrides the top-level - // directory to treat as the extraction root, mirroring rpmbuild's `%setup -n`. When unset, the - // root is inferred: if the archive unpacks to a single top-level directory (the conventional - // `%{name}-%{version}` layout) that directory is used; otherwise the archive root is used. - // Set this when an archive's top-level directory does not follow that convention. + // Overrides the archive's extraction root (rpmbuild's `%setup -n` equivalent). When unset, the + // root is inferred: a single top-level directory is used, otherwise the archive root. ArchiveRoot string `toml:"archive-root,omitempty" json:"archiveRoot,omitempty" jsonschema:"title=Archive root,description=Top-level directory inside the archive to treat as the extraction root (mirrors %setup -n); inferred when unset"` // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). diff --git a/internal/projectconfig/overlay_test.go b/internal/projectconfig/overlay_test.go index ece5a574..63734bc1 100644 --- a/internal/projectconfig/overlay_test.go +++ b/internal/projectconfig/overlay_test.go @@ -566,10 +566,14 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { projectconfig.ComponentOverlayAddFile, } - // Archive-scoped overlays: only file-remove becomes archive-scoped, and only - // when its Archive field is set. + // Archive-scoped overlays: only file-remove/file-search-replace becomes archive-scoped, + // and only when its Archive field is set. archiveOverlays := []projectconfig.ComponentOverlay{ {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg-1.0.tar.gz", Filename: "f"}, + { + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: "pkg-1.0.tar.gz", Filename: "f", Regex: "old", Replacement: "new", + }, } for _, overlayType := range specOverlayTypes { From 2a8345b8eb4ca6d4fc4456fb89491867f37c3f7f Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 10 Jun 2026 21:42:56 +0000 Subject: [PATCH 5/5] Fixed test pipeline profile conflict --- .github/workflows/test.yml | 10 ++++++++++ internal/app/azldev/core/sources/sourceprep.go | 12 +++++++++--- internal/fingerprint/fingerprint_test.go | 11 +++-------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 16d9cf6b..a2d85153 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -55,6 +55,11 @@ jobs: set -euxo pipefail sudo apt-get update sudo apt-get purge firefox passt + # Microsoft Edge (preinstalled on the runner image) ships two AppArmor + # profiles attached to the same binary (/etc/apparmor.d/msedge and + # /etc/apparmor.d/microsoft-edge-stable). That duplicate attachment makes + # aa-disable abort while parsing all profiles, so remove them first. + sudo rm -f /etc/apparmor.d/msedge /etc/apparmor.d/microsoft-edge-stable sudo systemctl reload apparmor.service sudo apt-get install apparmor-utils sudo aa-disable /usr/sbin/unix_chkpwd @@ -90,6 +95,11 @@ jobs: set -euxo pipefail sudo apt-get update sudo apt-get purge firefox passt + # Microsoft Edge (preinstalled on the runner image) ships two AppArmor + # profiles attached to the same binary (/etc/apparmor.d/msedge and + # /etc/apparmor.d/microsoft-edge-stable). That duplicate attachment makes + # aa-disable abort while parsing all profiles, so remove them first. + sudo rm -f /etc/apparmor.d/msedge /etc/apparmor.d/microsoft-edge-stable sudo systemctl reload apparmor.service sudo apt-get install apparmor-utils sudo aa-disable /usr/sbin/unix_chkpwd diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 526158aa..46781805 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -671,9 +671,15 @@ func (p *sourcePreparerImpl) updateSourcesFile( config := component.GetConfig() sourceFiles := config.SourceFiles - // Derive archive names from the component's overlays — no need to thread - // them through the overlay application chain. - modifiedArchives := archiveNamesFromOverlays(config.Overlays) + // Derive the archives whose 'sources' hash needs refreshing because an archive + // overlay repacked them. Only meaningful when archive overlays actually ran: + // when skipLookaside is set, archive overlays are skipped (see + // [applyArchiveOverlayGroup]) and the archives may not even be present on disk, + // so rehashing would either fail or pointlessly rewrite an unchanged hash. + var modifiedArchives []string + if !p.skipLookaside { + modifiedArchives = archiveNamesFromOverlays(config.Overlays) + } if len(sourceFiles) == 0 && len(modifiedArchives) == 0 { return nil diff --git a/internal/fingerprint/fingerprint_test.go b/internal/fingerprint/fingerprint_test.go index 3c748a46..2248cd0c 100644 --- a/internal/fingerprint/fingerprint_test.go +++ b/internal/fingerprint/fingerprint_test.go @@ -252,14 +252,9 @@ func TestComputeIdentity_OverlaySourceFileChange(t *testing.T) { } func TestComputeIdentity_OverlayArchiveScopingChangesFP(t *testing.T) { - // Archive scoping fields are hashed with `,omitempty`: an unset scope must - // not perturb the fingerprint (so the overwhelming majority of overlays stay - // idempotent against already-rendered specs), but a SET scope genuinely - // changes the build output (a file removed from inside an archive) and so - // must change the fingerprint. This test pins the second half of that - // contract — guarding against an over-correction that excludes the fields - // entirely (e.g. `fingerprint:"-"`), which would let an archive overlay slip - // through without triggering a rebuild. + // Archive-scoping fields are part of the hashed config, so setting them must + // change the fingerprint. Guards against excluding them (e.g. `fingerprint:"-"`), + // which would let an archive overlay skip a rebuild. ctx := newTestFS(t, map[string]string{ "/specs/test.spec": "Name: testpkg\nVersion: 1.0", })