[rpms/pyproject-rpm-macros] f44: getopt: Fix global macro clobbering with save/restore stack

[rpms/pyproject-rpm-macros] f44: getopt: Fix global macro clobbering with save/restore stack

[@ 2026-07-24 19:58 UTC (permalink / raw)]

            A new commit has been pushed.

            Repo   : rpms/pyproject-rpm-macros
            Branch : f44
            Commit : 83ea9a1a0186a271d968297df63acb411311e094
            Author : Miro Hrončok <miro@hroncok.cz>
            Date   : 2026-07-24T16:05:06+02:00
            Stats  : +158/-23 in 4 file(s)
            URL    : https://src.fedoraproject.org/rpms/pyproject-rpm-macros/c/83ea9a1a0186a271d968297df63acb411311e094?branch=f44

            Log:
            getopt: Fix global macro clobbering with save/restore stack

When calling one getopt-using macro from another,
the macro definitions from the inner macro leaked to the outer one.

rpm.define()/rpm.undefine() operate globally, not scoped to parametric
macro invocations. When a macro using getopt() is called nested inside
another getopt-using macro with overlapping option letters, the inner
cleanup_macros() wipes the outer macro's __pyproject_opt_* values.

Note: RPM's native %define/%undefine have stack-like behavior (pushing
and popping definitions), and we can invoke %define from Lua via
rpm.expand(). However, there is no way to push "not defined" onto
this stack and later pop it. Since getopt treats "not defined" and
"defined to %{nil}" differently (the former means the option was not
passed, the latter means a flag was passed without a value), the
native stacking cannot represent the full state space. We implement
our own save/restore stack on the Lua side instead.

Add a Lua-side save/restore stack: getopt() saves current macro state
before cleanup, and a new restore() function pops and reinstates it.
All getopt-using macros call %{__pyproject_getopt_restore} at the end
of their body.

Assisted-By: Claude Opus 4.6
Assisted-By: Codex

---
diff --git a/macros.pyproject b/macros.pyproject
index cb36d2e..3fe295b 100644
--- a/macros.pyproject
+++ b/macros.pyproject
@@ -23,6 +23,8 @@
 # PEP 503 name normalization: lowercase, one or more [-_.] → single -
 %__pyproject_normalize_name(-) %{lua:print((rpm.expand("%1"):lower():gsub("[%-_.]+", "-")))}
 
+%__pyproject_getopt_restore %{lua:require("fedora.rpm.pyproject_getopt").restore()}
+
 %_pyproject_files_base %{_builddir}/%{_pyproject_files_prefix}-pyproject-files
 %_pyproject_modules_base %{_builddir}/%{_pyproject_files_prefix}-pyproject-modules
 %_pyproject_ghost_distinfo_base %{_builddir}/%{_pyproject_files_prefix}-pyproject-ghost-distinfo
@@ -30,17 +32,17 @@
 %pyproject_files(-) %{lua:
 require("fedora.rpm.pyproject_getopt").getopt({
     {short="D", long="dist-name", value=true},
-})}%{_pyproject_files_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}
+})}%{_pyproject_files_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}%{__pyproject_getopt_restore}
 
 %_pyproject_modules(-) %{lua:
 require("fedora.rpm.pyproject_getopt").getopt({
     {short="D", long="dist-name", value=true},
-})}%{_pyproject_modules_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}
+})}%{_pyproject_modules_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}%{__pyproject_getopt_restore}
 
 %_pyproject_ghost_distinfo(-) %{lua:
 require("fedora.rpm.pyproject_getopt").getopt({
     {short="D", long="dist-name", value=true},
-})}%{_pyproject_ghost_distinfo_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}
+})}%{_pyproject_ghost_distinfo_base}%{?__pyproject_opt_D:-%{__pyproject_normalize_name %{__pyproject_opt_D}}}%{__pyproject_getopt_restore}
 
 %_pyproject_record %{_builddir}/%{_pyproject_files_prefix}-pyproject-record
 %_pyproject_buildrequires %{_builddir}/%{_pyproject_files_prefix}-pyproject-buildrequires
@@ -71,7 +73,7 @@ mkdir -p "%{_pyproject_builddir}"
 TMPDIR="%{_pyproject_builddir}" \\\
 %{__python3} -Bs %{_rpmconfigdir}/redhat/pyproject_wheel.py %{?**} %{_pyproject_wheeldir}
 %{?__pyproject_opt_d:popd}
-}
+%{__pyproject_getopt_restore}}
 
 
 %pyproject_build_lib %{!?__pyproject_build_lib_warned:%{warn:The %%{pyproject_build_lib} macro is deprecated.
@@ -183,7 +185,7 @@ fi
   --pyproject-record "%{_pyproject_record}" \\
   --prefix "%{_prefix}" \\
   %{**}
-}
+%{__pyproject_getopt_restore}}
 
 %pyproject_check_import(-) \
 %{lua:require("fedora.rpm.pyproject_getopt").getopt({
@@ -204,7 +206,7 @@ if [ ! -f "%{_pyproject_modules %{?__pyproject_optflag_D}}" ]; then
   exit 1
 fi
 %py3_check_import -f "%{_pyproject_modules %{?__pyproject_optflag_D}}" %{?__pyproject_optflag_e} %{?__pyproject_optflag_t} %{?__pyproject_positional_args}
-}
+%{__pyproject_getopt_restore}}
 
 
 %_pyproject_check_import_allow_no_modules(-) \
@@ -217,7 +219,8 @@ if [ -z "$(cat %{_pyproject_modules %{?__pyproject_optflag_D}})" ]; then\
   echo "No modules to check found, exiting check"\
 else\
   %pyproject_check_import %{?**}\
-fi
+fi\
+%{__pyproject_getopt_restore}
 
 
 %default_toxenv py%{python3_version_nodots}
@@ -293,7 +296,7 @@ fi
 # Incomplete .dist-info dir might confuse importlib.metadata
 rm -rfv *.dist-info/ >&2
 %{?__pyproject_opt_d:popd >&2}
-}
+%{__pyproject_getopt_restore}}
 
 
 %tox(-) \
@@ -308,7 +311,7 @@ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_
 %{?__pytest_addopts:PYTEST_ADDOPTS="${PYTEST_ADDOPTS:-} %{__pytest_addopts}"}} \\
 HOSTNAME="rpmbuild" \\
 %{__python3} -m tox --current-env --assert-config -q --recreate -e "%{?__pyproject_opt_e}%{!?__pyproject_opt_e:%{toxenv}}" %{?__pyproject_positional_args}
-}
+%{__pyproject_getopt_restore}}
 
 
 # Declarative buildsystem, requires RPM 4.20+ to work

diff --git a/pyproject-rpm-macros.spec b/pyproject-rpm-macros.spec
index f625e1f..8deb49c 100644
--- a/pyproject-rpm-macros.spec
+++ b/pyproject-rpm-macros.spec
@@ -14,8 +14,8 @@ License:        MIT
 #   Increment Y and reset Z when new macros or features are added
 #   Increment Z when this is a bugfix or a cosmetic change
 # Dropping support for EOL Fedoras is *not* considered a breaking change
-Version:        1.23.0
-Release:        2%{?dist}
+Version:        1.23.1
+Release:        1%{?dist}
 
 # Macro files
 Source:         macros.pyproject
@@ -179,6 +179,9 @@ export HOSTNAME="rpmbuild"  # to speedup tox in network-less mock, see rhbz#1856
 
 
 %changelog
+* Thu Jul 23 2026 Miro Hrončok <mhroncok@redhat.com> - 1.23.1-1
+- getopt: Fix global macro clobbering with save/restore stack
+
 * Thu Jul 16 2026 Fedora Release Engineering <releng@fedoraproject.org> - 1.23.0-2
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_45_Mass_Rebuild
 

diff --git a/pyproject_getopt.lua b/pyproject_getopt.lua
index 861cb25..d5bf986 100644
--- a/pyproject_getopt.lua
+++ b/pyproject_getopt.lua
@@ -213,6 +213,7 @@ local function cleanup_macros(opt_spec)
         rpm.undefine("__pyproject_opt_" .. spec.short)
         rpm.undefine("__pyproject_optflag_" .. spec.short)
     end
+    rpm.undefine("__pyproject_positional_args")
 end
 
 
@@ -242,16 +243,18 @@ local function quote_values(values)
     return parts
 end
 
--- Define %__pyproject_opt_{short} and %__pyproject_optflag_{short} for each found option.
+-- Define %__pyproject_opt_{short}, %__pyproject_optflag_{short}, and
+-- %__pyproject_positional_args from a parse result.
+-- Takes a table {found, opt_spec, positional}.
 -- opt: Flags get %{nil} (defined but empty), value options get each individual
 --      value joined by separator.
 -- optflag: Ready-to-forward form: "-X" for flags, "-X value" for value options.
 -- See the quote_value function about how values are quoted.
-local function define_macros(found, opt_spec)
-    for _, spec in ipairs(opt_spec) do
-        if found[spec.short] then
+local function define_macros(state)
+    for _, spec in ipairs(state.opt_spec) do
+        if state.found[spec.short] then
             if spec.value then
-                local parts = quote_values(found[spec.short])
+                local parts = quote_values(state.found[spec.short])
                 local value = table.concat(parts, spec.separator)
                 rpm.define("__pyproject_opt_" .. spec.short .. " " .. value)
                 rpm.define("__pyproject_optflag_" .. spec.short .. " -" .. spec.short .. " " .. value)
@@ -261,6 +264,28 @@ local function define_macros(found, opt_spec)
             end
         end
     end
+    if #state.positional > 0 then
+        local parts = quote_values(state.positional)
+        rpm.define("__pyproject_positional_args " .. table.concat(parts, " "))
+    end
+end
+
+
+-- Save/restore stack for nested getopt calls.
+-- rpm.define()/rpm.undefine() are global (not scoped to parametric macros),
+-- so an inner getopt cleanup clobbers outer values. We save the structured
+-- parse result and re-run define_macros() in restore(), avoiding any
+-- rpm.expand() round-trip that would strip %{quote:} wrappers.
+local _current = nil  -- {found, opt_spec, positional} from the most recent getopt()
+local _save_stack = {} -- stack of {found, opt_spec, positional} tables
+
+function M.restore()
+    if _current then
+        cleanup_macros(_current.opt_spec)
+    end
+    _current = table.remove(_save_stack)
+    if not _current then return end
+    define_macros(_current)
 end
 
 
@@ -274,8 +299,8 @@ function M.getopt(opt_spec, exclusion_rules, tokens, macro_name)
     tokens = normalize_tokens(tokens or M.rpm_args())
     macro_name = macro_name or rpm.expand("%0")
     local by_short, by_long = build_lookup(opt_spec)
+    _save_stack[#_save_stack + 1] = _current
     cleanup_macros(opt_spec)
-    rpm.undefine("__pyproject_positional_args")
 
     local result = parse(macro_name, tokens, by_short, by_long)
     if not result then return end
@@ -286,12 +311,8 @@ function M.getopt(opt_spec, exclusion_rules, tokens, macro_name)
         end
     end
 
-    define_macros(result.found, opt_spec)
-
-    if #result.positional > 0 then
-        local parts = quote_values(result.positional)
-        rpm.define("__pyproject_positional_args " .. table.concat(parts, " "))
-    end
+    _current = {found = result.found, opt_spec = opt_spec, positional = result.positional}
+    define_macros(_current)
 end
 
 return M

diff --git a/test_pyproject_getopt.lua b/test_pyproject_getopt.lua
index dcd1742..1bff602 100644
--- a/test_pyproject_getopt.lua
+++ b/test_pyproject_getopt.lua
@@ -448,6 +448,114 @@ function M.test_optflag_cleanup_between_calls()
 end
 
 
+-- Nested macro scope: save/restore prevents inner getopt from clobbering outer values
+
+function M.test_nested_getopt_with_restore_preserves_outer_opts()
+    rpm.define([[_test_inner(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}inner%{lua:require("pyproject_getopt").restore()}]]
+    )
+    rpm.define([[_test_outer(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_inner}/%{?__pyproject_opt_d}]]
+    )
+
+    local result = rpm.expand("%_test_outer -d hello")
+    assert(result == "inner/hello",
+        "expected 'inner/hello', got '" .. result .. "'")
+end
+
+function M.test_nested_getopt_with_restore_preserves_outer_optflags()
+    rpm.define([[_test_inner2(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}inner%{lua:require("pyproject_getopt").restore()}]]
+    )
+    rpm.define([[_test_outer2(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_inner2}/%{?__pyproject_optflag_d}]]
+    )
+
+    local result = rpm.expand("%_test_outer2 -d hello")
+    assert(result == "inner/-d hello",
+        "expected 'inner/-d hello', got '" .. result .. "'")
+end
+
+function M.test_nested_getopt_without_restore_clobbers()
+    rpm.define([[_test_inner3(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}inner]]
+    )
+    rpm.define([[_test_outer3(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_inner3}/%{?__pyproject_opt_d}]]
+    )
+
+    local result = rpm.expand("%_test_outer3 -d hello")
+    -- Without restore(), inner getopt clobbers outer's value
+    assert(result == "inner/",
+        "expected 'inner/' (clobbered without restore), got '" .. result .. "'")
+end
+
+function M.test_nested_getopt_with_restore_preserves_outer_positional_args()
+    rpm.define([[_test_inner4(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}inner%{lua:require("pyproject_getopt").restore()}]]
+    )
+    rpm.define([[_test_outer4(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_inner4}/%{?__pyproject_positional_args}]]
+    )
+
+    local result = rpm.expand("%_test_outer4 -d hello world")
+    assert(result == "inner/world",
+        "expected 'inner/world', got '" .. result .. "'")
+end
+
+function M.test_nested_getopt_with_different_values()
+    rpm.define([[_test_inner5(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{?__pyproject_opt_d}%{lua:require("pyproject_getopt").restore()}]]
+    )
+    rpm.define([[_test_outer5(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_inner5 -d other}/%{?__pyproject_opt_d}]]
+    )
+
+    local result = rpm.expand("%_test_outer5 -d hello")
+    assert(result == "other/hello",
+        "expected 'other/hello', got '" .. result .. "'")
+end
+
+function M.test_triple_nested_getopt_restores_all_levels()
+    rpm.define([[_test_innermost(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{?__pyproject_opt_d}%{lua:require("pyproject_getopt").restore()}]])
+    rpm.define([[_test_middle(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_innermost -d deep}+%{?__pyproject_opt_d}%{lua:require("pyproject_getopt").restore()}]])
+    rpm.define([[_test_outermost(-) %{lua:
+        require("pyproject_getopt").getopt({
+            {short="d", long="directory", value=true},
+        })}%{_test_middle -d mid}+%{?__pyproject_opt_d}]])
+
+    local result = rpm.expand("%_test_outermost -d top")
+    assert(result == "deep+mid+top",
+        "expected 'deep+mid+top', got '" .. result .. "'")
+end
+
+
 -- Raw error functions (without pcall) for Python-side stderr checking.
 -- These are not discovered by list() since they don't start with test_.