Identify the correct binary for DNF4 #2936
Conversation
Conan-Kudo
force-pushed
the
dnf4-binary-name
branch
from
def9d8f to
4b38720
Compare
Conan-Kudo
force-pushed
the
dnf4-binary-name
branch
from
4b38720 to
1a8bf27
Compare
schaefi
left a comment
schaefi
left a comment
There was a problem hiding this comment.
Thanks for looking into this 👍 I think one code part could be done a bit simpler and I have a few questions regarding the change
| dnf4_binary = 'dnf-3' | ||
| dnf4_search_env = { | ||
| 'PATH': os.sep.join([root, 'usr', 'bin']) | ||
| } if root else None | ||
|
|
||
| # Python interpreter specific path | ||
| if Path.which( | ||
| filename='dnf4', | ||
| custom_env=dnf4_search_env, | ||
| access_mode=os.X_OK | ||
| ): | ||
| dnf4_binary = 'dnf4' |
There was a problem hiding this comment.
I think you can change this to
dnf_binary = 'dnf-4' if Path.which(
filename='dnf4', access_mode=os.X_OK, root_dir=root
) else 'dnf-3'
return dnf_binaryThere was a problem hiding this comment.
It actually means that we are using dnf-4 if that exists in the root tree of the image and else we are using dnf-3 unconditionally. Doesn't this open some room for issues ? What if dnf-3 doesn't exist ? Also when checking inside the root tree then checking outside of it would also be needed as the bootstrap phase will use the package manager from the host. The decision logic here is not quite clear to me
There was a problem hiding this comment.
If the idea was to lookup PATH (host) and in $root then the code needs to run two lookup calls, because the first call would be happy if there is just one hit in the given PATH
There was a problem hiding this comment.
This is the exact implementation we used in the old yum one: https://github.com/OSInside/kiwi/blob/v9.16.36/kiwi/package_manager/yum.py#L89-L110
There was a problem hiding this comment.
At least with all the current supported distributions, the dnf-3 binary exists unless the per-Python version suffixed binaries don't exist, and those have dnf4 instead.
There was a problem hiding this comment.
We just can't check for dnf because it's an ambiguous binary now.
This is pretty much the same solution we used back when we supported YUM and had to deal with yum vs yum-deprecated.