#525 Don't report deprecated() dependencies if other packages satisfy them
Merged 15 days ago by ngompa. Opened 2 months ago by churchyard.
churchyard/FedoraReview deprecated_no_multiple  into  master

file modified
+11 -18
@@ -2145,11 +2145,10 @@ 

      def run_on_applicable(self):

  

          # pylint: disable=R0912

-         def resolve(requires_arg):

+         def resolve_as_groups(requires_arg):

              """

              Resolve list of symbols to packages in srpm or by repoquery

              """

-             pkgs = []

              requires_to_process = list(requires_arg)

              for r in requires_arg:

                  if r.startswith("rpmlib"):
@@ -2162,27 +2161,21 @@ 

                      if r in self.rpms.get(pkg).provides:

                          requires_to_process.remove(r)

                          break

-             if requires_to_process:

-                 pkgs.extend(deps.resolve(requires_to_process))

-             pkgs = set(pkgs)

-             # Dirty workaround for pytest https://pagure.io/FedoraReview/issue/392

-             if "python3-pytest" in pkgs and "python3-pytest4" in pkgs:

-                 pkgs.remove("python3-pytest4")

-             return pkgs

+             return deps.resolve_as_groups(requires_to_process)

  

-         pkg_deps = set()

          pkg_list = set()

          for pkg in self.spec.packages:

-             pkg_list |= set(self.rpms.get(pkg).requires)

-         pkg_list |= set(self.spec.build_requires)

+             pkg_list |= set(self.rpms.get(pkg).requires_nevrs)

+         pkg_list |= set(self.spec.build_requires_nevrs)

  

-         pkg_deps |= resolve(pkg_list)

- 

-         for pkg in pkg_deps:

-             provides = deps.list_provides(pkg)

-             if any("deprecated()" in p for p in provides):

+         for dependency, providers in resolve_as_groups(pkg_list).items():

+             for pkg in providers:

+                 provides = deps.list_provides(pkg)

+                 if all("deprecated()" not in p for p in provides):

+                     break

+             else:

                  self.set_passed(

-                     self.FAIL, pkg + " is deprecated, you must not depend on it."

+                     self.FAIL, dependency + " is deprecated, you must not depend on it."

                  )

                  return

  

file modified
+7 -1
@@ -162,7 +162,13 @@ 

      # to specify multiple packages at once, but dnf repoquery only allows one

      # at a time).  Then concatenate all those lists of providers together

      # and return.

-     return [elem for list_ in map(resolve_one, reqs) for elem in list_]

+     return [elem for list_ in resolve_as_groups(reqs).values() for elem in list_]

+ 

+ 

+ def resolve_as_groups(reqs):

+     """Return the packages providing the reqs symbols, grouped by each symbol"""

+ 

+     return {req: resolve_one(req) for req in reqs}

  

  

  def resolve_one(req):

@@ -149,6 +149,12 @@ 

          return self.header[rpm.RPMTAG_REQUIRES]

  

      @property

+     def requires_nevrs(self):

+         """List of requires with versions, also auto-generated for rpm."""

+         self.init()

+         return self.header[rpm.RPMTAG_REQUIRENEVRS]

+ 

+     @property

      def provides(self):

          """List of provides, also auto-generated for rpm."""

          self.init()

@@ -278,6 +278,11 @@ 

          """Return the list of build requirements."""

          return self.spec.sourceHeader[rpm.RPMTAG_REQUIRES]

  

+     @property

+     def build_requires_nevrs(self):

+         """Return the list of build requirements, but with versions."""

+         return self.spec.sourceHeader[rpm.RPMTAG_REQUIRENEVRS]

+ 

      def get_requires(self, pkg_name=None):

          """Return list of requirements i. e., Requires:"""

          package = self._get_pkg_by_name(pkg_name)

Instead of putting all packages that satisfy all dependencies to one pile,
we group them by the dependency itself.
And we only report if all packages in one group are deprecated().

As a specific example consider python3-pytest4 is deprecated(),
and a package BuildRequires:

  • python3-devel
  • python3dist(pytest)

Previously, we would get:

{python3-devel, python3-pytest, python3-pytest4}

And one of the is deprecated() => false report

Now we get:

{python3-devel: [python3-devel], python3dist(pytest): [python3-pytest, python3-pytest4]}

And none of the groups is all-deprecated => no false report

Fixes https://pagure.io/FedoraReview/issue/392

rebased onto d158c7b

2 months ago

It seems to work now.

I still think this disregards generated BuildRequires (as it reads them directly from the specfile), which is a problem, but a different one.

Anybody to review, merge and release or backport this?

Pull-Request has been merged by ngompa

15 days ago