Re: [gentoo-portage-dev] Signing off patches

2014-01-18 Thread Tom Wijsman
On Sat, 18 Jan 2014 08:43:12 -0800
W. Trevor King wk...@tremily.us wrote:

 On Sat, Jan 18, 2014 at 04:02:02PM +0100, Tom Wijsman wrote:
 I think the idea is that you shouldn't need to refer to an external
 resource like the mailing list to understand the idea behind the
 patch,

Either someone cares about the background of a patch or he/she doesn't.

The idea is not documented by these annotations, hence if one wants to
know the reasoning behind the certain way it is implemented he/she will
have to go to the mailing list to know that. That is if the existing
comments / commit message were insufficient for what one wonders about.

 or the amount of review it received.

The amount of review is a statistic; as there is no requirement for a
minimal amount of review(er)s, knowing that amount brings us no gain.

 The body of the commit message should summarize the consensus reached
 on the mailing list,

That message is written as part of the patch that is reviewed; it
rarely gets updated with the consensus, unless we suggest / require
people to do that. However, similar to vapier's response, I'd think
introducing such processes feel like unnecessary efforts.

 and these tags are basically standardized thank-you notes crediting
 non-authors who were involved in that process.  They don't have to go
 on every patch, but if you want to mention somebody:
 
   Reviewed-by: Random J Developer ran...@developer.example.org
   Reviewed-by: Other R Developer ot...@developer.example.org
 
 at the end of the commit message is easier to write and read than:
 
   This patch was reviewed Random J Developer
   ran...@developer.example.org and Other R Developer
   ot...@developer.example.org.

Exactly: Do we want to spend time on this or not? Do we add everyone
involved? Or do we just add people whom are not on the Portage team?

People in the team can be expected to be respectful and thankful, thus
I prefer the latter effort (non-Portage team only) if possible. 

Unless we intend to introduce this for statistics, although I think
that prior annotation history missing as well as people that will
casually forgot to add these annotations will make the statistics a
misrepresentation. At least as long as humans instead of a system add
these annotations, it be more nice to have a review system that adds
these for us; but well, that would be over-engineering for Portage...

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : tom...@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D


signature.asc
Description: PGP signature


Re: [gentoo-portage-dev] GitHub presence

2014-01-18 Thread Alexander Berntsen
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 18/01/14 22:52, Robin H. Johnson wrote:
 You mean like... https://github.com/gentoo/portage
This didn't exist back when. I should have done more research though.
Thanks.

- -- 
Alexander
alexan...@plaimi.net
http://plaimi.net/~alexander
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlLbDVMACgkQRtClrXBQc7XXsgD/akEHqlFs2ipNGyHI0tAt3XMD
V1IpKXrggIPhHrJUiD4BALNDcc0aHgkYPlf3ruzmnx0QUVqxgi+kfi5JYjWFIH3L
=pXb2
-END PGP SIGNATURE-



[gentoo-portage-dev] [PATCH 3/3] emerge: Make --autounmask=y if --ask=y

2014-01-18 Thread Alexander Berntsen
---
 man/emerge.1| 9 +
 pym/_emerge/depgraph.py | 5 +++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/man/emerge.1 b/man/emerge.1
index e23d1b4..52f8ed7 100644
--- a/man/emerge.1
+++ b/man/emerge.1
@@ -331,10 +331,11 @@ Write required unmask changes to the relevant config 
files, respecting
 \fBCONFIG_PROTECT\fR. If invoked together with \fB\-\-ask\fR, emerge will
 prompt you to write the changes. If invoked along with \fB\-\-pretend\fR,
 emerge will merely output the required changes and not make any of them by
