-
Notifications
You must be signed in to change notification settings - Fork 367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
posix.redirect2null deprecated without replacement #3192
Comments
It's unfortunate that nobody saw this coming wrt rpm.execute(). I guess it's a matter of a few posix_spawn_file_actions_*() calls but the big question is what (and how) to expose on rpm.execute() level. If we previously had a special function for redirecting to null then maybe it could be something like two optional extra args to silence stdout and/or stderr, even if it seems a bit limiting. |
You could add a Lua table argument with some sort of encoding of the |
Possible, sure. To me it's more of a question how to do it in a nice way. First of all it basically requires adding an alternative syntax of rpm.execute() that accepts the command and its arguments as a table, the current syntax is not enhanceable because it's defined as all arguments to be passed to the command itself. Adding this table form isn't hard or a bad thing at all, but it's a pre-requisite nevertheless. Then we could add eg file actions as an optional table of tables argument. One problem with extending rpm.execute() is that it'd be hard/impossible to detect whether the current rpm version supports this new syntax, so maybe this should be a new function instead 🤔 OTOH, I spot exactly one user of rpm.execute() in Fedora package set, with glibc being a potential second, and two users of redirect2null(). So it seems the clientele here is a highly specialized group of packages in the early bootstrap set, and others are just using os.execute(). Another issue here is that exposing the raw posix fileactions seems fiddly and ugly for the alleged user of rpm.execute() who just wants to call a damn helper (but, sooner or later you need output control). And given the way you're supposed to pass exact descriptor number to the low-level actions, wrapping it all in a higher level action (eg redirect fd to /dev/null) seems problematic. |
#3236 adds the alternative table based calling mode as a pre-requisite to this. |
After letting the "downstairs guys" chew on this while I had lunch, I think the kind of interface rpm.execute() wants is simply two optional extra arguments for redirecting stdout and stderr, respectively (for the variant that takes the command and options as a table). If it one day turns out that we need something fancier, we can wrap the full posix_spawn() API and call it thus. |
A simple path-based redirection added in the linked PR now. It's simple for simple things but might be too limiting for something more advanced. Comments + suggestions welcome. |
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
On a related note, I just realized that we can't really remove these even in v6, because as rare as the use of these functions are, those uses are on some of the most central packages of distros and removing them would suddenly make all current Fedora/RHEL versions uninstallable. Which would be the single biggest incompatibility in v6, and that doesn't make any sense. It'd be a whole lot more meaningful if we could issue those deprecation warnings on package build instead of install, but I don't see a way to do that. Another possibility is to only support these for v4 packages, somehow. |
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: #3192
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: rpm-software-management#3192 (cherry picked from commit 6444f71)
Turns out real-world usage needs more control than rpm.execute() delivers. This could be crammed into rpm.execute() but it'd be forward incompatible in a somewhat non-obvious way, we might just as well add a separate function for it. Supports redirecting stdin, stdout and stderr to a given path, support for file descriptors, other actions and spawn attributes can be added later. Fixes: #3192 (cherry picked from commit 6444f71)
On a related note, here's an attempt to make the deprecation warnings less obnoxious, ie avoid them on old packages: #3270 |
Describe the bug
Commit 46bd0ed added a deprecation warning to
posix.redirect2null
and others, pointing torpm.execute
as a replacement, butrpm.execute
does not provide a way to suppress output.Expected behavior
There is a way to run a program with its output suppressed.
We'd use this in the Fedora glibc package because we want to suppress the error message from
systemctl daemon-reexec
if systemd is not running. Bash is not necessarily installed at this point.The text was updated successfully, but these errors were encountered: