@keszybz commented on this pull request.
> @@ -0,0 +1,15 @@
+#!/bin/sh
+
+version_deps () {
+ while read dep
+ do
+ if [[ "${dep}" =~ ^[^\(]*\(\) ]] && rpm -q --whatprovides
"${dep}" &> /dev/null
+ then
+ printf "($dep with %s)\n" "$(rpm -q --whatprovides
"${dep}" --qf '%{NAME} >= %{VERSION}')"
+ else
+ printf "%s\n" "${dep}"
+ fi
+ done
+}
+
+/usr/lib/rpm/find-requires | version_deps
Shouldn't this be %{_rpmconfigdir}/find-requires, i.e. without a hardcoded path?
Without `set -o pipefail`, this will ignore the return value from
`find-requires`, which seems iffy.
> @@ -0,0 +1,15 @@
+#!/bin/sh
+
+version_deps () {
+ while read dep
+ do
+ if [[ "${dep}" =~ ^[^\(]*\(\) ]] && rpm -q --whatprovides
"${dep}" &> /dev/null
`[[ =~ ]]` is a bash syntax, but the file is declared as pure-sh.
I think it's it's nicer to avoid a space after the redirection operator.
The conditional is not robust wrt. to failure. If `rpm -q` fails, we just take
the other branch. If that is intentional, it'd be nice to add a comment
explaining that.
> @@ -0,0 +1,15 @@
+#!/bin/sh
+
+version_deps () {
+ while read dep
+ do
+ if [[ "${dep}" =~ ^[^\(]*\(\) ]] && rpm -q --whatprovides
"${dep}" &> /dev/null
+ then
+ printf "($dep with %s)\n" "$(rpm -q --whatprovides
"${dep}" --qf '%{NAME} >= %{VERSION}')"
Why not make this a separate dependency, i.e. keep the original dep as it was,
and just add a separate version dep?
I think `%{?_isa}` should be used, i.e. the dependency should specify the arch,
so that a package for another arch is not used to satisfy the dep
inadvertently.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2372#pullrequestreview-1274223226
You are receiving this because you are subscribed to this thread.
Message ID: <rpm-software-management/rpm/pull/2372/review/1274223...@github.com>
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint