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

2018-01-28 Thread Zac Medico 
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"):

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

2018-01-28 Thread Zac Medico 
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
Description: OpenPGP digital signature

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

2018-01-28 Thread Zac Medico 
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

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

2018-01-28 Thread Zac Medico 
On 01/28/2018 08:34 PM, Alec Warner wrote:
> 
> 
> On Sun, Jan 28, 2018 at 8:10 PM, Zac Medico  > 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
Description: OpenPGP digital signature

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

2018-01-28 Thread Zac Medico 
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

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

2018-01-28 Thread Alec Warner 
On Sun, Jan 28, 2018 at 8:10 PM, Zac Medico  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
>
>

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

2018-01-28 Thread Zac Medico 
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
Description: OpenPGP digital signature

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

2018-01-28 Thread Michael Orlitzky 
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.

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

2018-01-28 Thread Michał Górny 
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  > > 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."
> 
> 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

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

2018-01-28 Thread Alec Warner 
On Sun, Jan 28, 2018 at 1:09 PM, Zac Medico  wrote:

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

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

2018-01-28 Thread Zac Medico 
On 01/28/2018 09:35 AM, Alec Warner wrote:
> 
> 
> On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico  > 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."

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
Description: OpenPGP digital signature

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

2018-01-28 Thread Alec Warner 
On Sun, Jan 28, 2018 at 9:51 AM, Zac Medico  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
>
> +

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

2018-01-28 Thread Zac Medico 
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
+
+

Re: [gentoo-portage-dev] [PATCH v2] install-qa-check: New QA check/cleanup for empty directories

2018-01-28 Thread Michał Górny 
W dniu nie, 28.01.2018 o godzinie 02∶22 -0800, użytkownik Zac Medico
napisał:
> On 01/28/2018 01:50 AM, Michał Górny wrote:
> > Warn about empty directories installed to /var in install-qa-check phase
> > (that were not "filled" using keepdir), to help developers stop relying
> > upon Portage preserving them. Those directories are rather unlikely to
> > be false positives.
> > 
> > Furthermore, remove all the empty directories if FEATURES=strict-keepdir
> > is used to catch even more problems (intended for developers). Here
> > warnings are not really suitable since there will be a high number
> > of false positives.
> > 
> > The PMS specifies the behavior upon merging empty directories
> > as undefined, and specifically prohibits ebuilds from attempting
> > to install empty directories. However, ebuilds occasionally still fall
> > into the trap of relying on 'dodir' preserving the directory. Make
> > the Portage behavior more strict in order to prevent that.
> > ---
> >  bin/install-qa-check.d/95empty-dirs | 43 
> > +
> >  1 file changed, 43 insertions(+)
> >  create mode 100644 bin/install-qa-check.d/95empty-dirs
> > 
> > diff --git a/bin/install-qa-check.d/95empty-dirs 
> > b/bin/install-qa-check.d/95empty-dirs
> > new file mode 100644
> > index 0..a9cda7fe1
> > --- /dev/null
> > +++ b/bin/install-qa-check.d/95empty-dirs
> > @@ -0,0 +1,43 @@
> > +# Warn about and/or remove empty directories installed by ebuild.
> > +
> > +# Rationale: PMS prohibits ebuilds from installing empty directories.
> > +# Cleaning them up from the installation image provides an easy way
> > +# to make sure that ebuilds are not relying on it while making it easy
> > +# for users to override this if they need to.
> > +#
> > +# The ebuilds that need to preserve empty directories should use keepdir
> > +# as documented e.g.:
> > +# 
> > https://devmanual.gentoo.org/function-reference/install-functions/index.html
> > +#
> > +# For now, we emit QA warnings for empty directories in /var.
> > +# Additionally, if FEATURES=strict-keepdir is enabled we explicitly
> > +# remove *all* empty directories to trigger breakage.
> > +
> > +find_empty_dirs() {
> > +   local warn_dirs=()
> > +   local d strip=
> > +
> > +   [[ ${FEATURES} == *strict-keepdir* ]] && strip=1
> 
> strict-keepdir has to be added to portage.const.SUPPORTED_FEATURES.
> Maybe also document it in man/make.conf.5?

Will do.

> 
> > +
> > +   while IFS= read -r -d $'\0' d; do
> > +   warn_dirs+=( "${d}" )
> > +   done < <(find "${ED}" -mindepth 1 -type d -empty -print0)
> > +
> > +   if [[ ${warn_dirs[@]} ]]; then
> > +   eqawarn "One or more empty directories installed to /var:"
> > +   eqawarn
> > +   for d in "${warn_dirs[@]}"; do
> > +   eqawarn "  ${d#${ED%/}}"
> > +   [[ ${strip} ]] && rmdir "${d}"
> 
> Do you also want the strip logic to traverse upwards, removing
> directories that contain only empty directories?

Yes, I suppose I've missed '-depth' here.

> 
> > +   done
> > +   eqawarn
> > +   eqawarn "If those directories need to be preserved, please make 
> > sure to create"
> > +   eqawarn "or mark them for keeping using 'keepdir'. Future 
> > versions of Portage"
> > +   eqawarn "will strip empty directories from installation image."
> > +   fi
> > +}
> > +
> > +find_empty_dirs
> > +: # guarantee successful exit
> > +
> > +# vim:ft=sh
> > 
> 
> 

