feat(spec): preserve referenced macros during removal

Author: trungams

internal/rpm/spec/structural_tree_api.go

@@ -183,6 +183,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

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 <builder@example.com> - 1.0-1
+- Initial fixture.

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 <builder@example.com> - 1.0-1
+- Initial fixture.

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 <builder@example.com> - 1.0-1
+- Initial fixture.

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}\