Bug 500085

Summary: specfile macros with parameters corrupt macro scoping/visibility
Product: [Fedora] Fedora Reporter: Carl Roth <roth>
Component: rpmAssignee: Panu Matilainen <pmatilai>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 10CC: ffesti, jnovy, n3npq, pmatilai
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-05-18 09:48:42 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
RPM spec file none

Description Carl Roth 2009-05-10 19:57:14 UTC
Description of problem:

I discovered this strange behavior today while trying to construct a specfile that uses 'kmodtool' from rpmfusion...  I narrowed down the failing behavior to the following:

* spec file header defines an indirect macro using %{expand:...}
* unrelated macros invoked in the spec file header change the scope of the previously-defined macro.  Specifically, if the macro invocation is defined to take parameters (uses parens and getopt()) then some variables are no longer visible in the rest of the spec
* the behavior is order-dependent.  Moving the macro definitions around changes the result, even if the macros are not related.

Version-Release number of selected component (if applicable):

Affects RPM shipped with F9, F10 and F11.

How reproducible:

Always.  See attached spec.

Steps to Reproduce:
1. edit spec file to expose '__macro1' and/or '__macro2'
2. rpmbuild -bp foo.spec
3. See variable expansions that result
  
Actual results:


Expected results:

Additional info:

Comment 1 Carl Roth 2009-05-10 19:57:58 UTC
Created attachment 343294 [details]
RPM spec file

Comment 2 Jeff Johnson 2009-05-10 22:05:00 UTC
The answer is likely (you've neglected to supply details)
to use %global rather than %define.

Much like C automatic variables, macro definitions have
a scope, basically the recursion depth at which they are defined.

And like C auto variables, macros can/will be garbage collected
when they go out of scope.

Using %global rather than %define ensures the definition is
at recursion level 0. Otherwise %global and %define are identical.

And yes, macros are processed as encountered, including the
primitives of %global and %define, and so are always encounter
order dependent.

Comment 3 Jeff Johnson 2009-05-10 22:11:24 UTC
Ah I missed the spec file attachment.

Note that there is no reason to use %{expand:...}
in the example you have given.

Note also that using %{expand:...} introduces an
additional recursion level with %define as described.

Either removing the unnecessary %expand, or using %global
instead of %define, likely "fixes" (untested).

Note also that parameterized macros like %define foo() ... are
more likely to trigger garbage collection issues than otherwise,
because the arguments like %1, %2, etc are *always* garbage
collected while return from expansion recursions.

Comment 4 Carl Roth 2009-05-10 23:32:19 UTC
(In reply to comment #3)

> Note that there is no reason to use %{expand:...}
> in the example you have given.

In my case I was using kmodtool, which has the form

  %{expand:%(kmodtool ...)}

> Either removing the unnecessary %expand, or using %global
> instead of %define, likely "fixes" (untested).

So I can paraphrase by saying that kmodtool should emit '%global' instead of '%define'?

> Note also that parameterized macros like %define foo() ... are
> more likely to trigger garbage collection issues than otherwise,
> because the arguments like %1, %2, etc are *always* garbage
> collected while return from expansion recursions.  

I'm not sure I understand this statement.

Comment 5 Carl Roth 2009-05-10 23:38:06 UTC
(In reply to comment #2)

> The answer is likely (you've neglected to supply details)
> to use %global rather than %define.

Yes, it the case that changing everything to '%global' fixes the issue.

> Much like C automatic variables, macro definitions have
> a scope, basically the recursion depth at which they are defined.

"recursion depth"?  What does this mean?  I would expect that if I '%define' a macro in the spec headers, it should have the same scope, no matter which other macros I define or invoke later on.

> And like C auto variables, macros can/will be garbage collected
> when they go out of scope.

Again, I would expect that macros defined at the level of the SPEC headers would be effectively global.  I do agree with you though that macros defined *inside* a stanza (%prep etc.) have a local scope.
 
> Using %global rather than %define ensures the definition is
> at recursion level 0. Otherwise %global and %define are identical.

My confusion here is illustrated by the example I attached. -- expanding other macros *after* the initial definitions should not change the "recursion level" of the original definitions.  This is starting to sound more like m4/Sendmail type macro languages, which do have nasty which-eval-level-am-i-on issues.

Comment 6 Jeff Johnson 2009-05-10 23:42:55 UTC
1) Try %global and see. You are correct that %{expand:...} is needed
if macro values, not the macro name itself, are needed.

2) Yes.

3) I was trying to explain your observations:

* unrelated macros invoked in the spec file header change the scope of the
previously-defined macro.  Specifically, if the macro invocation is defined to
take parameters (uses parens and getopt()) then some variables are no longer
visible in the rest of the spec
* the behavior is order-dependent.  Moving the macro definitions around changes
the result, even if the macros are not related.

What is likely happening is that you are moving garbage collection that
is *always* done after a parameterized macro (you  know as "parens and getopt()")
expansion return is collecting a %define that has gone out of scope at the same time
as  %1, %2, ... are being garbage collected.

Comment 7 Jeff Johnson 2009-05-10 23:54:02 UTC
Rule #1: Macros are not variables.
   %define foo bar
   %{echo: "foo #1 has value %foo"}
   %define foo baz
   %{echo: "foo #2 has value %foo"}
   %undefine foo
   %{echo: "foo #3 has value %foo"}

I.e. %define is a "push", %undefine is a "pop".

Rule #2: Macros are recursively expanded lazily.
   %define foo %{bar}
   %define bar baz
   %define bing %foo
   %{echo: "bing has value %bing"}

Note the corollary buried in "lazily", as parameterized macros
are passed unexpanded, contrary to what you likely expect.
That's also why %{expand:...} exists, to force an expansion where needed.

So most definitely macros have a scope == the recursion level at which
they are defined. One can also have macros that %define other macros,
which are then cleared by garbage collection.

So your expectations are most definitely not in line with what is implemented.

And if that's not to your expectations, you likely can accomplish almost anything you
need to do in shell, using envvar's, instead.

Comment 8 Panu Matilainen 2009-05-18 09:48:42 UTC
> Using %global rather than %define ensures the definition is
> at recursion level 0. Otherwise %global and %define are identical.

They differ also in that the body of a %global is expanded at definition time whereas %define is lazily expanded. Might not make a difference here... but yes you'll might want to file a bug against kmodtool for this.