Re: [gentoo-portage-dev] [PATCH] emerge: add --changed-deps-report option (bug 645780)

[Alec Warner]

On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <zmedico@g.o> wrote:

> The --dynamic-deps=n default causes confusion for users that are
> accustomed to dynamic deps, therefore add a --changed-deps-report
> option that is enabled by default (if --usepkgonly is not enabled).
>
> The --quiet option will suppress the report if none of the packages
> having changed dependencies are in the dependency graph, since they
> are harmless in that case. If any of these packages *are* in the
> dependency graph, then --quiet suppresses the big NOTE section of
> the report, but the HINT section is still displayed since it might
> help users resolve problems that are solved by --changed-deps.
>
> Example output is as follows:
>
> !!! Detected ebuild dependency change(s) without revision bump:
>
>     net-misc/openssh-7.5_p1-r3::gentoo
>     sys-fs/udisks-2.7.5::gentoo
>
> NOTE: For the ebuild(s) listed above, a new revision may be warranted if
> there
>       has been a dependency change with significant consequences. Refer to
> the
>       following page of the Gentoo Development Guide for examples of
>       circumstances that may qualify:
>
>           https://devmanual.gentoo.org/general-concepts/ebuild-revisions/
>
>       If circumstances qualify, please report a bug which specifies the
> current
>       version of the ebuild listed above. Changes to ebuilds from the
> 'gentoo'
>       repository (ending with '::gentoo') can be browsed in GitWeb:
>
>           https://gitweb.gentoo.org/repo/gentoo.git/
>
>       Use Gentoo's Bugzilla to report bugs only for the 'gentoo'
> repository:
>
>           https://bugs.gentoo.org/
>
>       In order to suppress reports about dependency changes, add
>       --changed-deps-report=n to the EMERGE_DEFAULT_OPTS variable in
>       '/etc/portage/make.conf'.
>
> HINT: In order to avoid problems involving changed dependencies, use the
>       --changed-deps option to automatically trigger rebuilds when changed
>       dependencies are detected. Refer to the emerge man page for more
>       information about this option.
>
> Bug: https://bugs.gentoo.org/645780


I can't really support sending this large report to users.

1) Its fairly common practice today.
2) All users will get the report.
3) A subset of them will file bugs about the report.
4) Devs make a decision about revbumping vs not; there doesn't seem to be a
way for devs to say "no this change is intentional, stop nagging users."

Ultimately I'm unsure what we are trying to accomplish here. Do we think
Devs are doing 4 on accident?
If so, why can't we give them tools to find it ahead of time (repoman et.
al.) or tools to find it post-commit (tinderbox or similar; but these are
single-sourced reports.)

I also feel like we are pushing action onto users here. If we agree with
the premise that this is a bug (and devs should always bump) then we don't
need users to take any action;
we could build automation that sends these 'reports'. Its not like the user
is going to add anything interesting to the report. Making them do it by
hand just introduces transcription errors in filing. We just need to get
their permission to file the reports automatically when portage finds them.

-A


>
> ---
>  man/emerge.1                          |  10 ++++
>  pym/_emerge/create_depgraph_params.py |   5 ++
>  pym/_emerge/depgraph.py               | 103 ++++++++++++++++++++++++++++++
> +++-
>  pym/_emerge/main.py                   |  13 +++++
>  4 files changed, 128 insertions(+), 3 deletions(-)
>
> diff --git a/man/emerge.1 b/man/emerge.1
> index 3c81b9c9f..9de1b7f74 100644
> --- a/man/emerge.1
> +++ b/man/emerge.1
> @@ -465,6 +465,16 @@ option also implies the \fB\-\-selective\fR option.
> Behavior with
>  respect to changed build\-time dependencies is controlled by the
>  \fB\-\-with\-bdeps\fR option.
>  .TP
> +.BR "\-\-changed\-deps\-report [ y | n ]"
> +Tells emerge to report ebuilds for which the ebuild dependencies have
> +changed since the installed instance was built. Behavior with respect to
> +changed build\-time dependencies is controlled by the
> +\fB\-\-with\-bdeps\fR option. This option is enabled automatically for
> +a dependency calculation if the cost of report generation is relatively
> +insignificant (any calculation exclusively involving binary packages is
> +exempt). The \fIEMERGE_DEFAULT_OPTS\fR variable may be used to disable
> +this option by default.
> +.TP
>  .BR \-\-changed\-use ", " \-U
>  Tells emerge to include installed packages where USE flags have
>  changed since installation. This option also implies the
> diff --git a/pym/_emerge/create_depgraph_params.py
> b/pym/_emerge/create_depgraph_params.py
> index cdea029ba..7d01610bb 100644
> --- a/pym/_emerge/create_depgraph_params.py
> +++ b/pym/_emerge/create_depgraph_params.py
> @@ -126,6 +126,11 @@ def create_depgraph_params(myopts, myaction):
>         if changed_deps is not None:
>                 myparams['changed_deps'] = changed_deps
>
> +       changed_deps_report = myopts.get('--changed-deps-report')
> +       if (changed_deps_report != 'n' and not
> +               (myaction == 'remove' or '--usepkgonly' in myopts)):
> +               myparams['changed_deps_report'] = True
> +
>         if myopts.get("--selective") == "n":
>                 # --selective=n can be used to remove selective
>                 # behavior that may have been implied by some
> diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
> index 27bec3b32..8fd45f3a1 100644
> --- a/pym/_emerge/depgraph.py
> +++ b/pym/_emerge/depgraph.py
> @@ -462,6 +462,7 @@ class _dynamic_depgraph_config(object):
>                 self._highest_pkg_cache = {}
>                 self._highest_pkg_cache_cp_map = {}
>                 self._flatten_atoms_cache = {}
> +               self._changed_deps_pkgs = {}
>
>                 # Binary packages that have been rejected because their USE
>                 # didn't match the user's config. It maps packages to a set
> @@ -974,6 +975,90 @@ class depgraph(object):
>
>                 return False
>
> +       def _changed_deps_report(self):
> +               """
> +               Report ebuilds for which the ebuild dependencies have
> +               changed since the installed instance was built.
> +               """
> +               quiet = '--quiet' in self._frozen_config.myopts
> +
> +               report_pkgs = []
> +               for pkg, ebuild in self._dynamic_config._changed_
> deps_pkgs.items():
> +                       if pkg.repo != ebuild.repo:
> +                               continue
> +                       report_pkgs.append((pkg, ebuild))
> +
> +               if not report_pkgs:
> +                       return
> +
> +               # TODO: Detect and report various issues:
> +               # - packages with unsatisfiable dependencies
> +               # - packages involved directly in slot or blocker conflicts
> +               # - direct parents of conflict packages
> +               # - packages that prevent upgrade of dependencies to
> latest versions
> +               graph = self._dynamic_config.digraph
> +               in_graph = False
> +               for pkg, ebuild in report_pkgs:
> +                       if pkg in graph:
> +                               in_graph = True
> +
> +               # Packages with changed deps are harmless if they're not
> in the
> +               # graph, so it's safe to silently ignore them for --quiet
> mode.
> +               if quiet and not in_graph:
> +                       return
> +
> +               writemsg("\n%s\n\n" % colorize("WARN",
> +                       "!!! Detected ebuild dependency change(s) without
> revision bump:"),
> +                       noiselevel=-1)
> +
> +               for pkg, ebuild in report_pkgs:
> +                       writemsg("    %s::%s" % (pkg.cpv, pkg.repo),
> noiselevel=-1)
> +                       if pkg.root_config.settings["ROOT"] != "/":
> +                               writemsg(" for %s" % (pkg.root,),
> noiselevel=-1)
> +                       writemsg("\n", noiselevel=-1)
> +
> +               msg = []
> +               if not quiet:
> +                       msg.extend([
> +                               "",
> +                               "NOTE: For the ebuild(s) listed above, a
> new revision may be warranted if there",
> +                               "      has been a dependency change with
> significant consequences. Refer to the",
> +                               "      following page of the Gentoo
> Development Guide for examples of",
> +                               "      circumstances that may qualify:",
> +                               "",
> +                               "          https://devmanual.gentoo.org/
> general-concepts/ebuild-revisions/",
> +                               "",
> +                               "      If circumstances qualify, please
> report a bug which specifies the current",
> +                               "      version of the ebuild listed above.
> Changes to ebuilds from the 'gentoo'",
> +                               "      repository (ending with '::gentoo')
> can be browsed in GitWeb:",
> +                               "",
> +                               "          https://gitweb.gentoo.org/
> repo/gentoo.git/",
> +                               "",
> +                               "      Use Gentoo's Bugzilla to report
> bugs only for the 'gentoo' repository:",
> +                               "",
> +                               "          https://bugs.gentoo.org/",
> +                               "",
> +                               "      In order to suppress reports about
> dependency changes, add",
> +                               "      --changed-deps-report=n to the
> EMERGE_DEFAULT_OPTS variable in",
> +                               "      '/etc/portage/make.conf'.",
> +                       ])
> +
> +               # Include this message for --quiet mode, since the user
> may be experiencing
> +               # subtle problems that are solvable by using
> --changed-deps.
> +               if self._dynamic_config.myparams.get("changed_deps", "n")
> == "n":
> +                       msg.extend([
> +                               "",
> +                               "HINT: In order to avoid problems
> involving changed dependencies, use the",
> +                               "      --changed-deps option to
> automatically trigger rebuilds when changed",
> +                               "      dependencies are detected. Refer to
> the emerge man page for more",
> +                               "      information about this option.",
> +                       ])
> +
> +               for line in msg:
> +                       if line:
> +                               line = colorize("INFORM", line)
> +                       writemsg(line + "\n", noiselevel=-1)
> +
>         def _show_ignored_binaries(self):
>                 """
>                 Show binaries that have been ignored because their USE
> didn't
> @@ -2569,6 +2654,10 @@ class depgraph(object):
>
>                                 changed = built_deps != unbuilt_deps
>
> +                               if (changed and pkg.installed and
> +                                       self._dynamic_config.myparams.
> get("changed_deps_report")):
> +                                       self._dynamic_config._changed_deps_pkgs[pkg]
> = ebuild
> +
>                 return changed
>
>         def _create_graph(self, allow_unsatisfied=False):
> @@ -6354,6 +6443,8 @@ class depgraph(object):
>                                         changed_deps = (
>
> self._dynamic_config.myparams.get(
>                                                 "changed_deps", "n") !=
> "n")
> +                                       changed_deps_report =
> self._dynamic_config.myparams.get(
> +                                               "changed_deps_report")
>                                         binpkg_changed_deps = (
>
> self._dynamic_config.myparams.get(
>                                                 "binpkg_changed_deps",
> "n") != "n")
> @@ -6395,9 +6486,13 @@ class depgraph(object):
>
> continue
>                                                                 break
>
> -                                               if (((installed and
> changed_deps) or
> -                                                       (not installed and
> binpkg_changed_deps)) and
> -
>  self._changed_deps(pkg)):
> +                                               installed_changed_deps =
> False
> +                                               if installed and
> (changed_deps or changed_deps_report):
> +
>  installed_changed_deps = self._changed_deps(pkg)
> +
> +                                               if
> ((installed_changed_deps and changed_deps) or
> +                                                       (not installed and
> binpkg_changed_deps and
> +
>  self._changed_deps(pkg))):
>                                                         if not installed:
>
> self._dynamic_config.\
>
> ignored_binaries.setdefault(
> @@ -8753,6 +8848,8 @@ class depgraph(object):
>
>                 self._show_ignored_binaries()
>
> +               self._changed_deps_report()
> +
>                 self._display_autounmask()
>
>                 for depgraph_sets in self._dynamic_config.sets.values():
> diff --git a/pym/_emerge/main.py b/pym/_emerge/main.py
> index 6a4bb87d0..fbc2ce01c 100644
> --- a/pym/_emerge/main.py
> +++ b/pym/_emerge/main.py
> @@ -136,6 +136,7 @@ def insert_optional_args(args):
>                 '--binpkg-changed-deps'  : y_or_n,
>                 '--buildpkg'             : y_or_n,
>                 '--changed-deps'         : y_or_n,
> +               '--changed-deps-report'  : y_or_n,
>                 '--complete-graph'       : y_or_n,
>                 '--deep'       : valid_integers,
>                 '--depclean-lib-check'   : y_or_n,
> @@ -408,6 +409,12 @@ def parse_opts(tmpcmdline, silent=False):
>                         "choices" : true_y_or_n
>                 },
>
> +               "--changed-deps-report": {
> +                       "help"    : ("report installed packages with "
> +                               "outdated dependencies"),
> +                       "choices" : true_y_or_n
> +               },
> +
>                 "--config-root": {
>                         "help":"specify the location for portage
> configuration files",
>                         "action":"store"
> @@ -833,6 +840,12 @@ def parse_opts(tmpcmdline, silent=False):
>                 else:
>                         myoptions.changed_deps = 'n'
>
> +       if myoptions.changed_deps_report is not None:
> +               if myoptions.changed_deps_report in true_y:
> +                       myoptions.changed_deps_report = 'y'
> +               else:
> +                       myoptions.changed_deps_report = 'n'
> +
>         if myoptions.changed_use is not False:
>                 myoptions.reinstall = "changed-use"
>                 myoptions.changed_use = False
> --
> 2.13.6
>
>
>