Oldies but goodies: the max depth is already initialized in declaration
to _MAX_MACRO_DEPTH which has been the same 16 since late nineties. This
wasn't always the case of course...
Send all through rpmlog(), unify formatting - previously echo and
warn were missing the trailing newline. The other change is that
echo now goes to stdout.
This simply breaks too many things - whole macro ecosystems exist based
on the assumption that quotes in arguments will pass to macros
untouched. Macro argument quoting support is necessary but it'll
need some entirely different approach that is either opt-in or
based on a different syntax altogether.
This reverts commit 47f0a899ef.
Some of scoping level values are defined in rpmmacros.h:
#define RMIL_DEFAULT -15
#define RMIL_MACROFILES -13
...
#define RMIL_GLOBAL 0
Before this patch RPM set these values decreased of 1 (for all macros).
Because of the (level - 1) in
pushMacro(dst, me->name, me->opts, me->body, (level - 1), me->flags);
in function copyMacros scoping level for some CMDLINE macros was decreased of 1.
Because of the (level - 1) in
pushMacro(mb->mc, n, o, b, (level - 1), ME_NONE);
in function doDefine scoping level values of all other macro types and
some CMDLINE macros was decreased of 1.
The patch shifts scoping level values back to the numbers defined
in rpmmacros.h.
This was spotted when investigating a regression introduced in
commit 237f35f16f where locally scoped
scoped macros did not get deleted on scope closing.
This also fixes the regression in question.
The first test in this patch tests automatic deleting of %defined macros.
The second test checks whether the behavior changed by
237f35f16f is still working correctly.
Thanks to Panu Matilainen for investigation and proposal of the main part
of the patch!
Single or double quotes can be used in macro arguments
as expected - they denote the begin and the end of the argument.
Example with 2-nd and 3-rd argument in double quotes:
./rpm --define "%foo() 1:%1 2:%2 3:%3" \
--eval '%foo Next_argument_will_be_empty "" "Last argument"'
1:Next_argument_will_be_empty 2: 3:Last argument
Example with all arguments in single quotes,
without spaces between arguments:
./rpm --define "%foo() 1:%1 2:%2 3:%3" \
--eval "%foo 'Next argument will be empty''''Last argument'"
1:Next argument will be empty 2: 3:Last argument
This essentially reverts commit 9ae7d1df31.
Leaving the testcase in place, just adjusting it to the expected
behavior where arguments that expand to empty are eaten away.
This actually matches what the shell does, so it follows the path of
least surprise in that sense - we emulate shell behavior with the
arguments etc already in many ways. What the shell has and rpm
has not is a way to pass those empty arguments by quoting. So
now we need a way to quote arguments - not just for passing empty
arguments but arguments containing whitespaces too.
There needs to be a way to pass escaped macros names (and other strings
containging %%) as parametric macro arguments. Add an internal mode
to preserve %% when expanding the arguments. Fixes a regression introduced
in commit 5adc56897b.
Thanks to Michael Schroeder for pointing this out!
This makes makes the behavior much more sensible eg in case an
argument expands to an empty strings, such as:
$ rpm --define '%foo() 1:%1 2:%2' --eval '%foo %nil bar'
After commit 5adc56897b rpm was
evaluating that to "1:bar 2:%2" which is hardly what you'd expect.
Thanks to Michael Schroeder for pointing this out! (#217)
This too is quite a fundamental change for macros: up to now, arguments
to parametric macros have not been expanded. It doesn't sound so bad
until you consider something like the case in RhBug:1397209:
%global rev 133
...
%setup %{?rev:-c}
%autosetup %{?rev:-c}
One would expect %setup and %autosetup behave the same when you replace
one with the other, but no. %setup gets "called" with -c, %autosetup
does not. Instead %autosetup receives a completely useless, literal
"%{?rev:-c}" as its argument. That's just brain-meltingly non-sensical.
This certainly has the potential to break advanced macro constructs,
but OTOH what breaks might well be only written that way in order to
work around the former behavior.
There are some funny subtleties involved as the argument expansion
must occur in the callers scope, ie before we create any of the
automatic macros. For example, Fedora's font packages break if only
this or the macro scope visibility enforcement is applied but start
working again once both are present.
When a parametric macro "calls" another parametric macro with fewer
arguments than it received, the inner macro would see the %<n>
macros of the outer call which is an obvious scoping violation
and quirky behavior, making macro writing harder than it needs be.
Similar scoping issues exist for manually defined macros but those
have further complications (because of %undefine semantics) that we
sheepishly avoid here by limiting the visibility enforcing to automatic
macros only (for now at least).
This changes the macro scoping rules quite fundamentally: macro
definitions are local to the parametric macro they were defined in,
and everything else is global. Among other things, this makes this
common spec idiom (RhBug:552944, RhBug:551971 etc) behave
deterministically because "foo" is placed into global scope:
%{?!foo: %define foo bar}
In theory it's certainly possible that this breaks somebodys carefully
crafted advanced macros but it seems quite unlikely, considering how
broken the alleged block-scoping has been. OTOH for macros defined
within parametric macros, nothing actually changes as that scoping has
always been enforced by rpm. The non-global define tracking is also
no longer needed for emitting warnings, because the case where it
was needed simply no longer exists.
Note that macro recursion depth is a different thing and still needs
to be tracked separately.
Since we actually setup all the same automatic macros whether there
are arguments or not, doing it centrally only makes sense. Shuffle
things around a bit in preparation for the next steps.
The missing space style-error has been recently coming common enough
somebody might think that IS the expected style, its not. Some of these
are actually very old, but fix across the board for consistency..
Strictly white-space only change.
The whole point of %trace is that some macro is being evaluated in a
surprising way, and you want to know what it is. This is often the
result of an unfortunately complex macro with difficult to see
intermediate results.
Shortening the output because it doesn't fit past 61 or so characters
does not help, and we have big screens these days. Don't do it.
Signed-off-by: Peter Jones <pjones@redhat.com>
In %trace mode, evaluating a macro which is undefined causes an invalid
read of 1 byte when searching for the end of the string:
trillian:~$ valgrind rpmspec --eval '%trace' --eval '%{?myUndefinedMacro}'
==21534== Memcheck, a memory error detector
==21534== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==21534== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==21534== Command: rpmspec --trace --eval %{?myUndefinedMacro}
==21534==
1> %{?myUndefinedMacro}^==21534== Invalid read of size 1
==21534== at 0x55018D4: printMacro (macro.c:296)
==21534== by 0x5502DFC: expandMacro (macro.c:1077)
==21534== by 0x5503710: doExpandMacros (macro.c:1280)
==21534== by 0x5504AB6: rpmExpand (macro.c:1629)
==21534== by 0x508F59A: rpmcliAllArgCallback (poptALL.c:120)
==21534== by 0x6DAF71D: invokeCallbacksOPTION (popt.c:156)
==21534== by 0x6DAF75B: invokeCallbacksOPTION (popt.c:139)
==21534== by 0x6DB1428: poptGetNextOpt (popt.c:1515)
==21534== by 0x508F912: rpmcliInit (poptALL.c:302)
==21534== by 0x1095B2: main (rpmspec.c:63)
==21534== Address 0x8a010f3 is 0 bytes after a block of size 19 alloc'd
==21534== at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
==21534== by 0x5507C17: rmalloc (rpmmalloc.c:44)
==21534== by 0x5502788: expandMacro (macro.c:927)
==21534== by 0x5503710: doExpandMacros (macro.c:1280)
==21534== by 0x5504AB6: rpmExpand (macro.c:1629)
==21534== by 0x508F59A: rpmcliAllArgCallback (poptALL.c:120)
==21534== by 0x6DAF71D: invokeCallbacksOPTION (popt.c:156)
==21534== by 0x6DAF75B: invokeCallbacksOPTION (popt.c:139)
==21534== by 0x6DB1428: poptGetNextOpt (popt.c:1515)
==21534== by 0x508F912: rpmcliInit (poptALL.c:302)
==21534== by 0x1095B2: main (rpmspec.c:63)
==21534==
1> %{?_transaction_color}^
1> %{?_prefer_color}^
1> %{_netsharedpath}^
1> %{_install_langs}^
==21534==
==21534== HEAP SUMMARY:
==21534== in use at exit: 7,183 bytes in 71 blocks
==21534== total heap usage: 7,811 allocs, 7,740 frees, 3,500,361 bytes allocated
==21534==
==21534== LEAK SUMMARY:
==21534== definitely lost: 19 bytes in 1 blocks
==21534== indirectly lost: 0 bytes in 0 blocks
==21534== possibly lost: 0 bytes in 0 blocks
==21534== still reachable: 7,164 bytes in 70 blocks
==21534== suppressed: 0 bytes in 0 blocks
==21534== Rerun with --leak-check=full to see details of leaked memory
==21534==
==21534== For counts of detected and suppressed errors, rerun with: -v
==21534== ERROR SUMMARY: 5 errors from 1 contexts (suppressed: 0 from 0)
trillian:~$
This can easily be avoided by checking the first byte as well as the
second for our sentinal value (NUL).
Signed-off-by: Peter Jones <pjones@redhat.com>
When a %{lua:...} macro calls rpm.expand(), it re-enters the macro
environment at depth zero regardless of what the "calling" nesting
level was. This could lead to wrong macros getting mopped out on
complex, nested lua + native macro calls.
Store the depth in global macro context and pick up the initial value
from there to keep the nesting level increasing on when entering %{lua:...}
space.
These haven't been in any public release yet so there's time to
fiddle with stuff like this. Both always succeed currently so no
change in that respect but it's better to have room for expansion...
This WILL cause behavior changes in cases where there have been
illegal macro names etc which have previously been more or less
ignored with an logged error, but now they will actually abort.
With direct rc assignments there's a risk that a previously
flagged error is overwritten by a success, only flagging errors
specifically avoids that. It also makes things simpler in some ways.
This probably changes *some* behavior wrt errors, in that errors
might actually get counted as such.
These are not deprecated at all no matter what the header has been
saying for the past 15+ years, they're used by rpm itself all over
the place as rpmDefineMacro() serves a slightly different purpose
and there's no rpmUndefineMacro() anyway.
Lets make 'em into proper citizens and move them into rpm namespace,
and while at it, call the operations push and pop since that's much
closer to what actually happens.
Finally, add simple wrapper macros to keep external code compilable
while getting the non-namespaced stuff out of ABI.
This allegedly been deprecated for no less than sixteen years,
but until very recently it's been in use by rpm itself and only
now with rpmExpandMacros() its possible to eliminate this one.
All sane external callers have been using rpmExpand() anyway
since expandMacros() is "buffer limited".
Now rpmExpandMacros() returns integer as a return value. Negative return
value implies failure. Return values can be more utilized in the future.
E. g. return value could specify how many macros were expanded in given
string.
This removes a seemingly undocumented, and not even well defined, limit
on the size of a macro expansion when parsing a spec file.
[lkardos@redhat.com: created new funtion expandMacrosU() (Unlimited)
instead of modifying expandMacros() in order not to change API/ABI]
Signed-off-by: Lubos Kardos <lkardos@redhat.com>
The problem evinced itself when somebody tried to use the macro
expansion on the string "%!". The problem was revealed by compiling
with "--fsanitize=memory" (rhbz:#1260248).
- Track non-global macro definitions in a helper variable, this
eliminates the need for costly macro table walking at end of each
expansion cycle as we only bother doing it if non-global macros
were defined during a given expansion cycle.
- Commit 1bdcd05008 to fix RhBug:1045723
broke some funky java macros in Fedora which include line continuation
in the argument (comments 6-7 in the bug). That it ever worked seems
far more like luck than by design but since this seems to fix it...
- Test for terminating ')' existence before copying, otherwise we'll
end up walking over the edge of the world.
- Return address from doDefine() on error will likely differ after
this, whether that actually affects anything remains to be seen...
- Both doDefine() and doUndefine() assumed the macro string would
always fit into MACROBUFSIZ, which of course is true for any
normal use but we cant make such assumptions.
- In the case of %define/%global there are various other overrun-issues
that need further changes to fix.
- In the current macro implementation the check is simply far too
expensive to leave on always. Its useful though, so enable it
when tracing macro expansion, in which case you're probably
troubleshooting some macro-issues...
- Reword the message to something hopefully little more understandable,
change from error to a warning (only matters for output)
- Check unused macros at end of every scope, but unlike with parametrized
macros, dont actually delete
- Only whine once per unused macro due to the above to avoid spurious
output due to above
- This catches the common error in specs where %define is used in a scope
unintentionally, eg "%{!?foo: %define foo 1}" where the just defined
macro should actually fall out of scope upon the closing }, but
only gets erased if any parametrized macro is "called" and causes
insane behavior such as described in RhBug:552944. So at least we'll
warn on these situations.
- In practise doesn't change anything visible as the "macro not used"
message is disabled, but tracking a flag bit is saner than
strlen() + strchr() + bunch of other stuff which isn't even correct:
prior to this %## would've triggered "unused" errors which is plain
wrong, and complaining about unused %1 %2 ... isn't really right
either.
- Revert back to pre commit c22d5b1299
state wrt macro substitution. The commit does have fixes we want
but it also breaks valid cases which is not okay. We really need
a far more thorough macro test suite before these kind of changes,
and also need to do the changes in more controlled pieces.
Panu Matilainen