Parsing a specfile multiple times leaves the lua tables dirty

[hroncok]

So, I recently discovered a weird sitation.

Consider https://src.fedoraproject.org/rpms/fedora-obsolete-packages with this:

%obsolete_ticket https://bugzilla.redhat.com/show_bug.cgi?id=2307970
%obsolete gimp-save-for-web 0.29.3-20
%obsolete_ticket https://bugzilla.redhat.com/show_bug.cgi?id=2307966
%obsolete gimp-layer-via-copy-cut 1.6-27
%obsolete_ticket https://bugzilla.redhat.com/show_bug.cgi?id=2307966
%obsolete gimp-layer-via-copy-cut 1.6-30

Regular RPM passing errors like this -- this is expected and correct:

$ rpmspec -P fedora-obsolete-packages.spec
error: gimp-layer-via-copy-cut obsoleted multiple times (1.6-27 and 1.6-30).
error: lua script failed: [string "obsolete"]:27: error expanding macro

However, rpmautospec (e.g. via fedpkg verrel) errors like this:

$ fedpkg verrel
Could not execute verrel: Couldn’t parse spec file fedora-obsolete-packages.spec:
error: gimp-save-for-web obsoleted multiple times (0.29.3-20 and 0.29.3-20).
error: lua script failed: [string "obsolete"]:27: error expanding macro

This is certainly unexpected: gimp-save-for-web is not obsoleted multiple times.

Adding this to the beginning of the spec:

%{lua:obs = {}}

Makes the problem go away.

What happens is that the specfile does not suspect it will be parsed twice with the same Lua state. But rpmautospec does that in https://github.com/fedora-infra/rpmautospec/blob/31978b4832bda151652332bc298c4b59a065f426/rpmautospec/pkg_history.py#L158

...which is kinda my doing :/

The easy solution is to move the macro definitions into the for:

https://github.com/fedora-infra/rpmautospec/blob/31978b4832bda151652332bc298c4b59a065f426/rpmautospec/pkg_history.py#L133-L140

And call rpm.reloadConfig() after each attempt.

However, that might be overkill. I will see if we can "restart" the Lua state somehow.

[pmatilai]

It's not just Lua state, spec parsing also affects global macro state so parsing specs without resetting the entire rpm context in between can and will yield different results. So, rpm.reloadConfig() before another parse (whether the same spec or different) is the right thing to do, as blunt as that hammer may seem.

I would love to have a have the entire spec parse context isolated to the spec object itself, but that's a long road.

[pmatilai]

Come to think of it, this little fact that you need to call rpm.reloadConfig() probably isn't documented anywhere...

[nphilipp]

Good catch, thanks for digging into the problem and implementing a solution!