Use libdnf.utils.checksum_{check,value} #1743

malmond77 · 2021-03-11T23:12:53Z

libdnf has the canonical implementation of checksum handling. We aim to
replace all use of dnf.yum.misc.checksum() with this. In doing so, this
fixes installing previously downloaded and transcoded rpms to support

rpm-software-management/librepo#222

This also has some minor performance benefits: librepo's checksum
handling employs caching of previously downloaded files via extended
attributes. This works for ordinary rpms in the dnf cache, but does not
work (yet) for rpm paths specified on the command line due to
rpm-software-management/librepo#233. That
issue is pretty minor, and the fix ends up in libdnf later.

The previous implementation maps all runtime errors to MiscError. We do
this still by taking the libdnf.error.Error class (defined in SWIG) and
map it directly back to the Python exception as before.

malmond77 · 2021-03-11T23:19:50Z

More context: https://bugzilla.redhat.com/show_bug.cgi?id=1935465

lgtm-com · 2021-03-11T23:22:14Z

This pull request introduces 1 alert when merging 9c29239 into 5b2e24d - view on LGTM.com

new alerts:

1 for Unused import

libdnf has the canonical implementation of checksum handling. We aim to replace all use of dnf.yum.misc.checksum() with this. In doing so, this fixes installing previously downloaded and transcoded rpms to support rpm-software-management/librepo#222 This also has some minor performance benefits: librepo's checksum handling employs caching of previously downloaded files via extended attributes. This works for ordinary rpms in the dnf cache, but does not work (yet) for rpm paths specified on the command line due to rpm-software-management/librepo#233. That issue is pretty minor, and the fix ends up in libdnf later. The previous implementation maps all runtime errors to MiscError. We do this still by taking the libdnf.error.Error class (defined in SWIG) and map it directly back to the Python exception as before.

lgtm-com · 2021-03-12T19:43:56Z

This pull request fixes 1 alert when merging 054cd9d into 5b2e24d - view on LGTM.com

fixed alerts:

1 for Module is imported with 'import' and 'import from'

DNF has been carrying around yum's old checksum function. These functions duplicate code in librepo. They are slower because librepo can employ caching of digests. Lastly, these functions in Python do not know about changes in checksum logic like rpm-software-management/librepo#222 The choices here are: 1. Replicate `lr_checksum_cow_fd()` and caching logic in Python 2. Just use librepo from dnf. This is 2. Note there was bug in librepo that forces no caching for `checksum_value()` (rpm-software-management/librepo#233). This is now fixed, so we depend on librepo-1.13.1 to ensure this fix is present. This change goes hand in hand with a change to `dnf` itself to make use of the new functions and eliminate the old ones. This is rpm-software-management/dnf#1743. We bump the version of this library to ensure this dependency is expressed properly. On errors, these functions raise libdnf.error.Error which can be easily mapped into MiscError in dnf

DNF has been carrying around yum's old checksum function. These functions duplicate code in librepo. They are slower because librepo can employ caching of digests. Lastly, these functions in Python do not know about changes in checksum logic like rpm-software-management/librepo#222 The choices here are: 1. Replicate `lr_checksum_cow_fd()` and caching logic in Python 2. Just use librepo from dnf. This is 2. Note there was bug in librepo that forces no caching for `checksum_value()` (rpm-software-management/librepo#233). This is now fixed, so we depend on librepo-1.13.1 to ensure this fix is present. This change goes hand in hand with a change to `dnf` itself to make use of the new functions and eliminate the old ones. This is rpm-software-management/dnf#1743. We bump the version of this library in the next commit to ensure this dependency is expressed properly. On errors, these functions raise libdnf.error.Error which can be easily mapped into MiscError in dnf

lgtm-com · 2021-03-17T22:38:42Z

This pull request fixes 1 alert when merging 17a6621 into 4763920 - view on LGTM.com

fixed alerts:

1 for Module is imported with 'import' and 'import from'

DNF has been carrying around yum's old checksum function. These functions duplicate code in librepo. They are slower because librepo can employ caching of digests. Lastly, these functions in Python do not know about changes in checksum logic like rpm-software-management/librepo#222 The choices here are: 1. Replicate `lr_checksum_cow_fd()` and caching logic in Python 2. Just use librepo from dnf. This is 2. Note there was bug in librepo that forces no caching for `checksum_value()` (rpm-software-management/librepo#233). This is now fixed, so we depend on librepo-1.13.1 to ensure this fix is present. This change goes hand in hand with a change to `dnf` itself to make use of the new functions and eliminate the old ones. This is rpm-software-management/dnf#1743. We bump the version of this library in the next commit to ensure this dependency is expressed properly. On errors, these functions raise libdnf.error.Error which can be easily mapped into MiscError in dnf

m-blaha

One small problem remains - config-manager plugin uses yum.misc.Checksums class (see https://github.com/rpm-software-management/dnf-plugins-core/blob/93c76f546ae9b0a766b15b3c0c302da06e206ee9/plugins/config_manager.py#L256). But its use case is simply shortening a long string so it could be easily replaced by direct usage of hashlib standard library.
So dnf-plugins-core must be first changed and conflicts_dnf_plugins_core_version in dnf.spec set accordingly not to allow installation of older dnf-plugins-core with this newer dnf.

The change itself looks good to me and ready to be merged.

This is a pre-requisite for rpm-software-management/dnf#1743 That PR removes a class that is used here. The actual benefit of the class is marginal. The only part that needs preserved is the automatic coercion of str/unicode into bytes.