-itself. If the corresponding package.* is a file, the changes are appended to
-it, if it is a directory, changes are written to the lexicographically last
-file. This way it is always ensured that the new changes take precedence over
-existing changes.
+itself. This option is enabled by default if are running emerge with
+\fB\-\-ask\fR or \fB\-\-pretend\fR, and disabled by default elsewise. If the
+corresponding package.* is a file, the changes are appended to it, if it is a
+directory, changes are written to the lexicographically last file. This way it
+is always ensured that the new changes take precedence over existing changes.
 .TP
 .BR \-\-autounmask\-unrestricted\-atoms [ y | n ]
 Keyword and mask changes using the \'=\' operator will be written. With this
diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index 5b4b723..e8b680d 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -6806,12 +6806,13 @@ class depgraph(object):
(using CONFIG_PROTECT). The message includes the comments and 
the changes.

 
-   autounmask_write = 
self._frozen_config.myopts.get(--autounmask, n) == True
+   ask = --ask in self._frozen_config.myopts
+   autounmask_write = ask or \
+   self._frozen_config.myopts.get(--autounmask, 
n) == True
autounmask_unrestricted_atoms = \

self._frozen_config.myopts.get(--autounmask-unrestricted-atoms, n) == True
quiet = --quiet in self._frozen_config.myopts
pretend = --pretend in self._frozen_config.myopts
-   ask = --ask in self._frozen_config.myopts
enter_invalid = '--ask-enter-invalid' in 
self._frozen_config.myopts
 
def check_if_latest(pkg):
-- 
1.8.3.2




[gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask

2014-01-18 Thread Alexander Berntsen
Rename --autounmask-write to --autounmask. Please note that removing the
option does not mean that the variable used for keeping track of
autounmask writing is not removed from depgraph.py.
---
 man/emerge.1| 19 +++
 pym/_emerge/depgraph.py |  4 ++--
 pym/_emerge/main.py | 18 +-
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/man/emerge.1 b/man/emerge.1
index 58bdc94..e23d1b4 100644
--- a/man/emerge.1
+++ b/man/emerge.1
@@ -322,10 +322,20 @@ invalid input. This helps prevent accidental
 acceptance of the first choice. This option is
 intended to be set in the \fBmake.conf\fR(5)
 \fBEMERGE_DEFAULT_OPTS\fR variable.
-\fB\-\-autounmask\-write\fR option. The
+\fB\-\-autounmask\fR option. The
 \fBEMERGE_DEFAULT_OPTS\fR variable may be used to
 disable this option by default in \fBmake.conf\fR(5).
 .TP
+.BR \-\-autounmask [ y | n ]
+Write required unmask changes to the relevant config files, respecting
+\fBCONFIG_PROTECT\fR. If invoked together with \fB\-\-ask\fR, emerge will
+prompt you to write the changes. If invoked along with \fB\-\-pretend\fR,
+emerge will merely output the required changes and not make any of them by
+itself. If the corresponding package.* is a file, the changes are appended to
+it, if it is a directory, changes are written to the lexicographically last
+file. This way it is always ensured that the new changes take precedence over
+existing changes.
+.TP
 .BR \-\-autounmask\-unrestricted\-atoms [ y | n ]
 Keyword and mask changes using the \'=\' operator will be written. With this
 option, \'=\' operators will be used whenever possible. USE and license
@@ -335,13 +345,6 @@ changes always use the latter behavior.
 No package.unmask or ** keyword changes will be created if this is activated.
 This leads to unsatisfied dependencies if no other solution exists.
 .TP
-.BR \-\-autounmask\-write [ y | n ]
-Write required unmask changes to the relevant config files, respecting
-\fBCONFIG_PROTECT\fR and \fB\-\-ask\fR. If the corresponding package.* is a
-file, the changes are appended to it, if it is a directory, changes are written
-to the lexicographically last file. This way it is always ensured that the new
-changes take precedence over existing changes.
-.TP
 .BR \-\-backtrack=COUNT
 Specifies an integer number of times to backtrack if
 dependency calculation fails due to a conflict or an
diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index 9fdfa43..5b4b723 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -6806,7 +6806,7 @@ class depgraph(object):
(using CONFIG_PROTECT). The message includes the comments and 
the changes.

 
-   autounmask_write = 
self._frozen_config.myopts.get(--autounmask-write, n) == True
+   autounmask_write = 
self._frozen_config.myopts.get(--autounmask, n) == True
autounmask_unrestricted_atoms = \

self._frozen_config.myopts.get(--autounmask-unrestricted-atoms, n) == True
quiet = --quiet in self._frozen_config.myopts
@@ -7168,7 +7168,7 @@ class depgraph(object):
chk_updated_cfg_files(root,
[os.path.join(os.sep, 
USER_CONFIG_PATH)])
elif not pretend and not autounmask_write and roots:
-   writemsg(\nUse --autounmask-write to write changes to 
config files (honoring\n
+   writemsg(\nUse --autounmask to write changes to config 
files (honoring\n
CONFIG_PROTECT). Carefully examine the list of 
proposed changes,\n
paying special attention to mask or keyword 
changes that may expose\n
experimental or unstable packages.\n,
diff --git a/pym/_emerge/main.py b/pym/_emerge/main.py
index fc73ef7..247317d 100644
--- a/pym/_emerge/main.py
+++ b/pym/_emerge/main.py
@@ -123,9 +123,9 @@ def insert_optional_args(args):
 
default_arg_opts = {
'--ask'  : y_or_n,
+   '--autounmask'   : y_or_n,
'--autounmask-keep-masks': y_or_n,
'--autounmask-unrestricted-atoms' : y_or_n,
-   '--autounmask-write' : y_or_n,
'--buildpkg' : y_or_n,
'--complete-graph'   : y_or_n,
'--deep'   : valid_integers,
@@ -302,6 +302,11 @@ def parse_opts(tmpcmdline, silent=False):
choices : true_y_or_n
},
 
+   --autounmask: {
+   help: automatically unmask packages,
+   choices : true_y_or_n
+   },
+
--autounmask-unrestricted-atoms: {
help: write autounmask changes with = atoms if 
possible,
choices : true_y_or_n
@@ -312,11 +317,6 @@ def 

Re: [gentoo-portage-dev] Signing off patches

2014-01-18 Thread Tom Wijsman
On Sat, 18 Jan 2014 15:24:59 -0800
W. Trevor King wk...@tremily.us wrote:

 If it doesn't need to get updated, then it probably already started
 out explaining the consensus ;).

That is a guess, you can look this up in past patches.

 You spend time if you want to spend time and add whoever you feel
 moved to add.

We discuss whether to make it policy to add involved people.

 I think the spirit of Alexander's original proposal [1]
 was “here is a common syntax for crediting collaborators, we might
 want to use it” not “ye non-conformers will be hounded unto the ends
 of the Earth”.

Alexander is clear that best practices could be followed and that it is
a proposal, note the use of the particular words should be; discussing
spirits is more appropriate in another place than this mailing list.

 If you are submitting v2 of a patch, and feel a desire
 to credit reviewers / testers with this syntax, I think that's
 considerate of you. If you are committing someone else's patch to
 master, and want to record the folks who acked it on the list to
 distribute responsibility, that's fine too. If you want to use
 another syntax, or not do any of this at all, it's still fine by me
 ;). However, if a consistent syntax already exists, I see no reason
 not to use it when it suits your purpose.

We discuss here whether to make it policy to use the same syntax.

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : tom...@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D


signature.asc
Description: PGP signature


[gentoo-portage-dev] [PATCH v3] Test for read-only filesystems, bail out during preinst if there are any which will be written to and display a useful error message. Fixes bug 378869.

2014-01-18 Thread Chris Reffett
v2: Reformat, add a function to return an appropriate read-only checker
for the operating system, so that this can be extended to other OSes.

v3: minor formatting tweaks from bernalex, including use os.path.join()
instead of string concatenation
---
 pym/portage/dbapi/vartree.py  | 32 +
 pym/portage/util/rochecker.py | 81 +++
 2 files changed, 113 insertions(+)
 create mode 100644 pym/portage/util/rochecker.py

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index ed62323..917634f 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -32,6 +32,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.util.env_update:env_update',
'portage.util.listdir:dircache,listdir',
'portage.util.movefile:movefile',
+   'portage.util.rochecker:get_ro_checker',
'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
'portage.util._async.SchedulerInterface:SchedulerInterface',
@@ -3508,6 +3509,8 @@ class dblink(object):

This function does the following:

+   calls get_ro_checker to retrieve a function for checking 
whether Portage
+   will write to a read-only filesystem, then runs it against the 
directory list
calls self._preserve_libs if FEATURES=preserve-libs
calls self._collision_protect if FEATURES=collision-protect
calls doebuild(mydo=pkg_preinst)
@@ -3685,6 +3688,7 @@ class dblink(object):
eagain_error = False
 
myfilelist = []
+   mydirlist = []
mylinklist = []
paths_with_newlines = []
def onerror(e):
@@ -3717,6 +3721,9 @@ class dblink(object):

unicode_errors.append(new_parent[ed_len:])
break
 
+   relative_path = parent[srcroot_len:]
+   mydirlist.append(os.path.join(/, 
relative_path))
+
for fname in files:
try:
fname = _unicode_decode(fname,
@@ -3829,6 +3836,31 @@ class dblink(object):
for other in others_in_slot])
prepare_build_dirs(settings=self.settings, cleanup=cleanup)
 
+   # Check for read-only filesystems
+   ro_checker = get_ro_checker()
+   rofilesystems = ro_checker(mydirlist)
+
+   if rofilesystems:
+   msg = _(One or more files installed to this package 
are 
+   set to be installed to read-only filesystems. 
+   Please mount the following filesystems as 
read-write 
+   and retry.)
+   msg = textwrap.wrap(msg, 70)
+   msg.append()
+   for f in rofilesystems:
+   msg.append(\t%s % os.path.join(destroot,
+   f.lstrip(os.path.sep)))
+   msg.append()
+   self._elog(eerror, preinst, msg)
+
+   msg = _(Package '%s' NOT merged due to read-only file 
systems.) % \
+   self.settings.mycpv
+   msg += _( If necessary, refer to your elog 
+   messages for the whole content of the above 
message.)
+   msg = textwrap.wrap(msg, 70)
+   eerror(msg)
+   return 1
+
# check for package collisions
blockers = self._blockers
if blockers is None:
diff --git a/pym/portage/util/rochecker.py b/pym/portage/util/rochecker.py
new file mode 100644
index 000..67e8131
--- /dev/null
+++ b/pym/portage/util/rochecker.py
@@ -0,0 +1,81 @@
+#-*- coding:utf-8 -*-
+# Copyright 2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+Methods to check whether Portage is going to write to read-only filesystems.
+Since the methods are not portable across different OSes, each OS needs its
+own method. To expand RO checking for different OSes, add a method which
+accepts a list of directories and returns a list of mounts which need to be
+remounted RW, then add elif ostype == (the ostype value for your OS) to
+get_ro_checker()
+
+from __future__ import unicode_literals
+
+import logging
+import re
+
+from portage.util import writemsg_level
+from portage.localization import _
+from portage.data import ostype
+
+
+def get_ro_checker():
+   
+   Uses the system type to find an appropriate method for 

Re: [gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask

2014-01-18 Thread Mike Gilbert
On Sat, Jan 18, 2014 at 7:21 PM, Alexander Berntsen
alexan...@plaimi.net wrote:
 Rename --autounmask-write to --autounmask. Please note that removing the
 option does not mean that the variable used for keeping track of
 autounmask writing is not removed from depgraph.py.

Ok, so how do I tell portage to suggest autounmask entries without
writing them to /etc/portage? Is that feature gone after your changes?



[gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size

2014-01-18 Thread W. Trevor King
The current fetch() function is quite long, which makes it hard to
know what I can change without adverse side effects.  By pulling this
logic out of the main function, we get clearer logic in fetch() and
more explicit input for the config extraction.
---
 pym/portage/package/ebuild/fetch.py | 50 -
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/pym/portage/package/ebuild/fetch.py 
b/pym/portage/package/ebuild/fetch.py
index 4337247..bd572fa 100644
--- a/pym/portage/package/ebuild/fetch.py
+++ b/pym/portage/package/ebuild/fetch.py
@@ -271,6 +271,32 @@ def _get_checksum_failure_max_tries(settings, default=5):
return v
 
 
+def _get_fetch_resume_size(settings, default='350K'):
+   v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE)
+   if v is not None:
+   v = .join(v.split())
+   if not v:
+   # If it's empty, silently use the default.
+   v = default
+   match = _fetch_resume_size_re.match(v)
+   if (match is None or
+   match.group(2).upper() not in _size_suffix_map):
+   writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE
+contains an unrecognized format: '%s'\n) % \
+   settings[PORTAGE_FETCH_RESUME_MIN_SIZE],
+   noiselevel=-1)
+   writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE 
+   default value: %s\n) % default,
+   noiselevel=-1)
+   v = None
+   if v is None:
+   v = default
+   match = _fetch_resume_size_re.match(v)
+   v = int(match.group(1)) * \
+   2 ** _size_suffix_map[match.group(2).upper()]
+   return v
+
+
 def fetch(myuris, mysettings, listonly=0, fetchonly=0,
locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None,
allow_missing_digests=True):
@@ -296,29 +322,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 
checksum_failure_max_tries = _get_checksum_failure_max_tries(
settings=mysettings)
-
-   fetch_resume_size_default = 350K
-   fetch_resume_size = mysettings.get(PORTAGE_FETCH_RESUME_MIN_SIZE)
-   if fetch_resume_size is not None:
-   fetch_resume_size = .join(fetch_resume_size.split())
-   if not fetch_resume_size:
-   # If it's undefined or empty, silently use the default.
-   fetch_resume_size = fetch_resume_size_default
-   match = _fetch_resume_size_re.match(fetch_resume_size)
-   if match is None or \
-   (match.group(2).upper() not in _size_suffix_map):
-   writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE
-contains an unrecognized format: '%s'\n) % \
-   mysettings[PORTAGE_FETCH_RESUME_MIN_SIZE], 
noiselevel=-1)
-   writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE 
-   default value: %s\n) % 
fetch_resume_size_default,
-   noiselevel=-1)
-   fetch_resume_size = None
-   if fetch_resume_size is None:
-   fetch_resume_size = fetch_resume_size_default
-   match = _fetch_resume_size_re.match(fetch_resume_size)
-   fetch_resume_size = int(match.group(1)) * \
-   2 ** _size_suffix_map[match.group(2).upper()]
+   fetch_resume_size = _get_fetch_resume_size(settings=mysettings)
 
# Behave like the package has RESTRICT=primaryuri after a
# couple of checksum failures, to increase the probablility
-- 
1.8.5.2.8.g0f6c0d1




[gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring

2014-01-18 Thread W. Trevor King
I felt like I should actually make a useful contribution to balance
the less useful commit-message discussion ;).  Browsing through the
open Portage bugs, #175612 looked interesting.  After I looked at
pym/portage/package/ebuild/fetch.py, I decided to take a step back and
just try and refactor fetch(), which was pushing 1k lines.  Here are
three paches pulling fairly self-contained blocks out of fetch().  I
thought I'd float them to the list to see if this was a productive
avenue, or if this is going to be too much work to review.  I tried to
avoid making too many changes other than the function-extraction, but
in some places I couldn't help myself ;).

The patches aren't particularly well tested yet.  I ran the test suite
and got some errors, but they seemed to be related to my non-root
invocation, and not due to these changes themselves.  I thought I'd
post my work so far, before digging deeper into the test suite.

Cheers,
Trevor

[1]: https://bugs.gentoo.org/show_bug.cgi?id=175612

W. Trevor King (3):
  pym/portage/package/ebuild/fetch.py: Factor out
_get_checksum_failure_max_tries
  pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
  pym/portage/package/ebuild/fetch.py: Factor out _get_uris

 pym/portage/package/ebuild/fetch.py | 305 +---
 1 file changed, 177 insertions(+), 128 deletions(-)

