On Tue, Jan 22, 2013 at 7:16 PM, Panu Matilainen
<pmatilai@xxxxxxxxxxxxxxx> wrote:
> On 01/21/2013 10:14 PM, Alexey Tourbin wrote:
>>
>> This change introduces a separate routine to parse for valid macro
>> names. Valid macro names are either regular 3+ character identifiers,
>> or special names: "S", "P", "0", "#", "*", "**", macro options such as
>> "-o" and "-o*", and macro arguments such as "1". Other names are not
>> valid. This fixes a number of bugs seen earlier due to sloppy name
>> parsing: "%_libdir*" and "%01" were not expanded (these are now expanded
>> to e.g. "/usr/lib64*" and "<name>1", as expected). This also fixes
>> bugs in as-is substitution: "%!foo" was expanded to "%foo", and likewise
>> "%!!!" was expanded to "%" (and to "%<garbage>" at EOL).
>>
>> Also, bad names in %name and %{name...} substitutions are now handled
>> differently. In %name form, the name is parsed tentatively; a silent
>> fall-back to as-is substitution is provisioned when no valid name can
>> be obtain. In %{name...} form, a failure to obtain a valid name is now
>> a syntax error. Furthermore, only 3 variants are syntactically valid:
>> %{name} proper, %{name:...}, and %{name ...}. This renders invalid
>> ambiguous macro substitutions such as the one found in FC18 lvm2.spec:
>>
>> Requires: util-linux >= %{util-linux_version}
>> error: Invalid macro syntax: %{util-linux_version}
>
> Nice. The garbage spotted by this in Fedora specs is ... scary.
>
> OTOH changes of this scale in the macro parser are a bit scary in themselves
> as its sooooo easy to break some obscure corner-case :) I think I'll try to
> give this a spin with some of the more macro-intensive packages like
> Fedora's kernel and font-packages first, but unless I find something
> unexpected breakage, consider it applied. On a related note, the test-suite
> could really use more thorough and more advanced test-cases to make these
> kind of changes less scary.
Right, such changes must be extensively tested, and their practical
consequences must be well (and early) understood. For this, I employ
the "corpus" approach - that is, preprocess all specfiles from FC18
and examine the differences.
Here is roughly how the process goes.
0) Extract all specfiles from FC18:
$ mkdir /fc18-specs
$ for f in /fc18-src/?/*.src.rpm; do
rpm2cpio $f |(cd /fc18-specs; cpio -idmu --quiet '*.spec'); done
1) Preprocess:
$ mkdir /fc18-cpp1
$ for f in /fc18-specs/*.spec; do
rpmspec -P $f >/fc18-cpp1/${f##*/} 2>&1 ; done
2) Hack on rpmio/macro.c, then make(1). New code can be injected with e.g.
$ export LD_LIBRARY_PATH=~/rpm/rpmio/.libs
3) Preprocess again:
$ mkdir /fc18-cpp2
$ for f in /fc18-specs/*.spec; do
rpmspec -P $f >/fc18-cpp2/${f##*/} 2>&1 ; done
4) Compare:
$ duff -ur --ignore-m='^Build[Rr]oot:' /fc18-cpp1 /fc18-cpp2
This way, _every_ change in _every_ specfile will be noticed.
> BTW, while the rules are obviously different between define vs expansion due
> to the built-in special macros, perhaps parseMacroName() function here could
> be extended to cover the %define/%global/%undefine macro name sanity checks
> as well (see my reply wrt the missing-whitespace patch).
Macros defined with %define always have sane names; this might betray
user intentions, though; specifically, when a sane name is not
followed by a whitespace.
_______________________________________________
Rpm-list mailing list
Rpm-list@xxxxxxxxxxxxx
http://lists.rpm.org/mailman/listinfo/rpm-list
