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

[Neal Gompa]

On Fri, Feb 3, 2017 at 10:02 AM, Panu Matilainen
<pmatilai at laiskiainen.org> wrote:
> On 02/03/2017 04:54 PM, Panu Matilainen wrote:
>>
>> On 02/03/2017 03:26 PM, Pascal Terjan wrote:
>>>
>>> On 3 February 2017 at 12:14, Panu Matilainen
>>> <pmatilai at laiskiainen.org> wrote:
>>>>
>>>>
>>>> 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
>>>>
>>>> Another option is slightly changing the whole scoping notion: parametric
>>>> macros require locally scoped macros for the automatic argument
>>>> macros like
>>>> %#, %* and %1 anyway (it's flawed too currently, see below). So
>>>> perhaps the
>>>> macro scoping should follow the current "call level", ie a macro defined
>>>> inside a parametric macro body is local to that macro, and everything
>>>> else
>>>> is global. In this case the reproducer would emit
>>>> bar
>>>> true
>>>> bar
>>>
>>>
>>> My first reaction was to prefer this one because '%{!?foo: %define foo
>>> bar} has been used in tens of thousands of packages and sort of worked
>>> "forever"
>>> But replacing define with global is not very hard so no strong opinion...
>>
>>
>> Me too, but no strong opinion either. Also I'm biased, because
>
>
> Oops. What that was supposed to say is that I'm biased because for years
> I've been explaining this "implemented before my time" block scoping thing
> to baffled packagers, so changing it now would feel at least somewhat
> awkward or something. But ultimately I just want a sane, clearly defined
> behavior, never mind my feelings :)
>

The thing is, I think you're absolutely right. I thought it was weird
too, but I've adapted to it. If we want to change it to a sane
behavior, it just needs to be clearly described so that that rely on
broken behavior are able to be fixed.


-- 
真実はいつも一つ！/ Always, there's only one truth!