Bug 237448

Summary: %{!?bar: %define bar ...} construct broken by design, use %global instead
Product: [Fedora] Fedora Reporter: Axel Thimm <axel.thimm>
Component: rpmAssignee: Paul Nasrat <pnasrat>
Status: CLOSED NOTABUG QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: 6CC: mgarski
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-04-24 13:00:14 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Axel Thimm 2007-04-23 06:02:43 EDT
Description of problem:
%define foo() 0
%{!?bar: %define bar defined}
%define baz defined

Name:  testrpm
Version: 1
Release: 1
Summary: A test package

Group:  Some/Group
License: GPL

A test package

# If the next macro usage is commented it will work.
echo "%{foo x}"
echo "%{bar}"
echo "%{baz}"

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

How reproducible:

Steps to Reproduce:
1.rpmbuild -bp testrpm.spec
Actual results:
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.75380
+ umask 022
+ cd /usr/src/at/work/BUILD.teufli
+ export LANG
+ unset DISPLAY
+ echo 0
+ echo '%{bar}'
+ echo defined
+ exit 0

Expected results:
%{bar} should had been defined

Additional info:
o Replacing %define with %global seems to work. So the bug is not that
  the conditional macro is not run, but that for some reason the define
  happens at a deeper nesting level, and is later shadowed away.

o Defining parametrized macos does not trigger the bug, one must explicitely
  use them

o The order of when the paramatrized macro is used is relevant, if used after
  using the failing %{bar}, then bar is properly defined.
Comment 1 Jeff Johnson 2007-04-23 06:57:06 EDT
Use %global. There's no bug here, only useage inconsistent with what is implemented.

Comment 2 Axel Thimm 2007-04-23 07:13:19 EDT
I agree, usage is inconsitent with the implemenation, that's the definition of a
bug. But I think the bug is in the implemenation, not in the usage. I don't
think that this behaviour of cross-invalidating macros is intended.

If I'm wrong, then please clarify on what the proper usage is.
Comment 3 Ed Hill 2007-04-23 09:31:18 EDT
Hi folks, I do not understand RPM macros particularly well so I'm propably
unqualified to decide whether the above is an implementation bug (as Axel 
contends and I'm inclined to agree with) or improper usage (as Jeff claims).

However, I can say that this behavior is *mighty* difficult to grasp from 
a user perspective.  IMNSHO it totally violates "the principle of least 
surprise".  I spent hours yesterday trying to understand why the 
%{python_sitearch} macro in bz #199405 was defined early in a spec file 
and magically and very un-helpfully undefined later on.  If this is the 
way RPM macros are indeed supposed to work then it needs to be CLEARLY 
DOCUMENTED somewhere because other poor unwitting souls will trip over
this lovely feature.

And if it is documented somewhere then please post a link because repeated
Google searches failed to uncover any useful information for me yesterday.
Comment 4 Jeff Johnson 2007-04-23 13:15:39 EDT
The bugs is in lazily garbage collecting any/all macro definitions that were defined at level > 0
while returning from the expansion of a parameterized macro. A parameterized macro defines
several shell analogues (like %1, %2, %*, ...) while recursing downward, and undefines them
while recursing upwards.

Note that
    %{!?bar: %define bar defined}
is defined at recursion level 1, not level 0, because the definition is within another macro expansion.

So its arguable whether its a bug or a feature. The real flaw is neglecting garbage collection on
    %{!?bar: %define bar defined}
not that a later garbage collection undefines a macro defined at level 1.

Note that %global defines macros at level 0 so they are not garbage collected.

No matter what, writing any macro construct that depends on this (arguable) "bug" being
fixed is unlikely to build correctly anywhare.

Hence my comment

    There's ... only useage inconsistent with what is implemented.

Re: comment #3: The basic elements of macros (including defines at recursion level > 0)
are documented in /usr/share/doc/rpm-devel*/macros.

The usage of macros like %{python_sitearch} is packaging and distro specific, and needs to
documented at a packaging and/or distro basis. There's no way to document all macro
usage in the same sense that gcc is not responsible for documenting all C programs.
Comment 5 Jeff Johnson 2007-04-23 13:27:24 EDT
Re comment #3: Pardon, the path is /usr/share/doc/rpm*/macros, not rpm-devel.
Comment 6 Axel Thimm 2007-04-23 16:48:06 EDT
> So its arguable whether its a bug or a feature. The real flaw is neglecting
> garbage collection on
>     %{!?bar: %define bar defined}
> not that a later garbage collection undefines a macro defined at level 1.

OK, that actually makes a lot of sense and means that

%{!?bar: %define bar defined}

is semantically ill and only works by happenstance. So the true bugs are

a) that rpm doesn't always break on it and was too gentle on us, and that

b) users have found this working by trial and error and need to reeducated to
   only use %global inside of other macros (unless they intend to use scoped
   level-local %defines, which only advanced users would ever consider).

Jeff, is that view correct?

And thanks for the clarification on the garbage collection internals.
Comment 7 Jeff Johnson 2007-04-23 19:43:49 EDT
Yes, the squeaky clean technical cure for
  %{!?bar: %define bar defined}
behavior is gonna be worse than the disease was.

So use %global if you need a "working" construct.

FWIW, you might check this dialectal variant. I *think* it %define's at level 0
(but I've not checked)
   %{expand:%{!?bar:%%define bar defined}}

There are other flaws in parameterized macros, particularly when parameterized
macros recurse to other parameterized macros with different numbers of args.
Unlike sheel, %1, %2, %3, etc will be leaked to the child recursion, even though new
definitions of %1, %2, %3 ... will be pushed/popped as needed.
Comment 8 Ed Hill 2007-04-24 12:49:17 EDT
Hi Jeff, thank you for taking the time to explain the above.  Its 
very helpful.
Comment 9 Axel Thimm 2007-04-24 13:00:14 EDT

Closing this bug as it isn't really an rpm bug. It could be used as a blocker
for all packages using that idiom, but there are just too many.