-- 
Best regards,
Michał Górny

Re: [gentoo-portage-dev] [PATCH v2] install-qa-check: New QA check/cleanup for empty directories

2018-01-28 Thread Zac Medico 
On 01/28/2018 01:50 AM, Michał Górny wrote:
> Warn about empty directories installed to /var in install-qa-check phase
> (that were not "filled" using keepdir), to help developers stop relying
> upon Portage preserving them. Those directories are rather unlikely to
> be false positives.
> 
> Furthermore, remove all the empty directories if FEATURES=strict-keepdir
> is used to catch even more problems (intended for developers). Here
> warnings are not really suitable since there will be a high number
> of false positives.
> 
> The PMS specifies the behavior upon merging empty directories
> as undefined, and specifically prohibits ebuilds from attempting
> to install empty directories. However, ebuilds occasionally still fall
> into the trap of relying on 'dodir' preserving the directory. Make
> the Portage behavior more strict in order to prevent that.
> ---
>  bin/install-qa-check.d/95empty-dirs | 43 
> +
>  1 file changed, 43 insertions(+)
>  create mode 100644 bin/install-qa-check.d/95empty-dirs
> 
> diff --git a/bin/install-qa-check.d/95empty-dirs 
> b/bin/install-qa-check.d/95empty-dirs
> new file mode 100644
> index 0..a9cda7fe1
> --- /dev/null
> +++ b/bin/install-qa-check.d/95empty-dirs
> @@ -0,0 +1,43 @@
> +# Warn about and/or remove empty directories installed by ebuild.
> +
> +# Rationale: PMS prohibits ebuilds from installing empty directories.
> +# Cleaning them up from the installation image provides an easy way
> +# to make sure that ebuilds are not relying on it while making it easy
> +# for users to override this if they need to.
> +#
> +# The ebuilds that need to preserve empty directories should use keepdir
> +# as documented e.g.:
> +# 
> https://devmanual.gentoo.org/function-reference/install-functions/index.html
> +#
> +# For now, we emit QA warnings for empty directories in /var.
> +# Additionally, if FEATURES=strict-keepdir is enabled we explicitly
> +# remove *all* empty directories to trigger breakage.
> +
> +find_empty_dirs() {
> + local warn_dirs=()
> + local d strip=
> +
> + [[ ${FEATURES} == *strict-keepdir* ]] && strip=1

strict-keepdir has to be added to portage.const.SUPPORTED_FEATURES.
Maybe also document it in man/make.conf.5?

> +
> + while IFS= read -r -d $'\0' d; do
> + warn_dirs+=( "${d}" )
> + done < <(find "${ED}" -mindepth 1 -type d -empty -print0)
> +
> + if [[ ${warn_dirs[@]} ]]; then
> + eqawarn "One or more empty directories installed to /var:"
> + eqawarn
> + for d in "${warn_dirs[@]}"; do
> + eqawarn "  ${d#${ED%/}}"
> + [[ ${strip} ]] && rmdir "${d}"
Do you also want the strip logic to traverse upwards, removing
directories that contain only empty directories?

> + done
> + eqawarn
> + eqawarn "If those directories need to be preserved, please make 
> sure to create"
> + eqawarn "or mark them for keeping using 'keepdir'. Future 
> versions of Portage"
> + eqawarn "will strip empty directories from installation image."
> + fi
> +}
> +
> +find_empty_dirs
> +: # guarantee successful exit
> +
> +# vim:ft=sh
> 


-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature