public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
To: git-commits@fedoraproject.org
Subject: [rpms/pyproject-rpm-macros] f44: getopt: Fix global macro clobbering with save/restore stack
Date: Fri, 24 Jul 2026 19:58:56 GMT [thread overview]
Message-ID: <178492313669.1.15224229863352511400.rpms-pyproject-rpm-macros-83ea9a1a0186@fedoraproject.org> (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_.
reply other threads:[~2026-07-24 19:58 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=178492313669.1.15224229863352511400.rpms-pyproject-rpm-macros-83ea9a1a0186@fedoraproject.org \
--to=git-commits@fedoraproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox