[Rpm-ecosystem] Fwd: [Rpm-maint] Fixing macro scoping

Panu Matilainen pmatilai at laiskiainen.org
Mon Feb 6 15:15:12 UTC 2017


On 02/06/2017 05:00 PM, Panu Matilainen wrote:
> On 02/06/2017 04:12 PM, Neal Gompa wrote:
>> On Mon, Feb 6, 2017 at 4:50 AM, Panu Matilainen
>> <pmatilai at laiskiainen.org> wrote:
>>> On 02/06/2017 10:56 AM, Vít Ondruch wrote:
>>>>
>>>>
>>>>
>>>> Dne 3.2.2017 v 13:14 Panu Matilainen napsal(a):
>>>>>
>>>>>
>>>>> Stunned silence on rpm-maint, forwarding to rpm-ecosystem in hopes of
>>>>> a larger and livelier audience...
>>>>>
>>>>>     - Panu -
>>>>>
>>>>> -------- Forwarded Message --------
>>>>> Subject: [Rpm-maint] Fixing macro scoping
>>>>> Date: Mon, 23 Jan 2017 12:30:21 +0200
>>>>> From: Panu Matilainen <pmatilai at laiskiainen.org>
>>>>> To: rpm-maint at lists.rpm.org <rpm-maint at lists.rpm.org>
>>>>>
>>>>>
>>>>> Consider the following snippet, originating from
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=552944:
>>>>>
>>>>> %{!?foo: %define foo bar}
>>>>> %define dofoo() true
>>>>> echo 1: %{foo}
>>>>> %dofoo
>>>>> echo 2: %{foo}
>>>>>
>>>>> I'd assume everybody agrees both %{foo}'s should evaluate to the same
>>>>> value, but that is not the case currently. Using a cli-variant of the
>>>>> above:
>>>>>
>>>>> [pmatilai at sopuli rpm]$ rpm --define 'dofoo() true' --eval '%{!?foo:
>>>>> %define foo bar}' --eval '%foo' --eval '%dofoo' --eval '%foo'
>>>>> warning: Macro %foo defined but not used within scope
>>>>>
>>>>> bar
>>>>> true
>>>>> %foo
>>>>>
>>>>> The flaw here is that rpm supposedly implements block level scoping
>>>>> for macros (so in the above example, "foo" would only exist in the
>>>>> {!?foo:...} block), but doesn't actually enforce that, unless a
>>>>> parametric macro is "called". Current rpm warns about it, but warnings
>>>>> or not this behavior doesn't make the slightest sense.
>>>>>
>>>>> The question is, what do you think %{foo} should evaluate to in this
>>>>> case?
>>>>>
>>>>> Fixing it to honor the strict "block scoping" concept is not hard, now
>>>>> that the scoping level is honored from Lua too (see
>>>>>
>>>>> https://github.com/rpm-software-management/rpm/commit/1767bc4fd82bfacee622e698f9f0ae42c02126fa).
>>>>>
>>>>> In this case the above reproducer would emit
>>>>>
>>>>> %foo
>>>>> true
>>>>> %foo
>>>>
>>>>
>>>> As far as I can tell, usage of %define is discouraged and I never
>>>> really
>>>> understood why. But this example explains it and this should be the
>>>> right behavior IMO.
>>>
>>>
>>> Yup, people are told to avoid %define like it was the plague, which
>>> is just
>>> plain bullsh**. You just need to understand the scoping of macros. Which
>>> almost nobody does, no matter how many times it gets repeated here or
>>> there.
>>> That the rpm implementation of the scoping is very buggy doesn't exactly
>>> help of course.
>>>
>>> So I've really started thinking it's better to make rpm behave the way
>>> people expect it to instead of banging my head against this for
>>> another 10+
>>> years.
>>>
>>
>> For what it's worth, Andreas Scherer's debbuild[1] treats %define just
>> like %global in terms of scope, which I guess is the expectation of
>> most people (though that might be because it's not exactly well
>> documented that there *are* rules for %define scoping). That said, if
>> the scoping rules are clear (and work as they're supposed to!), I
>> don't see a reason why not to actually have scoping for %define.
>>
>> [1]: https://github.com/ascherer/debbuild
>>
>
> %define inside a parametric macro is scoped and almost correctly
> enforced too in all existing rpm versions. Ditto for the automatic
> macros (%*, %1, %2 etc) inside parametric macros.
>
> So what I'm leaning towards to is a simple two-level scoping:
> 1) macros %define'd inside parametric macro are local to that macro,
> including the automatic argument macros (%global is obviously %global
> there as well)
> 2) everything else is globally scoped
>
> This would mean that the common and troublesome idiom of:
>
>     "%{!?foo: %define foo 1}"
>
> ...is no more equals this useless C-construct:
>     if (foo) {
>         int foo = 1;
>     }
>
> ...but instead places the foo in the nearest meaningful scope: either
> the global scope or the local "function" scope, in case it occurred
> inside a parametric macro.
>
> Rpm doesn't currently limit macro visibility from other scopes at all,
> I'd like to change that too so that you only ever see the global scope,
> and additionally within parametric functions, the macros defined in
> *that* parametric scope. And nothing else, no leftovers from other
> "calls" etc.

I knew I was forgetting something:

Enforcing macro visibility also absolutely requires fixing the expansion 
of parametric macro arguments (GH issue #127). But that's also one of 
the long-standing warts in the macro engine, and I'd expect it to be 
well more than worth any breakage that it may cause.

	- Panu -



More information about the Rpm-ecosystem mailing list