[PATCH] emerge: add --changed-deps-report option (bug 645780)

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] emerge: add --changed-deps-report option (bug 645780)

Zac Medico-2
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
---
 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


Reply | Threaded
Open this post in threaded view
|

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

Alec Warner-2


On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
On 01/28/2018 09:35 AM, Alec Warner wrote:

>
>
> On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email]
> <mailto:[hidden email]>> 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/
>     <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/
>     <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."
I think in practice we need to revbump for most changes. If the changes
weren't worth propagating, then we wouldn't make them.

> Ultimately I'm unsure what we are trying to accomplish here. Do we think
> Devs are doing 4 on accident?

It doesn't matter whether it's intentional or an accident. The problem
is that users need to learn to react appropriately, and I think
--change-deps-report is probably the most efficient way to train them.

> 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.)

Sure that can be done, but it will take some work. Honestly I think the
devs can get it right without all that, they just need some
reinforcement that has been absent up until now.

Meanwhile, users need to be trained to cope with whatever comes their way.

> 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;

I really do think that the devs can learn to get it right with a little
reinforcement.

> 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.

The thing is, we let commits flow directly from git to rsync. Good luck
with finding volunteers to be the gatekeepers for this.
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Alec Warner-2


On Sun, Jan 28, 2018 at 1:09 PM, Zac Medico <[hidden email]> wrote:
On 01/28/2018 09:35 AM, Alec Warner wrote:
>
>
> On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email]
> <mailto:[hidden email]>> 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:
>
>           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/
>     <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."

I think in practice we need to revbump for most changes. If the changes
weren't worth propagating, then we wouldn't make them.

> Ultimately I'm unsure what we are trying to accomplish here. Do we think
> Devs are doing 4 on accident?

It doesn't matter whether it's intentional or an accident. The problem
is that users need to learn to react appropriately, and I think
--change-deps-report is probably the most efficient way to train them.

I guess I'm less convinced users can / should do it.

Like they have to read the ebuild-revisions guide in the devmanual and decide if the revbump is required or not?
Do we think users are able to do this?

Should we consider only showing reports to developers (e.g. putting it behind a 'dev' FEATURE?)
 

> 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.)

Sure that can be done, but it will take some work. Honestly I think the
devs can get it right without all that, they just need some
reinforcement that has been absent up until now.

Meanwhile, users need to be trained to cope with whatever comes their way.

> 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;

I really do think that the devs can learn to get it right with a little
reinforcement.

> 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.

The thing is, we let commits flow directly from git to rsync. Good luck
with finding volunteers to be the gatekeepers for this.

I'm not sure I follow on this part.

Today we let commits flow from git to rsync. This patch proposes that emerge detect when deps of installed ebuilds change w/o a revbump.
These reports encourage *users* to take an action (file a bug.)

I'm suggesting that we automate that action.

"here is a report for x,y,z, file a bug? (Y/n)"

And users hit Y and we file a bug report with a template and all the required fields filled in (because emerge has them.)

I'm not proposing a 'gatekeeper'.

-A

--
Thanks,
Zac


Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
On 01/28/2018 10:47 AM, Alec Warner wrote:

>
>
> On Sun, Jan 28, 2018 at 1:09 PM, Zac Medico <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 01/28/2018 09:35 AM, Alec Warner wrote:
>     >
>     >
>     > On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email] <mailto:[hidden email]>
>     > <mailto:[hidden email] <mailto:[hidden email]>>> 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/
>     <https://devmanual.gentoo.org/general-concepts/ebuild-revisions/>
>     >   
>      <https://devmanual.gentoo.org/general-concepts/ebuild-revisions/
>     <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/
>     <https://gitweb.gentoo.org/repo/gentoo.git/>
>     >     <https://gitweb.gentoo.org/repo/gentoo.git/
>     <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."
>
>     I think in practice we need to revbump for most changes. If the changes
>     weren't worth propagating, then we wouldn't make them.
>
>     > Ultimately I'm unsure what we are trying to accomplish here. Do we think
>     > Devs are doing 4 on accident?
>
>     It doesn't matter whether it's intentional or an accident. The problem
>     is that users need to learn to react appropriately, and I think
>     --change-deps-report is probably the most efficient way to train them.
>
>
> I guess I'm less convinced users can / should do it.
>
> Like they have to read the ebuild-revisions guide in the devmanual and
> decide if the revbump is required or not?
> Do we think users are able to do this?
Maybe some. The worst case is that they file a bug and the developer
gets to see a useful message in the bug report.

I don't expect this case to be the norm, because I expect that
developers usually do the right thing given the necessary reinforcement.

> Should we consider only showing reports to developers (e.g. putting it
> behind a 'dev' FEATURE?)

What are the users supposed to do if they have dependency calculation
failure and they don't know about the --changed-deps option? Why not
instantly notified them that the --changed-deps option is available to
solve their problems?

Again, I don't expect this case to be the norm. It's a safety net.

>     > 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.)
>
>     Sure that can be done, but it will take some work. Honestly I think the
>     devs can get it right without all that, they just need some
>     reinforcement that has been absent up until now.
>
>     Meanwhile, users need to be trained to cope with whatever comes
>     their way.
>
>     > 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;
>
>     I really do think that the devs can learn to get it right with a little
>     reinforcement.
>
>     > 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.
>
>     The thing is, we let commits flow directly from git to rsync. Good luck
>     with finding volunteers to be the gatekeepers for this.
>
>
> I'm not sure I follow on this part.
>
> Today we let commits flow from git to rsync. This patch proposes that
> emerge detect when deps of installed ebuilds change w/o a revbump.
> These reports encourage *users* to take an action (file a bug.)
>
> I'm suggesting that we automate that action.
>
> "here is a report for x,y,z, file a bug? (Y/n)"
>
> And users hit Y and we file a bug report with a template and all the
> required fields filled in (because emerge has them.)
>
> I'm not proposing a 'gatekeeper'.
Okay, so how do we train users to deal with the fallout, given that
there's no gate?
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Michał Górny-5
In reply to this post by Zac Medico-2
W dniu nie, 28.01.2018 o godzinie 10∶09 -0800, użytkownik Zac Medico
napisał:

> On 01/28/2018 09:35 AM, Alec Warner wrote:
> >
> >
> > On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email]
> > <mailto:[hidden email]>> 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/
> >     <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/
> >     <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."
>
> I think in practice we need to revbump for most changes. If the changes
> weren't worth propagating, then we wouldn't make them.

It's not about 'being worth propagating', it's about 'being worth
the rebuild to propagate'.

I've bumped dependency inside all LLVM ebuilds today. The change fixes
only problem that hits people who:

a. don't use --deep when upgrading,

b. use clang with LTO.

I can't say how many people were hit by it since 5.0.0 was introduced
but yesterday I've got the first (and only) report so far.

Yes, I could technically revbump and cause people to have to spend most
of the day rebuilding 1-2 versions of LLVM for change that doesn't make
any difference to them.

Yes, I could consider the change 'not worth making'. But why shouldn't I
improve stuff for our users when it doesn't harm to do so?

But with your suggested solution, now we no longer have the choice
'revbump or not revbump'. Now we have a choice between forcing people to
rebuild via revbump or forcing people to get verbose report that most
likely will result in meaningless bug report and/or rebuild. So I end up
having a choice between 'force people to rebuild' or 'not fix minor bugs
at all'.

--
Best regards,
Michał Górny


Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
On 01/28/2018 12:16 PM, Michał Górny wrote:

> W dniu nie, 28.01.2018 o godzinie 10∶09 -0800, użytkownik Zac Medico
> napisał:
>> On 01/28/2018 09:35 AM, Alec Warner wrote:
>>>
>>>
>>> On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico <[hidden email]
>>> <mailto:[hidden email]>> 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/
>>>     <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/
>>>     <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."
>>
>> I think in practice we need to revbump for most changes. If the changes
>> weren't worth propagating, then we wouldn't make them.
>
> It's not about 'being worth propagating', it's about 'being worth
> the rebuild to propagate'.
>
> I've bumped dependency inside all LLVM ebuilds today. The change fixes
> only problem that hits people who:
>
> a. don't use --deep when upgrading,
>
> b. use clang with LTO.
>
> I can't say how many people were hit by it since 5.0.0 was introduced
> but yesterday I've got the first (and only) report so far.
>
> Yes, I could technically revbump and cause people to have to spend most
> of the day rebuilding 1-2 versions of LLVM for change that doesn't make
> any difference to them.
I can see how that would be troublesome for someone who runs ~arch and
syncs once or more per day. For a stable user that syncs once a week or
less often, it's an entirely different story.

> Yes, I could consider the change 'not worth making'. But why shouldn't I
> improve stuff for our users when it doesn't harm to do so?
>
> But with your suggested solution, now we no longer have the choice
> 'revbump or not revbump'. Now we have a choice between forcing people to
> rebuild via revbump or forcing people to get verbose report that most
> likely will result in meaningless bug report and/or rebuild. So I end up
> having a choice between 'force people to rebuild' or 'not fix minor bugs
> at all'.

Instead of suggesting to file a bug report, I think it should link to a
wiki page that we will allow us to easily refine the content over time.
Hopefully that will address everyone's concerns.

My main intent is to provide a safety net for users, such that they'll
be automatically notified that that --changed-deps option is available
when needed to solve problems involving stale dependencies.
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Michael Orlitzky
In reply to this post by Zac Medico-2
Since ::gentoo is the only repository governed by the PMS, can't we make
repoman do this? The problem with requiring "repoman --force" for an
in-place dependency change is that repoman generally won't have access
to the unedited ebuild; but for ::gentoo, we can probably hack something
together in git. Have it source the old and new ebuilds, and compare
their dependency lists.

It won't work outside of a git repo, but it will work in the one place
that really matters.

Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
On 01/28/2018 04:17 PM, Michael Orlitzky wrote:
> Since ::gentoo is the only repository governed by the PMS, can't we make
> repoman do this? The problem with requiring "repoman --force" for an
> in-place dependency change is that repoman generally won't have access
> to the unedited ebuild; but for ::gentoo, we can probably hack something
> together in git. Have it source the old and new ebuilds, and compare
> their dependency lists.

Yes I'm in favor of that, but it doesn't accomplish my current goal, as
I'll explain below.

> It won't work outside of a git repo, but it will work in the one place
> that really matters.

My goal for this patch is to provide a safety net for users, such that
they'll be automatically notified that the --changed-deps option is
available when needed to solve problems involving stale dependencies.
The reason behind this goal is that git commits flow directly to rsync
without any restriction, which makes users vulnerable to stale
dependencies, in the form of failed dependency calculations.

This goal is currently my highest priority, since I want to protect
users from having difficulty with problems triggered by the new
--dynamic-deps=n default [1].

[1] https://bugs.gentoo.org/645550
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Alec Warner-2


On Sun, Jan 28, 2018 at 8:10 PM, Zac Medico <[hidden email]> wrote:
On 01/28/2018 04:17 PM, Michael Orlitzky wrote:
> Since ::gentoo is the only repository governed by the PMS, can't we make
> repoman do this? The problem with requiring "repoman --force" for an
> in-place dependency change is that repoman generally won't have access
> to the unedited ebuild; but for ::gentoo, we can probably hack something
> together in git. Have it source the old and new ebuilds, and compare
> their dependency lists.

Yes I'm in favor of that, but it doesn't accomplish my current goal, as
I'll explain below.

> It won't work outside of a git repo, but it will work in the one place
> that really matters.

My goal for this patch is to provide a safety net for users, such that
they'll be automatically notified that the --changed-deps option is
available when needed to solve problems involving stale dependencies.
The reason behind this goal is that git commits flow directly to rsync
without any restriction, which makes users vulnerable to stale
dependencies, in the form of failed dependency calculations.

What I would go along with is:

1) If we can determine the depgraph failed and
2) We can determine that dependencies changed.
3) Show a NOTE telling users about --changed-deps=y

But I'm not sure the patch does this today? I think it will trigger too aggressively. But ultimately I'm not against helping users work around this
sort of breakage.


This goal is currently my highest priority, since I want to protect
users from having difficulty with problems triggered by the new
--dynamic-deps=n default [1].

[1] https://bugs.gentoo.org/645550
--
Thanks,
Zac


Reply | Threaded
Open this post in threaded view
|

[PATCH v2] emerge: add --changed-deps-report option (bug 645780)

Zac Medico-2
In reply to this post by Zac Medico-2
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 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: Refer to the following page for more information about dependency
      change(s) without revision bump:

          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps

      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
---
[PATCH v2] changes:

 * Remove bug report instructions, and instead refer to
   https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps

 * Suppress HINT about --changed-deps when --dynamic-deps is enabled

 man/emerge.1                          | 10 ++++
 pym/_emerge/create_depgraph_params.py |  5 ++
 pym/_emerge/depgraph.py               | 92 +++++++++++++++++++++++++++++++++--
 pym/_emerge/main.py                   | 13 +++++
 4 files changed, 117 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..6561a57c3 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,79 @@ 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: Refer to the following page for more information about dependency",
+ "      change(s) without revision bump:",
+ "",
+ "          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps",
+ "",
+ "      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
+ # problems that are solvable by using --changed-deps.
+ if (self._dynamic_config.myparams.get("changed_deps", "n") == "n" and
+ self._dynamic_config.myparams.get("dynamic_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 +2643,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 +6432,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 +6475,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 +8837,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


Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
In reply to this post by Alec Warner-2
On 01/28/2018 08:34 PM, Alec Warner wrote:

>
>
> On Sun, Jan 28, 2018 at 8:10 PM, Zac Medico <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 01/28/2018 04:17 PM, Michael Orlitzky wrote:
>     > Since ::gentoo is the only repository governed by the PMS, can't we make
>     > repoman do this? The problem with requiring "repoman --force" for an
>     > in-place dependency change is that repoman generally won't have access
>     > to the unedited ebuild; but for ::gentoo, we can probably hack something
>     > together in git. Have it source the old and new ebuilds, and compare
>     > their dependency lists.
>
>     Yes I'm in favor of that, but it doesn't accomplish my current goal, as
>     I'll explain below.
>
>     > It won't work outside of a git repo, but it will work in the one place
>     > that really matters.
>
>     My goal for this patch is to provide a safety net for users, such that
>     they'll be automatically notified that the --changed-deps option is
>     available when needed to solve problems involving stale dependencies.
>     The reason behind this goal is that git commits flow directly to rsync
>     without any restriction, which makes users vulnerable to stale
>     dependencies, in the form of failed dependency calculations.
>
>
> What I would go along with is:
>
> 1) If we can determine the depgraph failed and
If we use depgraph failure as a condition, then we're ignoring
potentially missed updates due to stale dependencies. So, it's safer to
show the message regardless of depgraph failure, and that's what it does.

> 2) We can determine that dependencies changed.

Yes, _changed_deps_report returns silently if there are no changed
dependencies. Also, it returns silently with --quiet if none of the
packages with changed dependencies are in the graph.

Actually, if none of the packages with changed dependencies are in the
graph, then we can be silent even without --quiet because the emphasis
on filing bugs has been removed in PATCH v2. So, in PATCH v3, I'll make
it silent regardless of --quiet in this case.

> 3) Show a NOTE telling users about --changed-deps=y

This is in the HINT section, which is displayed if both --changed-deps
and --dynamic-deps are disabled in PATCH v2.