-- 
1.8.5.2.8.g0f6c0d1




[gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris

2014-01-18 Thread W. Trevor King
The current fetch() function is quite long, which makes it hard to
know what I can change without adverse side effects.  By pulling this
logic out of the main function, we get clearer logic in fetch() and
more explicit input for the config extraction.

This block was especially complicated, so I also created the helper
functions _get_file_uri_tuples and _expand_mirror.  I'd prefer if
_expand_mirror iterated through URIs instead of (group, URI) tuples,
but we need a distinct marker for third-party URIs to build
third_party_mirror_uris which is used to build primaryuri_dict which
is used way down in fetch():

  if checksum_failure_count == \
  checksum_failure_primaryuri:
  # Switch to primaryuri mode in order
  # to increase the probablility of
  # of success.
  primaryuris = \
  primaryuri_dict.get(myfile)
  if primaryuris:
  uri_list.extend(
  reversed(primaryuris))

I don't know if this is useful enough to motivate the uglier
_expandmirror return values, but I'll kick that can down the road for
now.
---
 pym/portage/package/ebuild/fetch.py | 197 +---
 1 file changed, 117 insertions(+), 80 deletions(-)

diff --git a/pym/portage/package/ebuild/fetch.py 
b/pym/portage/package/ebuild/fetch.py
index bd572fa..a617945 100644
--- a/pym/portage/package/ebuild/fetch.py
+++ b/pym/portage/package/ebuild/fetch.py
@@ -15,9 +15,9 @@ import sys
 import tempfile
 
 try:
-   from urllib.parse import urlparse
+   from urllib.parse import urlparse, urlunparse
 except ImportError:
-   from urlparse import urlparse
+   from urlparse import urlparse, urlunparse
 
 import portage
 portage.proxy.lazyimport.lazyimport(globals(),
@@ -297,6 +297,118 @@ def _get_fetch_resume_size(settings, default='350K'):
return v
 
 
+def _get_file_uri_tuples(uris):
+   Return a list of (filename, uri) tuples
+   
+   file_uri_tuples = []
+   # Check for 'items' attribute since OrderedDict is not a dict.
+   if hasattr(uris, 'items'):
+   for filename, uri_set in uris.items():
+   for uri in uri_set:
+   file_uri_tuples.append((filename, uri))
+   if not uri_set:
+   file_uri_tuples.append((filename, None))
+   else:
+   for uri in uris:
+   if urlparse(uri).scheme:
+   file_uri_tuples.append(
+   (os.path.basename(myuri), myuri))
+   else:
+   file_uri_tuples.append(
+   (os.path.basename(myuri), None))
+   return file_uri_tuples
+
+
+def _expand_mirror(uri, custom_mirrors=(), third_party_mirrors=()):
+   Replace the 'mirror://' scheme in the uri
+
+   Returns an iterable listing expanded (group, URI) tuples,
+   where the group is either 'custom' or 'third-party'.
+
+   parsed = urlparse(uri)
+   mirror = parsed.netloc
+   path = parsed.path
+   if path:
+   # Try user-defined mirrors first
+   if mirror in custom_mirrors:
+   for cmirr in custom_mirrors[mirror]:
+   m_uri = urlparse(cmirr)
+   yield ('custom', urlunparse((
+   cmirr.scheme, cmirr.netloc, path) +
+   parsed[3:]))
+
+   # now try the official mirrors
+   if mirror in third_party_mirrors:
+   uris = []
+   for locmirr in third_party_mirrors[mirror]:
+   m_uri = urlparse(cmirr)
+   uris.append(urlunparse((
+   cmirr.scheme, cmirr.netloc, path) +
+   parsed[3:]))
+   random.shuffle(uris)
+   for uri in uris:
+   yield ('third-party', uri)
+   else:
+   writemsg(_(Invalid mirror definition in SRC_URI:\n),
+noiselevel=-1)
+   writemsg(  %s\n % (uri), noiselevel=-1)
+
+
+def _get_uris(uris, settings, custom_mirrors=(), locations=()):
+   third_party_mirrors = settings.thirdpartymirrors()
+   third_party_mirror_uris = {}
+   filedict = OrderedDict()
+   primaryuri_dict = {}
+   for filename, uri in _get_file_uri_tuples(uris=uris):
+   if filename not in filedict:
+   filedict[filename] = [
+   os.path.join(location, 'distfiles', filename)
+   for location in locations]
+   if uri is None:
+   continue
+   if uri.startswith('mirror://'):
+   uris = _expand_mirror(
+

Re: [gentoo-portage-dev] Signing off patches

2014-01-18 Thread W. Trevor King
On Sun, Jan 19, 2014 at 02:33:06AM +0100, Tom Wijsman wrote:
 On Sat, 18 Jan 2014 15:24:59 -0800
 W. Trevor King wk...@tremily.us wrote:
  If it doesn't need to get updated, then it probably already
  started out explaining the consensus ;).
 
 That is a guess, you can look this up in past patches.

Sure.  Will you?  If I want to touch some code, and it looks
confusing, I'll use blame to see who wrote it and whay they were
thinking about.  If the commit message is not informative, I usually
give up.  I have a hard time imaging folks tracking down the thread
that spawned that patch, assuming such a thread even exists, for each
troublesome line they'd like to touch.  It's much easier to summarize
any issues the list resolved (because they're likely the same
questions the new dev is asking) in the commit message, where future
developers can find them.

  You spend time if you want to spend time and add whoever you feel
  moved to add.
 
 We discuss whether to make it policy to add involved people.

But “involved” can be hard to pin down, especially by someone who may
be applying v5 of a patch that hasn't been carefully following the
whole discussion in earlier versions.  The Linux kernel docs say [1]:

  If this patch fixes a problem reported by somebody else, consider
  adding a Reported-by: tag to credit the reporter for their
  contribution.

Note the “consider” wiggle word.  They are a bit more formal about
Reviewed-by, but only because it's signing off on their Reviewer's
statement of oversight.  In general, if you're not signing some
statement with the tag, formalizing “involved” is hard.

  If you are submitting v2 of a patch, and feel a desire
  to credit reviewers / testers with this syntax, I think that's
  considerate of you. If you are committing someone else's patch to
  master, and want to record the folks who acked it on the list to
  distribute responsibility, that's fine too. If you want to use
  another syntax, or not do any of this at all, it's still fine by me
  ;). However, if a consistent syntax already exists, I see no reason
  not to use it when it suits your purpose.

 We discuss here whether to make it policy to use the same syntax.

I don't understand the distinction between “here are some guidelines,
apply as and if you see fit” and “make it a policy to …”.  Say you
have a situation like this:

  1. Alice submits a bug-report to bugs.g.o.
  2. Bob codes up a Portage patch and sends it to the list.
  3. Charlie responds to Bob's patch on the list with Reviewed-by.
  4. Dan responds to Bob's patch on the list with Reviewed-by, and
 asks for any opposition.

 … time passes, and nobody speaks up …

  5. Dan applies Bob's patch to the master branch, but neglects:

   Submitted-by: Alice a...@example.net
   Reviewed-by: Charlie c...@example.net
   Reviewed-by: Dan d...@example.net

  6. ?

As I understand it, 6 should be:

  6a. Everyone gets on with their lives.

I could see a situation where:

  6b. Charlie reminds Dan that he could have used the tags.  Everyone
  gets on with their lives.

Is there another alternative step 6 implied by the “policy” keyword?
Or is the policy workflow even more different somehow?

Cheers,
Trevor

[1]: https://www.kernel.org/doc/Documentation/SubmittingPatches

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature