Add new rpmExpandThisMacro() public method #1414
Conversation
|
This needs to be connected to python/lua as well. How should the interface look like? |
|
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. |
|
...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 different here, I'll rebase the Lua work on top of this and see how it looks like now. |
|
Ah, this takes 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. |
|
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. |
|
@pmatilai |
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 %_sourcedir. |
|
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. |
|
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 actual plan as that what exactly). Just had a closer look at the actual patches, my only real complaint is the amount duplication of code between expandMacro() and expandThisMacro(). That's the main difference between your version and mine, and avoids the recursion issues I ran into (so difference between working and non-working 😁 ), but... maybe some of the entry/exit mb checks could be further refactored to use common code? |
|
Okay, just for the reference, I suggested considering using an error code instead of NULL: #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 like the inner calls do (so you could just pass the inner rc on), I certainly would not mind... |
|
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 build the argv anyway... |
|
So you're suggesting that we remove the argvFree() call in the freeArgs() function? |
|
I'll update the patch set after lunch. |
That's true, however, it is reasonable to assume that other programs written in Python would need to do this, especially as parametric macros become more common. RPMLint does this in some places too. I've written code to use |
|
@Conan-Kudo , the point is that Python and macros reside on different planes of existence entirely. In the embedded Lua realm, you are a macro most of the time, whereas Python only sometimes casually observes the value of one from the outside. Nobody is forbidding folks from adding more sophisticated macro bindings for Python but in reality the use-cases don't seem to be there. |
|
Ok, I factored out the common code. I'm not super happy about the naming, though. Comment welcome ;) |
|
Names are annoying... for a theme that's used elsewhere in rpm too, mbInit() and mbFini() maybe? |
No functional changes.
This gets rid of the weird and unneeded ownership transfer and makes the args seetup more symmetric.
This eleminates duplicate code.
This expands the maco with the specified name. Argument expandsion for parametric macros can be turned on with the RPMEXPAND_EXPAND_ARGS flag.
|
Oh, sorry I missed that last push from yesterday. Looks fine now, except that the last commit message still talks about mbStartExpansion() and mbFinishExpansion(). |
Add a mbInit() function that allocates the buffer, increases the expansion depth and stashes some values into a MacroExpansionData area. Add a mbFini() function that decreases the depth again, restores the values and optionally prints the expansion result.
|
Thanks! |
No description provided.