From 740f79171ee82711b8de90704c427aac53b69ebc Mon Sep 17 00:00:00 2001 From: Thien Trung Vuong Date: Wed, 2 Sep 2026 20:39:24 +0000 Subject: [PATCH] feat(spec): preserve referenced macros during removal --- internal/rpm/spec/structural_tree_api.go | 4 + .../specs/subpackage-define-referenced.spec | 39 + .../specs/subpackage-define-shadowed.spec | 33 + .../specs/subpackage-define-transitive.spec | 38 + internal/rpm/spec/testdata_test.go | 35 + internal/rpm/spec/tree_hoist.go | 857 ++++++++++++++ internal/rpm/spec/tree_hoist_internal_test.go | 1021 +++++++++++++++++ 7 files changed, 2027 insertions(+) create mode 100644 internal/rpm/spec/testdata/specs/subpackage-define-referenced.spec create mode 100644 internal/rpm/spec/testdata/specs/subpackage-define-shadowed.spec create mode 100644 internal/rpm/spec/testdata/specs/subpackage-define-transitive.spec create mode 100644 internal/rpm/spec/tree_hoist.go create mode 100644 internal/rpm/spec/tree_hoist_internal_test.go diff --git a/internal/rpm/spec/structural_tree_api.go b/internal/rpm/spec/structural_tree_api.go index 58b8eae8b..afa10311b 100644 --- a/internal/rpm/spec/structural_tree_api.go +++ b/internal/rpm/spec/structural_tree_api.go @@ -179,6 +179,10 @@ func (t *specTree) RemoveSections(handles []*sectionHandle) error { return err } + if err := t.hoistReferencedMacros(sections); err != nil { + return err + } + removeSections(t.root, sections) return nil diff --git a/internal/rpm/spec/testdata/specs/subpackage-define-referenced.spec b/internal/rpm/spec/testdata/specs/subpackage-define-referenced.spec new file mode 100644 index 000000000..f430da4bd --- /dev/null +++ b/internal/rpm/spec/testdata/specs/subpackage-define-referenced.spec @@ -0,0 +1,39 @@ +Name: subpackage-define-referenced +Version: 1.0 +Release: 1 +Summary: %%define inside a subpackage referenced from %%install (issue #203 repro) +License: MIT + +%description +Fixture mirroring issue #203 -- the helper macro is defined inside the +test subpackage but referenced from the unconditional install section. +Removing the subpackage naively drops the macro and leaves dangling +references in surviving sections. + +%package tests +Summary: Tests for %{name} +Requires: %{name} = %{version}-%{release} + +%define testsdir %{_libdir}/%{name}/tests-src + +%description tests +The %{name}-tests rpm contains test fixtures for %{name}. + +%files tests +%{testsdir} + +%build +make + +%install +make install DESTDIR=%{buildroot} +mkdir -p %{buildroot}%{testsdir}/python +mkdir -p %{buildroot}%{testsdir}/scripts +install -p -m 0644 tests/Makefile.include %{buildroot}%{testsdir}/ + +%files +/usr/bin/subpackage-define-referenced + +%changelog +* Thu Jan 01 1970 Builder - 1.0-1 +- Initial fixture. diff --git a/internal/rpm/spec/testdata/specs/subpackage-define-shadowed.spec b/internal/rpm/spec/testdata/specs/subpackage-define-shadowed.spec new file mode 100644 index 000000000..2eac029cb --- /dev/null +++ b/internal/rpm/spec/testdata/specs/subpackage-define-shadowed.spec @@ -0,0 +1,33 @@ +Name: subpackage-define-shadowed +Version: 1.0 +Release: 1 +Summary: Subpackage %%define overrides a surviving preamble macro +License: MIT + +%global toolsdir %{_libdir}/%{name} + +%description +Fixture verifying that a subpackage %%define whose name already has a +surviving definition in the preamble is hoisted when it is the exact effective +binding of the surviving %%install reference. + +%package tools +Summary: Tools for %{name} + +%global toolsdir %{_libdir}/%{name}/tools-override + +%description tools +Tools for %{name}. + +%files tools +%{toolsdir} + +%install +mkdir -p %{buildroot}%{toolsdir} + +%files +/usr/bin/subpackage-define-shadowed + +%changelog +* Thu Jan 01 1970 Builder - 1.0-1 +- Initial fixture. diff --git a/internal/rpm/spec/testdata/specs/subpackage-define-transitive.spec b/internal/rpm/spec/testdata/specs/subpackage-define-transitive.spec new file mode 100644 index 000000000..cdf0814d0 --- /dev/null +++ b/internal/rpm/spec/testdata/specs/subpackage-define-transitive.spec @@ -0,0 +1,38 @@ +Name: subpackage-define-transitive +Version: 1.0 +Release: 1 +Summary: Transitive %%define chain inside a subpackage (issue #203 follow-up) +License: MIT + +%description +Fixture for the transitive macro-hoisting case: the subpackage defines a +chain of helper macros (%%testroot -> %%testsdir) and only the outer one is +referenced from the surviving %%install section. Removing the subpackage must +hoist BOTH macros so the survivor reference resolves. + +%package tests +Summary: Tests for %{name} +Requires: %{name} = %{version}-%{release} + +%define testroot %{_libdir}/%{name} +%define testsdir %{testroot}/tests-src + +%description tests +The %{name}-tests rpm contains test fixtures for %{name}. + +%files tests +%{testsdir} + +%build +make + +%install +make install DESTDIR=%{buildroot} +mkdir -p %{buildroot}%{testsdir}/python + +%files +/usr/bin/subpackage-define-transitive + +%changelog +* Thu Jan 01 1970 Builder - 1.0-1 +- Initial fixture. diff --git a/internal/rpm/spec/testdata_test.go b/internal/rpm/spec/testdata_test.go index 1cdc670ba..576a4d20f 100644 --- a/internal/rpm/spec/testdata_test.go +++ b/internal/rpm/spec/testdata_test.go @@ -196,6 +196,41 @@ func TestStructuralParserFixtureSearchAndReplaceCoversLineTypes(t *testing.T) { assertReparseable(t, contents) } +func TestStructuralParserFixtureSubpackageMacroRemovalHoistsReferencedDefinitions(t *testing.T) { + for _, name := range []string{ + "subpackage-define-referenced.spec", + "subpackage-define-transitive.spec", + "subpackage-define-shadowed.spec", + } { + t.Run(name, func(t *testing.T) { + specification := openFixture(t, name) + require.NoError(t, specification.RemoveSubpackage( + map[string]string{ + "subpackage-define-referenced.spec": "tests", + "subpackage-define-transitive.spec": "tests", + "subpackage-define-shadowed.spec": "tools", + }[name], + )) + + contents := serializeFixture(t, specification) + assertReparseable(t, contents) + + switch name { + case "subpackage-define-referenced.spec": + assert.Contains(t, contents, "%define testsdir %{_libdir}/%{name}/tests-src") + assert.Less(t, strings.LastIndex(contents, "%define testsdir"), strings.Index(contents, "\n%install\n")) + case "subpackage-define-transitive.spec": + assert.Less(t, strings.Index(contents, "%define testroot"), strings.Index(contents, "%define testsdir")) + assert.Contains(t, contents, "%define testsdir %{testroot}/tests-src") + case "subpackage-define-shadowed.spec": + assert.Equal(t, 2, strings.Count(contents, "%global toolsdir")) + assert.Contains(t, contents, "tools-override") + assert.Less(t, strings.LastIndex(contents, "%global toolsdir"), strings.Index(contents, "\n%install\n")) + } + }) + } +} + func TestStructuralParserGDBShapedMacroBodyIsOpaqueKnownLimitation(t *testing.T) { input := `%define gdb_python_configure \ %if 0%{?with_python}\ diff --git a/internal/rpm/spec/tree_hoist.go b/internal/rpm/spec/tree_hoist.go new file mode 100644 index 000000000..e0f196f34 --- /dev/null +++ b/internal/rpm/spec/tree_hoist.go @@ -0,0 +1,857 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package spec + +import ( + "errors" + "fmt" + "log/slog" + "regexp" + "slices" + "strings" + "unicode" +) + +// ErrUnsafeMacroHoist is returned when moving a macro declaration out of a +// removed section could change RPM macro evaluation. +var ErrUnsafeMacroHoist = errors.New("unsafe macro hoist") + +var undefineDirective = regexp.MustCompile(`(?i)^\s*%undefine\s+([[:alnum:]_.-]+)\b`) + +const macroDirectiveSubmatches = 2 + +type macroDefinition struct { + block *block + global bool + conditional bool + parametered bool + lua bool + removed bool + order int + refs map[string]bool + dynamicRefs []string +} + +type macroReference struct { + name string + order int +} + +type macroDynamicReference struct { + pattern string + order int +} + +type macroDependencyTraversal struct { + definition *macroDefinition + useOrder int +} + +type selectedMacroClosure struct { + definitions map[*macroDefinition]bool +} + +type macroFacts struct { + removed []*macroDefinition + all map[string][]*macroDefinition + references []macroReference + dynamicRefs []macroDynamicReference + undefined map[string]bool + rootStarts map[*block]int +} + +// hoistReferencedMacros preserves only the exact declarations used by surviving +// references. RPM macro evaluation is contextual, so any ambiguous relocation +// is rejected rather than guessed. +func (t *specTree) hoistReferencedMacros(removeSet map[*block]bool) error { + insertAt := -1 + + for index, child := range t.root.Children { + if containsRemovedSection(child, removeSet) { + insertAt = index + + break + } + } + + if insertAt < 0 { + return fmt.Errorf("finding removed root ancestor for macro hoist:\n%w", ErrUnsafeMacroHoist) + } + + facts := collectMacroFacts(t.root, removeSet) + if len(facts.removed) == 0 { + return nil + } + + closure, err := selectMacroClosure(facts) + if err != nil { + return err + } + + if len(closure.definitions) == 0 { + return validateDynamicMacroReferences(closure.definitions, facts) + } + + if err := validateMacroHoist(closure, facts, facts.rootStarts[t.root.Children[insertAt]]); err != nil { + return err + } + + hoisted := make([]*block, 0, len(closure.definitions)) + for _, definition := range facts.removed { + if !closure.definitions[definition] { + continue + } + + hoisted = append(hoisted, &block{ + Kind: macroDefBlock, + Header: definition.block.Header, + Name: definition.block.Name, + Lines: slices.Clone(definition.block.Lines), + }) + slog.Debug("Hoisting macro definition from removed section", "macro", definition.block.Name) + } + + t.root.Children = append(t.root.Children[:insertAt], append(hoisted, t.root.Children[insertAt:]...)...) + + return nil +} + +func selectMacroClosure(facts *macroFacts) (*selectedMacroClosure, error) { + closure := &selectedMacroClosure{ + definitions: make(map[*macroDefinition]bool), + } + selectedByName := make(map[string]*macroDefinition) + traversed := make(map[macroDependencyTraversal]bool) + + var visit func(string, int) error + + visit = func(name string, useOrder int) error { + definition := effectiveMacroDefinition(facts.all[name], useOrder) + if definition == nil { + return nil + } + + if definition.removed { + closure.definitions[definition] = true + } + + traversal := macroDependencyTraversal{definition: definition, useOrder: useOrder} + if traversed[traversal] { + return nil + } + + traversed[traversal] = true + + // A %global is expanded at its definition, while a %define remains + // lazy. Follow lazy dependencies at each surviving use so a removed + // definition they resolve to can be preserved. Traversal includes the + // use order because separate invocations can resolve dependencies to + // different declarations. + dependencyOrder := useOrder + if definition.global { + dependencyOrder = definition.order - 1 + } + + for dependency := range definition.refs { + if err := visit(dependency, dependencyOrder); err != nil { + return err + } + } + + return nil + } + + for _, reference := range facts.references { + definition := effectiveMacroDefinition(facts.all[reference.name], reference.order) + if definition != nil && definition.removed { + if existing := selectedByName[reference.name]; existing != nil && existing != definition { + return nil, fmt.Errorf("multiple removed declarations of macro %#q are effective at surviving references:\n%w", + reference.name, ErrUnsafeMacroHoist) + } + + selectedByName[reference.name] = definition + } + + if err := visit(reference.name, reference.order); err != nil { + return nil, err + } + } + + return closure, nil +} + +func effectiveMacroDefinition(definitions []*macroDefinition, order int) *macroDefinition { + var effective *macroDefinition + for _, definition := range definitions { + if definition.order <= order && (effective == nil || effective.order < definition.order) { + effective = definition + } + } + + return effective +} + +func validateMacroHoist(closure *selectedMacroClosure, facts *macroFacts, insertionOrder int) error { + if err := validateSelectedMacroHoists(closure.definitions, facts, insertionOrder); err != nil { + return err + } + + if err := validateConditionalMacroSelections(closure.definitions, facts); err != nil { + return err + } + + if err := validateLazyDependencyBindings(closure.definitions, facts, insertionOrder); err != nil { + return err + } + + if err := validateDynamicMacroReferences(closure.definitions, facts); err != nil { + return err + } + + return nil +} + +func validateSelectedMacroHoists(selected map[*macroDefinition]bool, facts *macroFacts, insertionOrder int) error { + for definition := range selected { + if definition.conditional || definition.parametered || definition.lua { + return fmt.Errorf("cannot safely hoist macro %#q from conditional, parameterized, or Lua scope:\n%w", + definition.block.Name, ErrUnsafeMacroHoist) + } + + if facts.undefined[definition.block.Name] { + return fmt.Errorf("cannot safely hoist macro %#q across '%%undefine':\n%w", + definition.block.Name, ErrUnsafeMacroHoist) + } + + for _, candidate := range facts.all[definition.block.Name] { + if !candidate.removed && + insertionOrder < candidate.order && candidate.order < definition.order { + return fmt.Errorf("cannot safely hoist macro %#q across surviving declaration:\n%w", + definition.block.Name, ErrUnsafeMacroHoist) + } + } + + if err := validateRelocation(definition, facts.references, insertionOrder); err != nil { + return err + } + + if err := validateGlobalDependency(definition, facts); err != nil { + return err + } + + if err := validateSelectedGlobalDynamicBindings(definition, facts); err != nil { + return err + } + } + + return nil +} + +func validateSelectedGlobalDynamicBindings(definition *macroDefinition, facts *macroFacts) error { + if !definition.global { + return nil + } + + for _, pattern := range definition.dynamicRefs { + for _, candidate := range facts.all { + for _, binding := range candidate { + if dynamicMacroNameMayReferTo(pattern, binding.block.Name) { + return fmt.Errorf("cannot safely hoist eager '%%global' macro %#q with dynamic name %#q:\n%w", + definition.block.Name, pattern, ErrUnsafeMacroHoist) + } + } + } + } + + return nil +} + +//nolint:cyclop,funlen // Selected closure validation follows both reachability and dependency bindings. +func validateConditionalMacroSelections(selected map[*macroDefinition]bool, facts *macroFacts) error { + containsSelected := make(map[macroDependencyTraversal]bool) + searching := make(map[macroDependencyTraversal]bool) + relevant := make(map[macroDependencyTraversal]bool) + + var reachesSelected func(*macroDefinition, int) bool + + reachesSelected = func(definition *macroDefinition, useOrder int) bool { + if definition == nil { + return false + } + + traversal := macroDependencyTraversal{definition: definition, useOrder: useOrder} + if known, ok := containsSelected[traversal]; ok { + return known + } + + if searching[traversal] { + return selected[definition] + } + + searching[traversal] = true + found := selected[definition] + + dependencyOrder := useOrder + if definition.global { + dependencyOrder = definition.order - 1 + } + + for dependency := range definition.refs { + dependencyDefinition := effectiveMacroDefinition(facts.all[dependency], dependencyOrder) + found = found || reachesSelected(dependencyDefinition, dependencyOrder) + } + + delete(searching, traversal) + containsSelected[traversal] = found + + return found + } + + var markRelevant func(*macroDefinition, int) + + markRelevant = func(definition *macroDefinition, useOrder int) { + if definition == nil { + return + } + + traversal := macroDependencyTraversal{definition: definition, useOrder: useOrder} + if relevant[traversal] { + return + } + + relevant[traversal] = true + + dependencyOrder := useOrder + if definition.global { + dependencyOrder = definition.order - 1 + } + + for dependency := range definition.refs { + markRelevant(effectiveMacroDefinition(facts.all[dependency], dependencyOrder), dependencyOrder) + } + } + + for _, reference := range facts.references { + definition := effectiveMacroDefinition(facts.all[reference.name], reference.order) + if reachesSelected(definition, reference.order) { + markRelevant(definition, reference.order) + } + } + + for binding := range relevant { + for _, candidate := range facts.all[binding.definition.block.Name] { + if candidate.conditional && candidate.order <= binding.useOrder { + return fmt.Errorf("cannot safely choose macro %#q around conditional declarations:\n%w", + binding.definition.block.Name, ErrUnsafeMacroHoist) + } + } + } + + return nil +} + +func validateLazyDependencyBindings(selected map[*macroDefinition]bool, facts *macroFacts, insertionOrder int) error { + visited := make(map[macroDependencyTraversal]bool) + + var visit func(*macroDefinition, int) error + + visit = func(definition *macroDefinition, useOrder int) error { + if definition == nil || definition.global { + return nil + } + + traversal := macroDependencyTraversal{definition: definition, useOrder: useOrder} + if visited[traversal] { + return nil + } + + visited[traversal] = true + + for dependency := range definition.refs { + before := effectiveMacroDefinition(facts.all[dependency], useOrder) + + after := effectiveHoistedMacroDefinition(facts.all[dependency], selected, insertionOrder, useOrder) + if before != after { + return fmt.Errorf("cannot safely hoist dependency %#q used by lazy macro %#q:\n%w", + dependency, definition.block.Name, ErrUnsafeMacroHoist) + } + + if err := visit(before, useOrder); err != nil { + return err + } + } + + return nil + } + + for _, reference := range facts.references { + definition := effectiveMacroDefinition(facts.all[reference.name], reference.order) + if definition == nil || definition.global { + continue + } + + if err := visit(definition, reference.order); err != nil { + return err + } + } + + return nil +} + +func effectiveHoistedMacroDefinition( + definitions []*macroDefinition, + selected map[*macroDefinition]bool, + insertionOrder int, + useOrder int, +) *macroDefinition { + var effective *macroDefinition + + effectiveOrder := -1 + + for _, definition := range definitions { + order := definition.order + if definition.removed { + if !selected[definition] { + continue + } + + order = insertionOrder + } + + if order <= useOrder && order > effectiveOrder { + effective = definition + effectiveOrder = order + } + } + + return effective +} + +func validateDynamicMacroReferences(selected map[*macroDefinition]bool, facts *macroFacts) error { + dynamics := slices.Clone(facts.dynamicRefs) + visited := make(map[macroDependencyTraversal]bool) + + for definition := range selected { + for _, pattern := range definition.dynamicRefs { + dynamics = append(dynamics, macroDynamicReference{pattern: pattern, order: definition.order}) + } + } + + var visit func(*macroDefinition, int) + + visit = func(definition *macroDefinition, useOrder int) { + if definition == nil || definition.global { + return + } + + traversal := macroDependencyTraversal{definition: definition, useOrder: useOrder} + if visited[traversal] { + return + } + + visited[traversal] = true + + for _, pattern := range definition.dynamicRefs { + dynamics = append(dynamics, macroDynamicReference{pattern: pattern, order: useOrder}) + } + + for dependency := range definition.refs { + visit(effectiveMacroDefinition(facts.all[dependency], useOrder), useOrder) + } + } + + for _, reference := range facts.references { + visit(effectiveMacroDefinition(facts.all[reference.name], reference.order), reference.order) + } + + for _, dynamic := range dynamics { + for _, definition := range facts.removed { + if (definition.order <= dynamic.order || selected[definition]) && + dynamicMacroNameMayReferTo(dynamic.pattern, definition.block.Name) { + return fmt.Errorf("cannot safely resolve dynamic macro name %#q:\n%w", + dynamic.pattern, ErrUnsafeMacroHoist) + } + } + } + + return nil +} + +func dynamicMacroNameMayReferTo(pattern, name string) bool { + prefixEnd := strings.IndexByte(pattern, '%') + if prefixEnd < 0 { + return false + } + + suffixStart := strings.LastIndexByte(pattern, '}') + 1 + prefix, suffix := pattern[:prefixEnd], pattern[suffixStart:] + + return strings.HasPrefix(name, prefix) && strings.HasSuffix(name, suffix) +} + +func validateRelocation(definition *macroDefinition, references []macroReference, insertionOrder int) error { + if definition.order <= insertionOrder { + return nil + } + + for _, reference := range references { + if reference.name == definition.block.Name && + insertionOrder <= reference.order && reference.order < definition.order { + return fmt.Errorf("cannot safely hoist macro %#q across an earlier surviving reference:\n%w", + definition.block.Name, ErrUnsafeMacroHoist) + } + } + + return nil +} + +func validateGlobalDependency(definition *macroDefinition, facts *macroFacts) error { + if !definition.global { + return nil + } + + for dependency := range definition.refs { + if effectiveMacroDefinition(facts.all[dependency], definition.order-1) == nil { + for _, candidate := range facts.all[dependency] { + if candidate == definition { + return fmt.Errorf("cannot safely hoist eager '%%global' macro %#q before dependency %#q:\n%w", + definition.block.Name, dependency, ErrUnsafeMacroHoist) + } + } + } + + for _, candidate := range facts.all[dependency] { + if candidate == definition { + continue + } + + if !candidate.removed || candidate.order >= definition.order { + return fmt.Errorf("cannot safely hoist eager '%%global' macro %#q before dependency %#q:\n%w", + definition.block.Name, dependency, ErrUnsafeMacroHoist) + } + } + + if facts.undefined[dependency] { + return fmt.Errorf("cannot safely hoist eager '%%global' macro %#q across '%%undefine' of %#q:\n%w", + definition.block.Name, dependency, ErrUnsafeMacroHoist) + } + } + + return nil +} + +//nolint:cyclop,funlen,gocognit // Macro facts require one pass over structural block types. +func collectMacroFacts(root *block, removeSet map[*block]bool) *macroFacts { + facts := ¯oFacts{ + all: make(map[string][]*macroDefinition), + undefined: make(map[string]bool), + rootStarts: make(map[*block]int), + } + order := 0 + + addReferences := func(content string, referenceOrder int) { + references, dynamics := macroReferenceDetails(content) + for name := range references { + facts.references = append(facts.references, macroReference{name: name, order: referenceOrder}) + } + + for _, pattern := range dynamics { + facts.dynamicRefs = append(facts.dynamicRefs, macroDynamicReference{pattern: pattern, order: referenceOrder}) + } + } + + var walk func(*block, bool, bool) + + walk = func(current *block, removed, conditional bool) { + removed = removed || removeSet[current] + conditional = conditional || current.Kind == conditionalBlock + + switch current.Kind { + case rootBlock: + for _, child := range current.Children { + facts.rootStarts[child] = order + walk(child, false, false) + } + + return + case sectionBlock: + if !removed { + addReferences(sectionHeaderArguments(current.Header, current.Name), order) + } + + order++ + case macroDefBlock: + references, dynamics := macroReferenceDetails(strings.Join(current.Lines, "\n")) + definition := ¯oDefinition{ + block: current, + global: strings.HasPrefix(strings.ToLower(strings.TrimSpace(current.Header)), "%global"), + conditional: conditional, + parametered: strings.Contains(strings.Fields(strings.TrimSpace(current.Header))[1], "("), + lua: strings.Contains(strings.Join(current.Lines, "\n"), "%{lua:"), + removed: removed, + order: order, + refs: references, + dynamicRefs: dynamics, + } + + facts.all[current.Name] = append(facts.all[current.Name], definition) + if removed { + facts.removed = append(facts.removed, definition) + } else if definition.global { + // A %define body is expanded only at an invocation, so recording + // its references here would treat them as real earlier uses. + addReferences(strings.Join(current.Lines, "\n"), order-1) + } + + order++ + case textBlock: + for _, line := range current.Lines { + if matches := undefineDirective.FindStringSubmatch(line); len(matches) == macroDirectiveSubmatches { + facts.undefined[matches[1]] = true + } + + if !removed { + addReferences(line, order) + } + + order++ + } + case conditionalBlock: + if !removed { + addReferences(current.Header, order) + addReferences(current.ElseDirective, order) + } + + order++ + } + + for _, child := range current.Children { + walk(child, removed, conditional) + } + + for _, child := range current.Else { + walk(child, removed, conditional) + } + } + + walk(root, false, false) + + return facts +} + +// sectionHeaderArguments excludes the section marker, which RPM does not expand, +// while retaining the raw argument text where macro references are meaningful. +func sectionHeaderArguments(header, name string) string { + header = strings.TrimLeftFunc(header, unicode.IsSpace) + if !strings.HasPrefix(strings.ToLower(header), strings.ToLower(name)) { + return "" + } + + return header[len(name):] +} + +func macroReferences(content string) map[string]bool { + refs := make(map[string]bool) + scanMacroReferences(content, refs, nil) + + return refs +} + +func macroReferenceDetails(content string) (map[string]bool, []string) { + refs := make(map[string]bool) + + var dynamics []string + scanMacroReferences(content, refs, &dynamics) + + return refs, dynamics +} + +func scanMacroReferences(content string, refs map[string]bool, dynamics *[]string) { + for index := 0; index < len(content); { + if content[index] != '%' { + index++ + + continue + } + + runEnd := index + for runEnd < len(content) && content[runEnd] == '%' { + runEnd++ + } + + if runEnd == len(content) { + index = runEnd + + continue + } + + if (runEnd-index)%2 == 0 { + index = runEnd + + continue + } + + if content[runEnd] == '{' && percentRunOpensBracedMacro(content, index) { + end, ok := bracedMacroEnd(content, runEnd+1) + if !ok { + index = runEnd + 1 + + continue + } + + addBracedMacroReference(content[runEnd+1:end], refs, dynamics) + index = end + 1 + + continue + } + + if macroNameStart(content[runEnd]) { + end := runEnd + 1 + for end < len(content) && macroNameCharacter(content[end]) { + end++ + } + + if !macroDirectiveName(content[runEnd:end]) { + refs[content[runEnd:end]] = true + } + + index = end + + continue + } + + index = runEnd + 1 + } +} + +func addBracedMacroReference(body string, refs map[string]bool, dynamics *[]string) { + name := strings.TrimLeft(body, "!?") + + fields := strings.Fields(name) + if len(fields) == 0 { + return + } + + addBracedMacroName(fields, refs, dynamics) + scanMacroReferences(body, refs, dynamics) +} + +func addBracedMacroName(fields []string, refs map[string]bool, dynamics *[]string) { + if macroTestDirectiveName(fields[0]) { + if len(fields) > 1 { + refs[fields[1]] = true + } + + return + } + + macroName := strings.Split(fields[0], ":")[0] + if strings.Contains(macroName, "%") { + if dynamics != nil { + *dynamics = append(*dynamics, macroName) + } + + return + } + + if !macroDirectiveName(macroName) { + refs[macroName] = true + } +} + +func bracedMacroEnd(content string, start int) (int, bool) { + depth := 1 + + for index := start; index < len(content); index++ { + if content[index] == '%' { + nextIndex, nested, ok := bracedPercentIndex(content, index) + if !ok { + return 0, false + } + + if nested { + depth++ + } + + index = nextIndex + + continue + } + + if content[index] == '}' { + depth-- + if depth == 0 { + return index, true + } + } + } + + return 0, false +} + +func bracedPercentIndex(content string, index int) (nextIndex int, nested bool, ok bool) { + runEnd := index + for runEnd < len(content) && content[runEnd] == '%' { + runEnd++ + } + + if runEnd == len(content) || content[runEnd] != '{' { + return runEnd - 1, false, true + } + + if percentRunOpensBracedMacro(content, index) { + return runEnd, true, true + } + + escapedEnd, ok := escapedBracedMacroEnd(content, runEnd+1) + + return escapedEnd, false, ok +} + +func escapedBracedMacroEnd(content string, start int) (int, bool) { + depth := 1 + + for index := start; index < len(content); index++ { + switch content[index] { + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return index, true + } + } + } + + return 0, false +} + +func macroNameStart(character byte) bool { + return character == '_' || (character >= 'a' && character <= 'z') || (character >= 'A' && character <= 'Z') +} + +func macroNameCharacter(character byte) bool { + return macroNameStart(character) || + (character >= '0' && character <= '9') || + character == '.' || character == '-' +} + +func macroDirectiveName(name string) bool { + switch strings.ToLower(name) { + case "define", "defined", "global", "undefine", "undefined", + "if", "else", "elif", "endif", "ifarch", "ifnarch", "ifos", "ifnos": + return true + } + + return false +} + +func macroTestDirectiveName(name string) bool { + switch strings.ToLower(name) { + case "defined", "undefined": + return true + } + + return false +} diff --git a/internal/rpm/spec/tree_hoist_internal_test.go b/internal/rpm/spec/tree_hoist_internal_test.go new file mode 100644 index 000000000..48d75d508 --- /dev/null +++ b/internal/rpm/spec/tree_hoist_internal_test.go @@ -0,0 +1,1021 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package spec + +import ( + "bytes" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRemoveSectionsHoistsReferencedMacroClosure(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define root %{base}/tools", + "%define base /usr/lib", + "%define unused ignored", + "%description tools", + "%{root}", + "%install", + "install -d %{root}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, []string{ + "%define root %{base}/tools", + "%define base /usr/lib", + "%install", + "install -d %{root}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsMacroReferencedBySurvivingSectionHeader(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define suffix tools", + "%description tools", + "tools", + "%package %{name}-%{suffix}", + "%description %{name}-%{suffix}", + "survives", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, []string{ + "%define suffix tools", + "%package %{name}-%{suffix}", + "%description %{name}-%{suffix}", + "survives", + }, specification.rawLines) +} + +func TestRemoveSectionsDoesNotTreatBuildSectionMarkerAsMacroReference(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define build(arg) %{arg}", + "%description tools", + "tools", + "%build", + "make", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%build", + "make", + }, specification.rawLines) +} + +func TestRemoveSectionsDoesNotTreatInstallSectionMarkerAsMacroReference(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define install /usr/bin/install", + "%description tools", + "tools", + "%install", + "install -d %{buildroot}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%install", + "install -d %{buildroot}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsMacroReferencedByPackageHeaderArguments(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define name replacement", + "%define suffix extras", + "%description tools", + "tools", + "%package -n %{name}-%{suffix}", + "%description -n %{name}-%{suffix}", + "survives", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define name replacement", + "%define suffix extras", + "%package -n %{name}-%{suffix}", + "%description -n %{name}-%{suffix}", + "survives", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsMacroReferencedByFilesHeaderArguments(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define manifest files.list", + "%description tools", + "tools", + "%files -f %{manifest}", + "/usr/bin/tool", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define manifest files.list", + "%files -f %{manifest}", + "/usr/bin/tool", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsMacroReferencedByTriggerHeaderArguments(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define trigger_target other-package", + "%description tools", + "tools", + "%triggerin -- %{trigger_target}", + "echo triggered", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define trigger_target other-package", + "%triggerin -- %{trigger_target}", + "echo triggered", + }, specification.rawLines) +} + +func TestRemoveSectionsPreservesQemuIssue203Macro(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define qemu_target %{_arch}", + "%description tools", + "tools", + "%install", + "echo %{qemu_target}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define qemu_target %{_arch}", + "%install", + "echo %{qemu_target}", + }, specification.rawLines) +} + +func TestRemoveSectionsAtomicallyHoistsExpandMacroWithRawBraces(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package dates", + "%define date_corpus %{expand:", + "for date in 2024-02-29 2025-02-28; do", + ` if { test "${date#????-??-??}" = "$date"; }; then`, + " %if 0", + " printf '%s\\n' %{date}", + " %endif", + " fi", + "done", + "}", + "%description dates", + "dates", + "%install", + "printf '%s\\n' %{date_corpus}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("dates")) + })) + assert.Equal(t, []string{ + "%define date_corpus %{expand:", + "for date in 2024-02-29 2025-02-28; do", + ` if { test "${date#????-??-??}" = "$date"; }; then`, + " %if 0", + " printf '%s\\n' %{date}", + " %endif", + " fi", + "done", + "}", + "%install", + "printf '%s\\n' %{date_corpus}", + }, specification.rawLines) +} + +func TestRemoveSectionsIgnoresUnrelatedConditionalMacroDeclarations(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%if %{with kvm}", + "%define kvm_package qemu-kvm", + "%else", + "%define kvm_package qemu-system", + "%endif", + "%package tests", + "%define testsdir %{_libdir}/%{name}/tests-src", + "%description tests", + "tests", + "%install", + "install -d %{testsdir}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tests")) + })) + assert.Equal(t, []string{ + "%if %{with kvm}", + "%define kvm_package qemu-kvm", + "%else", + "%define kvm_package qemu-system", + "%endif", + "%define testsdir %{_libdir}/%{name}/tests-src", + "%install", + "install -d %{testsdir}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsLaterEffectiveGlobalMacro(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%global toolsdir /usr/share/tools", + "%package tools", + "%global toolsdir %{_libdir}/tools", + "%description tools", + "tools", + "%install", + "install -d %{toolsdir}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, []string{ + "%global toolsdir /usr/share/tools", + "%global toolsdir %{_libdir}/tools", + "%install", + "install -d %{toolsdir}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsPriorGlobalUsedByLaterGlobal(t *testing.T) { + tests := []struct { + name string + lines []string + expected []string + }{ + { + name: "later global survives", + lines: []string{ + "%package tools", + "%global foo old", + "%description tools", + "tools", + "%install", + "%global foo %{?foo}-new", + "echo %{foo}", + }, + expected: []string{ + "%global foo old", + "%install", + "%global foo %{?foo}-new", + "echo %{foo}", + }, + }, + { + name: "later global is selected", + lines: []string{ + "%package tools", + "%global foo old", + "%global foo %{?foo}-new", + "%description tools", + "tools", + "%install", + "echo %{foo}", + }, + expected: []string{ + "%global foo old", + "%global foo %{?foo}-new", + "%install", + "echo %{foo}", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + specification := newTreeAPISpec(test.lines) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, test.expected, specification.rawLines) + }) + } +} + +func TestRemoveSectionsHoistCycleTerminates(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define one %{two}", + "%define two %{one}", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define one %{two}", + "%define two %{one}", + "%install", + "echo %{one}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsLazyDependencyEffectiveAtSurvivingUse(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define root %{base}/tools", + "%define base /usr/lib", + "%description tools", + "tools", + "%install", + "install -d %{root}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define root %{base}/tools", + "%define base /usr/lib", + "%install", + "install -d %{root}", + }, specification.rawLines) +} + +func TestRemoveSectionsHoistsDependencyOfSurvivingLazyMacro(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define root %{base}/tools", + "%package tools", + "%define base /usr/lib", + "%description tools", + "tools", + "%install", + "install -d %{root}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define root %{base}/tools", + "%define base /usr/lib", + "%install", + "install -d %{root}", + }, specification.rawLines) +} + +func TestRemoveSectionsRejectsAmbiguousDependencyOfSurvivingLazyMacro(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define root %{base}/tools", + "%package tools", + "%define base /usr/lib", + "%description tools", + "tools", + "%install", + "install -d %{root}", + "%files tools", + "%define base /opt/lib", + "%check", + "install -d %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsDifferentRemovedLazyDependenciesAtSurvivingUses(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define root %{base}/tools", + "%define base /usr/lib", + "%description tools", + "tools", + "%install", + "install -d %{root}", + "%files tools", + "%define base /opt/lib", + "%check", + "install -d %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsRemovedLazyRootWithChangedDependencyBinding(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define base old", + "%package tools", + "%define root %{base}", + "%description tools", + "tools", + "%install", + "echo %{root}", + "%files tools", + "%define base new", + "%check", + "echo %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsHoistsLazyDependencyUsedAfterRemovedDeclaration(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%description tools", + "tools", + "%package other", + "%define root %{base}/other", + "%description other", + "other", + "%files tools", + "%define base /usr/lib", + "%check", + "install -d %{root}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define base /usr/lib", + "%package other", + "%define root %{base}/other", + "%description other", + "other", + "%check", + "install -d %{root}", + }, specification.rawLines) +} + +func TestRemoveSectionsRejectsHoistingAcrossSurvivingSameNameDefinition(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%description tools", + "tools", + "%install", + "%define location /usr/lib", + "%files tools", + "%define location /opt/lib", + "%check", + "echo %{location}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsHoistsWithoutCrossingSameNameDefinition(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define location /opt/lib", + "%description tools", + "tools", + "%install", + "echo %{location}", + "%check", + "%define location /usr/lib", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define location /opt/lib", + "%install", + "echo %{location}", + "%check", + "%define location /usr/lib", + }, specification.rawLines) +} + +func TestRemoveSectionsRejectsUnsafeMacroHoistsWithoutMutation(t *testing.T) { + tests := []struct { + name string + lines []string + }{ + { + name: "eager global dependency declared later", + lines: []string{ + "%package tools", + "%global one %{two}", + "%define two value", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }, + }, + { + name: "self-referential eager global", + lines: []string{ + "%package tools", + "%global one %{?one}", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }, + }, + { + name: "eager global dependency has ambiguous surviving definition", + lines: []string{ + "%global toolsdir /usr/share/tools", + "%package tools", + "%global toolpath %{toolsdir}/bin", + "%description tools", + "%{toolpath}", + "%install", + "echo %{toolpath}", + }, + }, + { + name: "surviving undefine", + lines: []string{ + "%package tools", + "%define one value", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + "%undefine one", + }, + }, + { + name: "removed undefine", + lines: []string{ + "%package tools", + "%define one value", + "%undefine one", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }, + }, + { + name: "parameterized declaration", + lines: []string{ + "%package tools", + "%define one(arg) %{arg}", + "%description tools", + "%one value", + "%install", + "echo %one value", + }, + }, + { + name: "conditional declaration", + lines: []string{ + "%package tools", + "%if 1", + "%define one value", + "%endif", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }, + }, + { + name: "Lua declaration", + lines: []string{ + "%package tools", + "%global one %{lua:print('value')}", + "%description tools", + "%{one}", + "%install", + "echo %{one}", + }, + }, + { + name: "different removed declarations effective at surviving references", + lines: []string{ + "%package tools", + "%define one first", + "%description tools", + "tools", + "%install", + "echo %{one}", + "%files tools", + "%define one second", + "%check", + "echo %{one}", + }, + }, + { + name: "conditional peer declaration", + lines: []string{ + "%if 1", + "%define one conditional", + "%endif", + "%package tools", + "%define one removed", + "%description tools", + "tools", + "%install", + "echo %{one}", + }, + }, + { + name: "relocation crosses earlier surviving reference", + lines: []string{ + "%if 1", + "echo %{one}", + "%package tools", + "%define one value", + "%description tools", + "tools", + "%endif", + "%install", + "echo %{one}", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + specification := newTreeAPISpec(test.lines) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) + }) + } +} + +func TestRemoveSectionsHoistsMultilineMacroAndLogs(t *testing.T) { + var logs bytes.Buffer + + previous := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelDebug}))) + t.Cleanup(func() { slog.SetDefault(previous) }) + + specification := newTreeAPISpec([]string{ + "%package tools", + "%define path /usr \\", + " /share/tools", + "%description tools", + "%{path}", + "%install", + "echo %{path}", + }) + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, []string{ + "%define path /usr \\", + " /share/tools", + "%install", + "echo %{path}", + }, specification.rawLines) + assert.Contains(t, logs.String(), "Hoisting macro definition from removed section") +} + +func TestMacroReferencesRecognizesDefinedAndUndefined(t *testing.T) { + refs := macroReferences("%{defined feature} %{undefined missing} %{?optional} %bare") + assert.Equal(t, map[string]bool{ + "feature": true, + "missing": true, + "optional": true, + "bare": true, + }, refs) +} + +func TestMacroReferencesRecognizesNestedAndArgumentReferences(t *testing.T) { + refs := macroReferences("%{expand:%{dep}} %{helper arg}") + assert.Equal(t, map[string]bool{ + "expand": true, + "dep": true, + "helper": true, + }, refs) +} + +func TestRemoveSectionsHoistsNestedAndArgumentMacroReferences(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define dep /usr/lib", + "%global expanded %{expand:%{dep}}", + "%define helper value", + "%description tools", + "tools", + "%install", + "echo %{expanded} %{helper arg}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + + assert.Equal(t, []string{ + "%define dep /usr/lib", + "%global expanded %{expand:%{dep}}", + "%define helper value", + "%install", + "echo %{expanded} %{helper arg}", + }, specification.rawLines) +} + +func TestRemoveSectionsRejectsLazyDependencyMovedBeforeEarlierBinding(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%if 1", + "%define root %{base}", + "%define base old", + "echo %{root}", + "%package tools", + "%define base new", + "%description tools", + "tools", + "%endif", + "%check", + "echo %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestValidateLazyDependencyBindingsChecksEveryInvocation(t *testing.T) { + root := ¯oDefinition{ + block: &block{Name: "root"}, + order: 1, + refs: map[string]bool{"base": true}, + } + oldBase := ¯oDefinition{block: &block{Name: "base"}, order: 2} + newBase := ¯oDefinition{block: &block{Name: "base"}, removed: true, order: 4} + facts := ¯oFacts{ + all: map[string][]*macroDefinition{ + "root": {root}, + "base": {oldBase, newBase}, + }, + references: []macroReference{ + {name: "root", order: 3}, + {name: "root", order: 5}, + }, + } + + err := validateLazyDependencyBindings(map[*macroDefinition]bool{newBase: true}, facts, 0) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) +} + +func TestRemoveSectionsRejectsConditionalDeclarationWithoutEvaluatingIt(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%if 0", + "%define base old", + "%endif", + "%define root %{base}", + "%package tools", + "%define base new", + "%description tools", + "tools", + "%install", + "echo %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsConditionalDependencyChosenAtSurvivingUse(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define root %{base}", + "%description tools", + "tools", + "%install", + "%if %{with alternate}", + "%define base first", + "%else", + "%define base second", + "%endif", + "%check", + "echo %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsSurvivingDynamicMacroNameMatchingRemovedDefinition(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define suffix name", + "%package tools", + "%define dirname value", + "%description tools", + "tools", + "%install", + "echo %{dir%{suffix}}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsSurvivingLazyMacroDynamicReferenceMatchingRemovedDefinition(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define suffix name", + "%define root %{dir%{suffix}}", + "%package tools", + "%define dirname value", + "%description tools", + "tools", + "%install", + "echo %{root}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsAllowsUnrelatedDynamicMacroName(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%define suffix name", + "%package tools", + "%define unrelated value", + "%description tools", + "tools", + "%install", + "echo %{dir%{suffix}}", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%define suffix name", + "%install", + "echo %{dir%{suffix}}", + }, specification.rawLines) +} + +func TestMacroReferencesHonorsPercentEscapes(t *testing.T) { + assert.Empty(t, macroReferences("%%{helper} %%helper %%%%{helper} %%%%helper")) + assert.Equal(t, map[string]bool{"helper": true}, + macroReferences("%%%{helper} %%%helper %%%%%{helper} %%%%%helper")) + assert.Equal(t, map[string]bool{"outer": true}, macroReferences("%{outer %%{helper}}")) +} + +func TestRemoveSectionsDoesNotHoistEscapedMacroReferences(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define helper value", + "%description tools", + "tools", + "%install", + "echo %%{helper} %%helper", + }) + + require.NoError(t, specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + })) + assert.Equal(t, []string{ + "%install", + "echo %%{helper} %%helper", + }, specification.rawLines) +} + +func TestRemoveSectionsRejectsSelectedSelfReferentialGlobal(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%global toolsdir %{toolsdir}", + "%description tools", + "tools", + "%install", + "echo %{toolsdir}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Contains(t, err.Error(), "toolsdir") + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsSelectedGlobalWithDynamicName(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define suffix name", + "%define dirname value", + "%global selected %{dir%{suffix}}", + "%description tools", + "tools", + "%install", + "echo %{selected}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Contains(t, err.Error(), "dir%{suffix}") + assert.Equal(t, before, specification.rawLines) +} + +func TestRemoveSectionsRejectsSelectedGlobalDynamicNameMatchingSurvivingDefinition(t *testing.T) { + specification := newTreeAPISpec([]string{ + "%package tools", + "%define suffix name", + "%global selected %{dir%{suffix}}", + "%description tools", + "tools", + "%install", + "%define dirname value", + "echo %{selected}", + }) + before := append([]string(nil), specification.rawLines...) + + err := specification.mutateTree(func(tree *specTree) error { + return tree.RemoveSections(tree.SectionsByPackage("tools")) + }) + + require.ErrorIs(t, err, ErrUnsafeMacroHoist) + assert.Contains(t, err.Error(), "dir%{suffix}") + assert.Equal(t, before, specification.rawLines) +}