Merged #1414 into master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1414#event-3940485078___
Rpm-maint mailing list
Thanks!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1414#issuecomment-719464028___
Rpm-maint mailing list
@pmatilai approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1414#pullrequestreview-520529113___
Oh, sorry I missed that last push from yesterday. Looks fine now, except that
the last commit message still talks about mbStartExpansion() and
mbFinishExpansion().
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Names are annoying... for a theme that's used elsewhere in rpm too, mbInit()
and mbFini() maybe?
Also seems to me in that the mbAllocBuf() calls could be moved to the
start/init-part too. But yeah much nicer this way, regardless of names.
--
You are receiving this because you are subscribed to
Ok, I factored out the common code. I'm not super happy about the naming,
though. Comment welcome ;)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> > spectool was rewritten to use RPM Python bindings, so I imagine at least it
> > would matter for that tool.
>
> Hardly. There's precisely _one_ rpm.expandMacro() in the entire new spectool
> code, and that doesn't involve parametric macros or anything else advanced,
> it's merely to expand
I'll update the patch set after lunch.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1414#issuecomment-717177932___
Rpm-maint
So you're suggesting that we remove the argvFree() call in the freeArgs()
function?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Yet another BTW: I don't think it would be unreasonable to expect
rpmExpandThisMacro() callers to supply the macro name as the first argument of
the argv (similar to the exec() family of functions) so we wouldn't need to
copy that internally just because - the caller knows the name and has to
@mlschroe pushed 1 commit.
6c96d96a091dd97ab52af44ebb09845e94549bf2 Remove common code from expandMacro()
and expandThisMacro()
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@Conan-Kudo , the point is that Python and macros reside on different levels of
existence entirely. In the embedded Lua realm, you *are* a macro most of the
time, whereas Python only sometimes casually observes the value from the
outside. Nobody is forbidding folks from adding more
> spectool was rewritten to use RPM Python bindings, so I imagine at least it
> would matter for that tool.
Hardly. There's precisely *one* rpm.expandMacro() in the entire new spectool
code, and that doesn't involve parametric macros or anything else advanced,
it's merely to expand
@pmatilai commented on this pull request.
> -
- /* Recursively expand body of macro */
- if (me->body && *me->body) {
- if ((me->flags & ME_LITERAL) != 0)
- mbAppendStr(mb, me->body);
- else
- expandMacro(mb, me->body, 0);
- }
-
Okay, just for the reference, I suggested considering using an error code
instead of NULL: https://github.com/rpm-software-management/rpm/pull/34 but as
to how that turned into -1/1 I plead innocent :D
The documented "return negative on error" would allow changing it to return 0
on success
Heh, I almost commented on the return code thing, before noticing that it was
actually to be same as rpmExpandMacro(). There was a kind of a reason for the
return code to be like that, basically to leave room for returning more
information in the future (but I don't recall if there was an
I wonder if we should make the interface more sane. I currently made it similar
to rpmExpandMacro(), but that was probably a bad idea. The interface is
currently return 1 on success and -1 on failure, put string in the obuf pointer.
But the straightforward interface would be return string on
@pmatilai `spectool` was rewritten to use RPM Python bindings, so I imagine at
least it would matter for that tool.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Oh and BTW, there's actually an existing use-case for this in the code-base:
this is exactly what runCall() in build/rpmfc.c wants.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
...except that's exactly where it starts getting trickier: from Lua the argv is
differently allocated and shouldn't be freed in freeArgs() and at least in my
version there was "fun" with me->opts processing retriggering the option
processing when it shouldn't have, etc. But many details are
I can pick up the Lua side once this is merged, no need to redo all that. As
for Python, the bindings wrt macros are really primitive as it is and nobody is
complaining so I dunno if anybody cares. At any rate, I don't consider that a
requirement for merging.
--
You are receiving this because
This needs to be connected to python/lua as well. How should the interface look
like?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Ah, making a copy of the argv so the allocation business is different from what
I had. https://github.com/pmatilai/rpm/tree/luamt-2 is a quick "port" on top of
this and the basic testcases pass as-is, but only very briefly tested.
--
You are receiving this because you are subscribed to this
23 matches
Mail list logo