>
> But I'm not sure the patch does this today? I think it will trigger too
> aggressively. But ultimately I'm not against helping users work around this
> sort of breakage.

Do my responses above sound reasonable?
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH v3] emerge: add --changed-deps-report option (bug 645780)

Zac Medico-2
In reply to this post by Zac Medico-2
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 report is entirely suppressed if none of the packages with
changed dependencies are in the graph, since they are entirely
harmless to the user in this case. This suppresses noise for the
unaffected user, even though some of the changed dependencies
might be worthy of revision bumps.

The --quiet option suppresses the 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. The HINT
section is suppressed if either --changed-deps or --dynamic-deps
is enabled.

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: Refer to the following page for more information about dependency
      change(s) without revision bump:

          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps

      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
---
[PATCH v3] changes:

 * Entirely suppress the report if none of the packages with
   changed dependencies are in the graph, since they are harmless
   to the user in this case. This suppresses noise for the
   unaffected user, even though some of the changed dependencies
   might be worthy of revision bumps.

 man/emerge.1                          | 10 ++++
 pym/_emerge/create_depgraph_params.py |  5 ++
 pym/_emerge/depgraph.py               | 94 +++++++++++++++++++++++++++++++++--
 pym/_emerge/main.py                   | 13 +++++
 4 files changed, 119 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..6738cdf18 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,81 @@ 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. This suppresses
+ # noise for the unaffected user, even though some of the changed
+ # dependencies might be worthy of revision bumps.
+ if 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: Refer to the following page for more information about dependency",
+ "      change(s) without revision bump:",
+ "",
+ "          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps",
+ "",
+ "      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
+ # problems that are solvable by using --changed-deps.
+ if (self._dynamic_config.myparams.get("changed_deps", "n") == "n" and
+ self._dynamic_config.myparams.get("dynamic_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 +2645,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 +6434,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 +6477,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 +8839,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


Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
In reply to this post by Zac Medico-2
On 01/28/2018 09:49 PM, Zac Medico wrote:
>> 3) Show a NOTE telling users about --changed-deps=y
>
> This is in the HINT section, which is displayed if both --changed-deps
> and --dynamic-deps are disabled in PATCH v2.

Actually, the whole report should be suppressed if either --changed-deps
or --dynamic-deps is enabled, so I'll send PATCH v4 for that.
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH v4] emerge: add --changed-deps-report option (bug 645780)

Zac Medico-2
In reply to this post by Zac Medico-2
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 report is entirely suppressed in the following cases in which
the packages with changed dependencies are entirely harmless to the
user:

  * --changed-deps or --dynamic-deps is enabled
  * none of the packages with changed deps are in the graph

These cases suppress noise for the unaffected user, even though some
of the changed dependencies might be worthy of revision bumps.

The --quiet option suppresses the 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: Refer to the following page for more information about dependency
      change(s) without revision bump:

          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps

      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
---
[PATCH v4] changes:

* Entirely suppress the report if --changed-deps or --dynamic-deps is enabled


 man/emerge.1                          | 10 ++++
 pym/_emerge/create_depgraph_params.py |  5 ++
 pym/_emerge/depgraph.py               | 98 +++++++++++++++++++++++++++++++++--
 pym/_emerge/main.py                   | 13 +++++
 4 files changed, 123 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..d937264ab 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,85 @@ 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. This is
+ completely silent in the following cases:
+
+  * --changed-deps or --dynamic-deps is enabled
+  * none of the packages with changed deps are in the graph
+ """
+ if (self._dynamic_config.myparams.get("changed_deps", "n") == "y" or
+ self._dynamic_config.myparams.get("dynamic_deps", "n") == "y"):
+ return
+
+ 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. This suppresses
+ # noise for the unaffected user, even though some of the changed
+ # dependencies might be worthy of revision bumps.
+ if 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 '--quiet' not in self._frozen_config.myopts:
+ msg.extend([
+ "",
+ "NOTE: Refer to the following page for more information about dependency",
+ "      change(s) without revision bump:",
+ "",
+ "          https://wiki.gentoo.org/wiki/Project:Portage/Changed_Deps",
+ "",
+ "      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
+ # problems that are solvable by using --changed-deps.
+ 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 +2649,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 +6438,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 +6481,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 +8843,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


Reply | Threaded
Open this post in threaded view
|

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

Duncan-42
In reply to this post by Zac Medico-2
Zac Medico posted on Sun, 28 Jan 2018 22:21:48 -0800 as excerpted:

> On 01/28/2018 09:49 PM, Zac Medico wrote:
>>> 3) Show a NOTE telling users about --changed-deps=y
>>
>> This is in the HINT section, which is displayed if both --changed-deps
>> and --dynamic-deps are disabled in PATCH v2.
>
> Actually, the whole report should be suppressed if either --changed-deps
> or --dynamic-deps is enabled, so I'll send PATCH v4 for that.

This is shaping up quite nicely and by (1) dramatically shortening the
original "wall of text" report and (2) aborting the report if no affected
packages are in the graph, it's vastly improved from the original.

I definitely expect it to be rather helpful here, since I have both
--dynamic-deps and --changed-deps off by default, and seeing that list
could be /quite/ helpful.  Looking forward to it! =:^)

My remaining concern, and I'm not sure there's a solution, is that for
routine 30-day-plus updaters, the warning could quickly become "routine
noise", due to valid no-r-bump exceptions such as the llvm example mgorny
provided, which very well /could/ happen often enough to trigger the
warning nearly every time for 30-day-plus updaters.  Then when it really
counts and could help, people will likely be ignoring it.

Maybe someone else has an idea, but as I said it's already vastly
improved from the original, and I believe usable as-is, now, while I'd
have found the original quite irritating by about the third time I saw
it, even if also helpful.

--
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman


Reply | Threaded
Open this post in threaded view
|

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

Zac Medico-2
On 01/29/2018 07:29 PM, Duncan wrote:

> Zac Medico posted on Sun, 28 Jan 2018 22:21:48 -0800 as excerpted:
>
>> On 01/28/2018 09:49 PM, Zac Medico wrote:
>>>> 3) Show a NOTE telling users about --changed-deps=y
>>>
>>> This is in the HINT section, which is displayed if both --changed-deps
>>> and --dynamic-deps are disabled in PATCH v2.
>>
>> Actually, the whole report should be suppressed if either --changed-deps
>> or --dynamic-deps is enabled, so I'll send PATCH v4 for that.
>
> This is shaping up quite nicely and by (1) dramatically shortening the
> original "wall of text" report and (2) aborting the report if no affected
> packages are in the graph, it's vastly improved from the original.
>
> I definitely expect it to be rather helpful here, since I have both
> --dynamic-deps and --changed-deps off by default, and seeing that list
> could be /quite/ helpful.  Looking forward to it! =:^)
Great!

> My remaining concern, and I'm not sure there's a solution, is that for
> routine 30-day-plus updaters, the warning could quickly become "routine
> noise", due to valid no-r-bump exceptions such as the llvm example mgorny
> provided, which very well /could/ happen often enough to trigger the
> warning nearly every time for 30-day-plus updaters.  Then when it really
> counts and could help, people will likely be ignoring it.

Until we invent something better, people will have to set
EMERGE_DEFAULT_OPTS="--changed-deps-report=n" if it bothers them too
much. This is acceptable to me because my main goal is simply to make
people aware of --changed-deps when they need it most. If the set
--changed-deps-report=n then it's their responsibility to know when to
use --changed-deps.

> Maybe someone else has an idea, but as I said it's already vastly
> improved from the original, and I believe usable as-is, now, while I'd
> have found the original quite irritating by about the third time I saw
> it, even if also helpful.

Great, thanks for the feedback!
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment