Re: [gentoo-portage-dev] [PATCH v2] emerge --search: auto-detect regular expressions (bug 737480)

2020-09-02 Thread Michał Górny
s for matches of the supplied string in the ebuild repository.
>  By default emerge uses a case-insensitive simple search, but you can
> -enable a regular expression search by prefixing the search string with %.
> +enable a regular expression search by prefixing the search string with %
> +(the % prefix can often be omitted if the
> +\fB\-\-regex\-search\-auto\fR option is enabled).
>  For example, \fBemerge \-\-search "%^kde"\fR searches for any package whose
>  name starts with "kde"; \fBemerge \-\-search "%gcc$"\fR searches for any
>  package that ends with "gcc"; \fBemerge \-\-search "office"\fR searches for
> @@ -764,6 +766,14 @@ matching packages due to \fB\-\-rebuild\fR.
>  A space separated list of package names or slot atoms. Emerge will not 
> rebuild
>  packages that depend on matching packages due to \fB\-\-rebuild\fR.
>  .TP
> +.BR "\-\-regex\-search\-auto < y | n >"
> +Enable or disable automatic regular expression detection for search actions.
> +If this option is enabled (the default), then regular expression search
> +is automatically enabled when the search string is a valid regular expression
> +which contains any of these commonly used regular expression characters or
> +character sequences:
> +^ $ * [ ] { } | ? .+
> +.TP
>  .BR \-\-oneshot ", " \-1
>  Emerge as normal, but do not add the packages to the world file
>  for later updating.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] profile masking

2020-08-14 Thread Michał Górny
On Fri, 2020-08-14 at 15:42 +, Joakim Tjernlund wrote:
> On Fri, 2020-08-14 at 17:31 +0200, Ulrich Mueller wrote:
> > > > > > > On Fri, 14 Aug 2020, Joakim Tjernlund wrote:
> > > When pkgs are masked in the profile, it affects all variants of that
> > > pkgs, even the ones that are in other overlays.
> > > Example:
> > > !!! The following installed packages are masked:
> > > - sys-auth/sssd-::transmode (masked by: package.mask)
> > > /usr/portage/profiles/package.mask:
> > > # Matt Turner  (2020-08-13)
> > > # Masked for testing
> > > My sssd- is now masked.
> > > Could the profile syntax be extended to include syntax allowed in
> > > /etc/portage ? Then one could use the ::gentoo syntax (or so I hope)
> > 
> > The :: syntax is Portage specific and doesn't exist in EAPI 7.
> > So there's no chance to get it into the profile dir anytime soon
> > (because that would imply :: to be added to a future EAPI and the
> > top-level profile dir to be bumped to that EAPI).
> 
> Is profile part of EAPI? masks are not defined/used in ebuilds directly.
> 
> > You could override the mask in your overlay's profile/package.mask
> > instead, using an entry with the "-" operator.
> 
> Yes, I know I can add that in profile/package.mask but I am looking for the 
> bigger
> picture here. This has to stop somehow, there need to be something that limits
> the mask scope to the repo/overlay it is defined.
> 

Why is that?  I dare say the bigger picture needs to include different
mask reasons.  Sure, 'masked for testing' may or may not make little
sense for live ebuilds.  However, 'masked for security issues' may
pretty apply to custom repo ebuilds as well.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH 1/3] Add cached portage.getpid() function

2020-08-07 Thread Michał Górny
On Fri, 2020-08-07 at 21:08 -0700, Zac Medico wrote:
> Since getpid is a syscall, cache results, and update them
> via an after fork hook.
> 
> Signed-off-by: Zac Medico 
> ---
>  lib/portage/__init__.py   | 14 +++
>  .../tests/process/test_AsyncFunction.py   | 24 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/lib/portage/__init__.py b/lib/portage/__init__.py
> index 916c93510..52902ba7d 100644
> --- a/lib/portage/__init__.py
> +++ b/lib/portage/__init__.py
> @@ -14,6 +14,7 @@ try:
>   if not hasattr(errno, 'ESTALE'):
>   # ESTALE may not be defined on some systems, such as interix.
>   errno.ESTALE = -1
> + import multiprocessing.util
>   import re
>   import types
>   import platform
> @@ -368,6 +369,19 @@ _internal_caller = False
>  
>  _sync_mode = False
>  
> +def _fork_watcher(_fork_watcher):
> + _fork_watcher.current_pid = _os.getpid()

I don't really like the idea of putting variables on functions.  Would
there be any problem with using a proper class here?

> +
> +_fork_watcher(_fork_watcher)
> +
> +multiprocessing.util.register_after_fork(_fork_watcher, _fork_watcher)
> +
> +def getpid():
> + """
> + Cached version of os.getpid(). ForkProcess updates the cache.
> + """
> + return _fork_watcher.current_pid
> +
>  def _get_stdin():
>   """
>   Buggy code in python's multiprocessing/process.py closes sys.stdin
> diff --git a/lib/portage/tests/process/test_AsyncFunction.py 
> b/lib/portage/tests/process/test_AsyncFunction.py
> index 55857026d..3b360e02f 100644
> --- a/lib/portage/tests/process/test_AsyncFunction.py
> +++ b/lib/portage/tests/process/test_AsyncFunction.py
> @@ -3,6 +3,7 @@
>  
>  import sys
>  
> +import portage
>  from portage import os
>  from portage.tests import TestCase
>  from portage.util._async.AsyncFunction import AsyncFunction
> @@ -36,3 +37,26 @@ class AsyncFunctionTestCase(TestCase):
>   def testAsyncFunctionStdin(self):
>   loop = asyncio._wrap_loop()
>   loop.run_until_complete(self._testAsyncFunctionStdin(loop))
> +
> + def _test_getpid_fork(self):
> + """
> + Verify that portage.getpid() cache is updated in a forked child 
> process.
> + """
> + loop = asyncio._wrap_loop()
> + proc = AsyncFunction(scheduler=loop, target=portage.getpid)
> + proc.start()
> + proc.wait()
> + self.assertEqual(proc.pid, proc.result)
> +
> + def test_getpid_fork(self):
> + self._test_getpid_fork()
> +
> + def test_getpid_double_fork(self):
> + """
> + Verify that portage.getpid() cache is updated correctly after
> + two forks.
> + """
> + loop = asyncio._wrap_loop()
> + proc = AsyncFunction(scheduler=loop, 
> target=self._test_getpid_fork)
> + proc.start()
> + self.assertEqual(proc.wait(), 0)

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] lib/*: Fix useless-return

2020-08-07 Thread Michał Górny
f print_changelog(self):
> @@ -687,7 +683,6 @@ class Display:
>   if ebuild_path_cl is not None:
>   self.changelogs.extend(_calc_changelog(
>   ebuild_path_cl, pkg_info.previous_pkg, 
> pkg.cpv))
> - return
>  
>  
>   def check_system_world(self, pkg):
> diff --git a/lib/portage/elog/mod_custom.py b/lib/portage/elog/mod_custom.py
> index 7cfafeccc..aaf1d3b1b 100644
> --- a/lib/portage/elog/mod_custom.py
> +++ b/lib/portage/elog/mod_custom.py
> @@ -18,4 +18,3 @@ def process(mysettings, key, logentries, fulltext):
>   retval = portage.process.spawn_bash(mylogcmd)
>   if retval != 0:
>   raise portage.exception.PortageException("!!! 
> PORTAGE_ELOG_COMMAND failed with exitcode %d" % retval)
> - return
> diff --git a/lib/portage/elog/mod_echo.py b/lib/portage/elog/mod_echo.py
> index 80f2b11ac..a026847b7 100644
> --- a/lib/portage/elog/mod_echo.py
> +++ b/lib/portage/elog/mod_echo.py
> @@ -3,9 +3,10 @@
>  # Distributed under the terms of the GNU General Public License v2
>  
>  import sys
> -from portage.output import EOutput, colorize
> +
>  from portage.const import EBUILD_PHASES
>  from portage.localization import _
> +from portage.output import EOutput, colorize
>  
>  
>  _items = []
> @@ -61,4 +62,3 @@ def _finalize():
>   for line in msgcontent:
>   fmap[msgtype](line.strip("\n"))
>   _items = []
> - return
> diff --git a/lib/portage/elog/mod_mail.py b/lib/portage/elog/mod_mail.py
> index 38eaa277f..f737a80ce 100644
> --- a/lib/portage/elog/mod_mail.py
> +++ b/lib/portage/elog/mod_mail.py
> @@ -41,5 +41,3 @@ def process(mysettings, key, logentries, fulltext):
>   portage.mail.send_mail(mysettings, mymessage)
>   except PortageException as e:
>   writemsg("%s\n" % str(e), noiselevel=-1)
> -
> - return
> diff --git a/lib/portage/glsa.py b/lib/portage/glsa.py
> index 9260e7e09..1870d9338 100644
> --- a/lib/portage/glsa.py
> +++ b/lib/portage/glsa.py
> @@ -492,7 +492,6 @@ class Glsa:
>   finally:
>   f.close()
>  
> - return None
>  
>   def parse(self, myfile):
>   """
> @@ -583,7 +582,6 @@ class Glsa:
>   self.packages[name].append(tmp)
>   # TODO: services aren't really used yet
>   self.services = self.affected.getElementsByTagName("service")
> - return None
>  
>   def dump(self, outstream=sys.stdout, encoding="utf-8"):
>   """
> @@ -684,7 +682,6 @@ class Glsa:
>   mode='a+', encoding=_encodings['content'], 
> errors='strict')
>   checkfile.write(_unicode_decode(self.nr + "\n"))
>   checkfile.close()
> - return None
>  
>   def getMergeList(self, least_change=True):
>   """
> diff --git a/lib/portage/mail.py b/lib/portage/mail.py
> index 6503b4cc9..f4fccd8c2 100644
> --- a/lib/portage/mail.py
> +++ b/lib/portage/mail.py
> @@ -136,4 +136,3 @@ def send_mail(mysettings, message):
>   raise portage.exception.PortageException(_("!!! An 
> error occurred while trying to send logmail:\n")+str(e))
>   except socket.error as e:
>   raise portage.exception.PortageException(_("!!! A 
> network error occurred while trying to send logmail:\n%s\nSure you configured 
> PORTAGE_ELOG_MAILURI correctly?") % str(e))
> - return
> diff --git a/lib/portage/sync/controller.py b/lib/portage/sync/controller.py
> index cb68e2c37..24ebf4ff8 100644
> --- a/lib/portage/sync/controller.py
> +++ b/lib/portage/sync/controller.py
> @@ -174,7 +174,7 @@ class SyncManager:
>  
>  
>   def do_callback(self, result):
> - #print("result:", result, "callback()", self.callback)
> + # print("result:", result, "callback()", self.callback)
>   exitcode, updatecache_flg = result
>   self.exitcode = exitcode
>   self.updatecache_flg = updatecache_flg
> @@ -184,7 +184,6 @@ class SyncManager:
>   writemsg_level(msg + "\n")
>   if self.callback:
>   self.callback(exitcode, updatecache_flg)
> - return
>  
>  
>   def perform_post_sync_hook(self, reponame, dosyncuri='', 
> repolocation='

[gentoo-portage-dev] [PATCH] Remove py<3.4 import compatibility

2020-07-17 Thread Michał Górny
---
 bin/binhost-snapshot  |  5 +--
 lib/_emerge/BinpkgFetcher.py  |  5 +--
 lib/_emerge/BlockerCache.py   |  6 +---
 lib/portage/cache/anydbm.py   | 31 +---
 lib/portage/dbapi/bintree.py  |  5 +--
 lib/portage/dbapi/porttree.py |  6 +---
 lib/portage/dbapi/vartree.py  |  6 +---
 lib/portage/getbinpkg.py  | 36 +--
 lib/portage/glsa.py   |  5 +--
 lib/portage/package/ebuild/fetch.py   | 12 ++-
 .../asyncio/test_policy_wrapper_recursion.py  |  8 +
 lib/portage/tests/util/futures/test_retry.py  |  5 +--
 lib/portage/tests/util/test_socks5.py | 11 ++
 lib/portage/tests/util/test_xattr.py  | 14 +---
 lib/portage/util/__init__.py  |  5 +--
 .../util/_dyn_libs/PreservedLibsRegistry.py   |  6 +---
 lib/portage/util/_eventloop/EventLoop.py  | 10 ++
 .../util/_eventloop/asyncio_event_loop.py |  9 ++---
 lib/portage/util/_urlopen.py  | 12 ++-
 lib/portage/util/futures/_asyncio/__init__.py |  7 ++--
 lib/portage/util/futures/_asyncio/tasks.py|  7 +---
 lib/portage/util/futures/events.py| 12 +++
 lib/portage/util/futures/futures.py   | 36 ---
 lib/portage/util/futures/transports.py|  5 +--
 lib/portage/util/futures/unix_events.py   | 21 ---
 25 files changed, 61 insertions(+), 224 deletions(-)

diff --git a/bin/binhost-snapshot b/bin/binhost-snapshot
index d677e7568..afab23592 100755
--- a/bin/binhost-snapshot
+++ b/bin/binhost-snapshot
@@ -8,10 +8,7 @@ import os
 import sys
 import textwrap
 
-try:
-   from urllib.parse import urlparse
-except ImportError:
-   from urlparse import urlparse
+from urllib.parse import urlparse
 
 from os import path as osp
 if osp.isfile(osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), 
".portage_not_installed")):
diff --git a/lib/_emerge/BinpkgFetcher.py b/lib/_emerge/BinpkgFetcher.py
index c26c9bd6b..01c9bc1e6 100644
--- a/lib/_emerge/BinpkgFetcher.py
+++ b/lib/_emerge/BinpkgFetcher.py
@@ -6,10 +6,7 @@ import functools
 from _emerge.AsynchronousLock import AsynchronousLock
 from _emerge.CompositeTask import CompositeTask
 from _emerge.SpawnProcess import SpawnProcess
-try:
-   from urllib.parse import urlparse as urllib_parse_urlparse
-except ImportError:
-   from urlparse import urlparse as urllib_parse_urlparse
+from urllib.parse import urlparse as urllib_parse_urlparse
 import stat
 import sys
 import portage
diff --git a/lib/_emerge/BlockerCache.py b/lib/_emerge/BlockerCache.py
index d0beb46ac..5f8fb360e 100644
--- a/lib/_emerge/BlockerCache.py
+++ b/lib/_emerge/BlockerCache.py
@@ -7,11 +7,7 @@ from portage.util import writemsg
 from portage.data import secpass
 import portage
 from portage import os
-
-try:
-   import cPickle as pickle
-except ImportError:
-   import pickle
+import pickle
 
 class BlockerCache(portage.cache.mappings.MutableMapping):
"""This caches blockers of installed packages so that dep_check does not
diff --git a/lib/portage/cache/anydbm.py b/lib/portage/cache/anydbm.py
index 121a4eaf2..c02d85b4f 100644
--- a/lib/portage/cache/anydbm.py
+++ b/lib/portage/cache/anydbm.py
@@ -4,29 +4,14 @@
 
 from __future__ import absolute_import
 
-try:
-   import anydbm as anydbm_module
-except ImportError:
-   # python 3.x
-   import dbm as anydbm_module
+import dbm
 
 try:
import dbm.gnu as gdbm
 except ImportError:
-   try:
-   import gdbm
-   except ImportError:
-   gdbm = None
+   gdbm = None
 
-try:
-   from dbm import whichdb
-except ImportError:
-   from whichdb import whichdb
-
-try:
-   import cPickle as pickle
-except ImportError:
-   import pickle
+import pickle
 from portage import _unicode_encode
 from portage import os
 import sys
@@ -53,15 +38,15 @@ class database(fs_template.FsBased):
self._db_path = os.path.join(self.location, 
fs_template.gen_label(self.location, self.label)+default_db)
self.__db = None
mode = "w"
-   if whichdb(self._db_path) in ("dbm.gnu", "gdbm"):
+   if dbm.whichdb(self._db_path) in ("dbm.gnu", "gdbm"):
# Allow multiple concurrent writers (see bug #53607).
mode += "u"
try:
# dbm.open() will not work with bytes in python-3.1:
#   TypeError: can't concat bytes to str
-   self.__db = anydbm_module.open(self._db_path,
+   self.__db = dbm.open(self._db_path,
mode, self._perms)
-   except anydbm_module.error:
+   except dbm.error:
# XXX handle this at some point
try:

Re: [gentoo-portage-dev] [PATCH 2/3] Remove support code for Python < 3.3

2020-07-17 Thread Michał Górny
Dnia July 17, 2020 8:10:25 AM UTC, Ulrich Mueller  napisał(a):
>>>>>> On Fri, 17 Jul 2020, Michał Górny wrote:
>
>> --- a/lib/portage/util/_dyn_libs/PreservedLibsRegistry.py
>> +++ b/lib/portage/util/_dyn_libs/PreservedLibsRegistry.py
>> @@ -34,12 +34,9 @@ class PreservedLibsRegistry(object):
>>  
>>  _json_write_opts = {
>>  "ensure_ascii": False,
>> -"indent": "\t",
>> -"sort_keys": True
>> +"indent": 4,
>
>This is assigning 4 now unconditionally, while it was "\t" before for
>modern Python versions. Is this intentional?

I'm sorry, this is my mistakd. I'll fix it when I get home.

>
>> +"sort_keys": True,
>
>Why the added comma?
>
>>  }
>> -if sys.hexversion < 0x30200F0:
>> -# indent only supports int number of spaces
>> -_json_write_opts["indent"] = 4
>>  
>>  def __init__(self, root, filename):
>>  """
>
>
>> --- a/lib/portage/util/mtimedb.py
>> +++ b/lib/portage/util/mtimedb.py
>> @@ -29,12 +29,9 @@ class MtimeDB(dict):
>>  
>>  _json_write_opts = {
>>  "ensure_ascii": False,
>> -"indent": "\t",
>> +"indent": 4,
>
>Same here.
>
>>  "sort_keys": True
>>  }
>> -if sys.hexversion < 0x30200F0:
>> -# indent only supports int number of spaces
>> -_json_write_opts["indent"] = 4
>>  
>>  def __init__(self, filename):
>>  dict.__init__(self)
>
>Ulrich


--
Best regards, 
Michał Górny



[gentoo-portage-dev] [PATCH 1/3] Remove support code for Python < 3.2

2020-07-16 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 lib/portage/tests/process/test_poll.py  |  9 +++--
 .../tests/util/futures/asyncio/test_pipe_closed.py  |  7 ++-
 lib/portage/util/configparser.py| 13 +++--
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/lib/portage/tests/process/test_poll.py 
b/lib/portage/tests/process/test_poll.py
index f505b5049..3ea176c0d 100644
--- a/lib/portage/tests/process/test_poll.py
+++ b/lib/portage/tests/process/test_poll.py
@@ -1,4 +1,4 @@
-# Copyright 1998-2019 Gentoo Authors
+# Copyright 1998-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import functools
@@ -34,11 +34,8 @@ class PipeReaderTestCase(TestCase):
 
def test_domain_socket(self):
def make_pipes():
-   if sys.version_info >= (3, 2):
-   read_end, write_end = socket.socketpair()
-   return (read_end.detach(), write_end.detach()), 
None
-   else:
-   self.skipTest('socket detach not supported')
+   read_end, write_end = socket.socketpair()
+   return (read_end.detach(), write_end.detach()), None
self._do_test(make_pipes)
 
def test_named_pipe(self):
diff --git a/lib/portage/tests/util/futures/asyncio/test_pipe_closed.py 
b/lib/portage/tests/util/futures/asyncio/test_pipe_closed.py
index 507385c04..b8e5556d0 100644
--- a/lib/portage/tests/util/futures/asyncio/test_pipe_closed.py
+++ b/lib/portage/tests/util/futures/asyncio/test_pipe_closed.py
@@ -1,4 +1,4 @@
-# Copyright 2018 Gentoo Foundation
+# Copyright 2018-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import errno
@@ -32,10 +32,7 @@ class _PipeClosedTestCase(object):
self._do_test(read_end, write_end)
 
def test_domain_socket(self):
-   if sys.version_info >= (3, 2):
-   read_end, write_end = socket.socketpair()
-   else:
-   self.skipTest('socket detach not supported')
+   read_end, write_end = socket.socketpair()
self._do_test(read_end.detach(), write_end.detach())
 
def test_named_pipe(self):
diff --git a/lib/portage/util/configparser.py b/lib/portage/util/configparser.py
index f3452231f..9bd9f9722 100644
--- a/lib/portage/util/configparser.py
+++ b/lib/portage/util/configparser.py
@@ -11,16 +11,9 @@ __all__ = ['ConfigParserError', 'NoOptionError', 
'ParsingError',
 import io
 import sys
 
-try:
-   from configparser import (Error as ConfigParserError,
-   NoOptionError, ParsingError, RawConfigParser)
-   if sys.hexversion >= 0x302:
-   from configparser import ConfigParser as SafeConfigParser
-   else:
-   from configparser import SafeConfigParser
-except ImportError:
-   from ConfigParser import (Error as ConfigParserError,
-   NoOptionError, ParsingError, RawConfigParser, SafeConfigParser)
+from configparser import (Error as ConfigParserError,
+   NoOptionError, ParsingError, RawConfigParser)
+from configparser import ConfigParser as SafeConfigParser
 
 from portage import _encodings
 from portage import _unicode_encode
-- 
2.27.0




[gentoo-portage-dev] [PATCH 2/3] Remove support code for Python < 3.3

2020-07-16 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 bin/chpathtool.py |  8 +--
 bin/doins.py  | 14 +
 lib/portage/_emirrordist/FetchTask.py | 13 ++--
 lib/portage/dbapi/vartree.py  | 25 ++--
 lib/portage/util/_compare_files.py| 22 +++
 .../util/_dyn_libs/PreservedLibsRegistry.py   |  7 +--
 lib/portage/util/movefile.py  | 60 +--
 lib/portage/util/mtimedb.py   |  7 +--
 8 files changed, 39 insertions(+), 117 deletions(-)

diff --git a/bin/chpathtool.py b/bin/chpathtool.py
index fbd18b987..c036046ae 100755
--- a/bin/chpathtool.py
+++ b/bin/chpathtool.py
@@ -128,12 +128,8 @@ def chpath_inplace(filename, is_text_file, old, new):
 
f.close()
if modified:
-   if sys.hexversion >= 0x303:
-   orig_mtime = orig_stat.st_mtime_ns
-   os.utime(filename, ns=(orig_mtime, orig_mtime))
-   else:
-   orig_mtime = orig_stat[stat.ST_MTIME]
-   os.utime(filename, (orig_mtime, orig_mtime))
+   orig_mtime = orig_stat.st_mtime_ns
+   os.utime(filename, ns=(orig_mtime, orig_mtime))
return modified
 
 def chpath_inplace_symlink(filename, st, old, new):
diff --git a/bin/doins.py b/bin/doins.py
index 98dc4f810..a08e3f8c9 100644
--- a/bin/doins.py
+++ b/bin/doins.py
@@ -110,10 +110,6 @@ def _parse_install_options(
parser.add_argument('-p', '--preserve-timestamps', action='store_true')
split_options = shlex.split(options)
namespace, remaining = parser.parse_known_args(split_options)
-   if namespace.preserve_timestamps and sys.version_info < (3, 3):
-   # -p is not supported in this case, since timestamps cannot
-   # be preserved with full precision
-   remaining.append('-p')
# Because parsing '--mode' option is partially supported. If unknown
# arg for --mode is passed, namespace.mode is set to None.
if remaining or namespace.mode is None:
@@ -151,15 +147,7 @@ def _set_timestamps(source_stat, dest):
source_stat: stat result for the source file.
dest: path to the dest file.
"""
-   os.utime(dest, (source_stat.st_atime, source_stat.st_mtime))
-
-
-if sys.version_info >= (3, 3):
-   def _set_timestamps_ns(source_stat, dest):
-   os.utime(dest, ns=(source_stat.st_atime_ns, 
source_stat.st_mtime_ns))
-
-   _set_timestamps_ns.__doc__ = _set_timestamps.__doc__
-   _set_timestamps = _set_timestamps_ns
+   os.utime(dest, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns))
 
 
 class _InsInProcessInstallRunner(object):
diff --git a/lib/portage/_emirrordist/FetchTask.py 
b/lib/portage/_emirrordist/FetchTask.py
index 322de79ba..a1ba58822 100644
--- a/lib/portage/_emirrordist/FetchTask.py
+++ b/lib/portage/_emirrordist/FetchTask.py
@@ -1,4 +1,4 @@
-# Copyright 2013-2019 Gentoo Authors
+# Copyright 2013-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__ import division
@@ -385,14 +385,9 @@ class FetchTask(CompositeTask):
# Apply the timestamp from the source file, but
# just rely on umask for permissions.
try:
-   if sys.hexversion >= 0x303:
-   os.utime(copier.dest_path,
-   
ns=(self._current_stat.st_mtime_ns,
-   self._current_stat.st_mtime_ns))
-   else:
-   os.utime(copier.dest_path,
-   
(self._current_stat[stat.ST_MTIME],
-   
self._current_stat[stat.ST_MTIME]))
+   os.utime(copier.dest_path,
+   ns=(self._current_stat.st_mtime_ns,
+   self._current_stat.st_mtime_ns))
except OSError as e:
msg = "%s %s utime failed unexpectedly: %s" % \
(self.distfile, current_mirror.name, e)
diff --git a/lib/portage/dbapi/vartree.py b/lib/portage/dbapi/vartree.py
index abb0a9308..2e29b25e5 100644
--- a/lib/portage/dbapi/vartree.py
+++ b/lib/portage/dbapi/vartree.py
@@ -435,10 +435,7 @@ class vardbapi(dbapi):
if mysplit[0] == '*':
mysplit[0] = mysplit[0][1:]
try:
-   if sys.hexversion >= 0x303:
-   mystat = 
os.stat(self.getpath(mysplit[0])).st_mtime_ns
-   else:
-   mystat = 
os

[gentoo-portage-dev] [PATCH 3/3] Remove support code for Python < 3.4

2020-07-16 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 lib/_emerge/AsynchronousLock.py   | 12 +---
 lib/_emerge/EbuildMetadataPhase.py| 12 +---
 lib/_emerge/FifoIpcDaemon.py  | 30 +--
 lib/_emerge/PipeReader.py | 12 +---
 lib/_emerge/SpawnProcess.py   | 16 --
 lib/portage/dbapi/_MergeProcess.py| 10 ---
 lib/portage/locks.py  | 11 ---
 lib/portage/process.py|  2 +-
 lib/portage/util/_async/PipeLogger.py | 10 ---
 lib/portage/util/_eventloop/EventLoop.py  | 23 --
 .../util/_eventloop/global_event_loop.py  | 15 --
 lib/portage/util/futures/_asyncio/__init__.py | 26 +---
 lib/portage/util/futures/unix_events.py   |  4 +--
 13 files changed, 20 insertions(+), 163 deletions(-)

diff --git a/lib/_emerge/AsynchronousLock.py b/lib/_emerge/AsynchronousLock.py
index aed1bcb15..d2a6773ff 100644
--- a/lib/_emerge/AsynchronousLock.py
+++ b/lib/_emerge/AsynchronousLock.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Gentoo Foundation
+# Copyright 2010-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import fcntl
@@ -192,16 +192,6 @@ class _LockProcess(AbstractPollTask):
fcntl.fcntl(in_pr, fcntl.F_SETFL,
fcntl.fcntl(in_pr, fcntl.F_GETFL) | os.O_NONBLOCK)
 
-   # FD_CLOEXEC is enabled by default in Python >=3.4.
-   if sys.hexversion < 0x304:
-   try:
-   fcntl.FD_CLOEXEC
-   except AttributeError:
-   pass
-   else:
-   fcntl.fcntl(in_pr, fcntl.F_SETFD,
-   fcntl.fcntl(in_pr, fcntl.F_GETFD) | 
fcntl.FD_CLOEXEC)
-
self.scheduler.add_reader(in_pr, self._output_handler)
self._registered = True
self._proc = SpawnProcess(
diff --git a/lib/_emerge/EbuildMetadataPhase.py 
b/lib/_emerge/EbuildMetadataPhase.py
index efe71892c..d00f194c2 100644
--- a/lib/_emerge/EbuildMetadataPhase.py
+++ b/lib/_emerge/EbuildMetadataPhase.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2018 Gentoo Foundation
+# Copyright 1999-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from _emerge.SubProcess import SubProcess
@@ -93,16 +93,6 @@ class EbuildMetadataPhase(SubProcess):
fcntl.fcntl(master_fd, fcntl.F_SETFL,
fcntl.fcntl(master_fd, fcntl.F_GETFL) | os.O_NONBLOCK)
 
-   # FD_CLOEXEC is enabled by default in Python >=3.4.
-   if sys.hexversion < 0x304:
-   try:
-   fcntl.FD_CLOEXEC
-   except AttributeError:
-   pass
-   else:
-   fcntl.fcntl(master_fd, fcntl.F_SETFD,
-   fcntl.fcntl(master_fd, fcntl.F_GETFD) | 
fcntl.FD_CLOEXEC)
-
fd_pipes[slave_fd] = slave_fd
settings["PORTAGE_PIPE_FD"] = str(slave_fd)
 
diff --git a/lib/_emerge/FifoIpcDaemon.py b/lib/_emerge/FifoIpcDaemon.py
index 2ec69d1cb..ab1fdb572 100644
--- a/lib/_emerge/FifoIpcDaemon.py
+++ b/lib/_emerge/FifoIpcDaemon.py
@@ -1,14 +1,8 @@
-# Copyright 2010-2018 Gentoo Foundation
+# Copyright 2010-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import sys
 
-try:
-   import fcntl
-except ImportError:
-   #  http://bugs.jython.org/issue1074
-   fcntl = None
-
 from portage import os
 from _emerge.AbstractPollTask import AbstractPollTask
 from portage.cache.mappings import slot_dict_class
@@ -28,17 +22,6 @@ class FifoIpcDaemon(AbstractPollTask):
self._files.pipe_in = \
os.open(self.input_fifo, os.O_RDONLY|os.O_NONBLOCK)
 
-   # FD_CLOEXEC is enabled by default in Python >=3.4.
-   if sys.hexversion < 0x304 and fcntl is not None:
-   try:
-   fcntl.FD_CLOEXEC
-   except AttributeError:
-   pass
-   else:
-   fcntl.fcntl(self._files.pipe_in, fcntl.F_SETFD,
-   fcntl.fcntl(self._files.pipe_in,
-   fcntl.F_GETFD) | 
fcntl.FD_CLOEXEC)
-
self.scheduler.add_reader(
self._files.pipe_in,
self._input_handler)
@@ -55,17 +38,6 @@ class FifoIpcDaemon(AbstractPollTask):
self._files.pipe_in = \
os.open(self.input_fifo, os.O_RDONLY|os.O_NONBLOCK)
 
-   # FD_CLOEXEC is enable

[gentoo-portage-dev] [PATCH] Clean up more py2 conditional code

2020-07-16 Thread Michał Górny
Closes: https://github.com/gentoo/portage/pull/575
Signed-off-by: Michał Górny 
---
 lib/portage/cache/anydbm.py   |  4 +-
 lib/portage/cache/mappings.py | 45 +---
 lib/portage/cache/sql_template.py |  5 +-
 lib/portage/cache/template.py | 12 ++---
 lib/portage/elog/messages.py  |  3 +-
 lib/portage/output.py |  3 +-
 lib/portage/package/ebuild/config.py  | 11 +---
 .../futures/asyncio/test_subprocess_exec.py   |  4 --
 lib/portage/tests/util/futures/test_retry.py  |  2 -
 lib/portage/tests/util/test_socks5.py | 16 ++
 lib/portage/util/__init__.py  | 53 +--
 lib/portage/util/_dyn_libs/NeededEntry.py | 10 
 lib/portage/util/digraph.py   |  3 --
 lib/portage/util/listdir.py   |  2 -
 lib/portage/util/whirlpool.py | 25 -
 lib/portage/xpak.py   |  2 -
 16 files changed, 52 insertions(+), 148 deletions(-)

diff --git a/lib/portage/cache/anydbm.py b/lib/portage/cache/anydbm.py
index 88d85b0da..121a4eaf2 100644
--- a/lib/portage/cache/anydbm.py
+++ b/lib/portage/cache/anydbm.py
@@ -112,5 +112,5 @@ class database(fs_template.FsBased):
self.__db.sync()
self.__db.close()
 
-   if sys.hexversion >= 0x300:
-   items = iteritems
+   # TODO: do we need iteritems()?
+   items = iteritems
diff --git a/lib/portage/cache/mappings.py b/lib/portage/cache/mappings.py
index 0432fdf60..0adecde4a 100644
--- a/lib/portage/cache/mappings.py
+++ b/lib/portage/cache/mappings.py
@@ -25,9 +25,6 @@ class Mapping(object):
def __iter__(self):
return iter(self.keys())
 
-   def keys(self):
-   return list(self.__iter__())
-
def __contains__(self, key):
try:
value = self[key]
@@ -46,12 +43,6 @@ class Mapping(object):
for _, v in self.items():
yield v
 
-   def values(self):
-   return [v for _, v in self.iteritems()]
-
-   def items(self):
-   return list(self.iteritems())
-
def get(self, key, default=None):
try:
return self[key]
@@ -64,10 +55,10 @@ class Mapping(object):
def __len__(self):
return len(list(self))
 
-   if sys.hexversion >= 0x300:
-   items = iteritems
-   keys = __iter__
-   values = itervalues
+   # TODO: do we need to keep iter*?
+   items = iteritems
+   keys = __iter__
+   values = itervalues
 
 class MutableMapping(Mapping):
"""
@@ -184,8 +175,8 @@ class UserDict(MutableMapping):
def clear(self):
self.data.clear()
 
-   if sys.hexversion >= 0x300:
-   keys = __iter__
+   keys = __iter__
+
 
 class ProtectedDict(MutableMapping):
"""
@@ -234,8 +225,8 @@ class ProtectedDict(MutableMapping):
def __contains__(self, key):
return key in self.new or (key not in self.blacklist and key in 
self.orig)
 
-   if sys.hexversion >= 0x300:
-   keys = __iter__
+   keys = __iter__
+
 
 class LazyLoad(Mapping):
"""
@@ -271,8 +262,8 @@ class LazyLoad(Mapping):
self.pull = None
return key in self.d
 
-   if sys.hexversion >= 0x300:
-   keys = __iter__
+   keys = __iter__
+
 
 _slot_dict_classes = weakref.WeakValueDictionary()
 
@@ -328,9 +319,6 @@ def slot_dict_class(keys, prefix="_val_"):
l += 1
return l
 
-   def keys(self):
-   return list(self)
-
def iteritems(self):
prefix = self._prefix
for k in self.allowed_keys:
@@ -339,16 +327,10 @@ def slot_dict_class(keys, prefix="_val_"):
except AttributeError:
pass
 
-   def items(self):
-   return list(self.iteritems())
-
def itervalues(self):
for k, v in self.iteritems():
yield v
 
-   def values(self):
-   return list(self.itervalues())
-
def __delitem__(self, k):
try:
delattr(self, self._prefix + k)
@@ -447,10 +429,9 @@ def slot_dict_class(keys, prefix="_val_"):
def __repr__(self):
re

[gentoo-portage-dev] [PATCH] Eliminate the most of explicit py3 conditionals

2020-07-16 Thread Michał Górny
Eliminate the most of py2/py3 conditions in the code.  Leave a few
where the relevant code is unclear, they will be addressed later.

Closes: https://github.com/gentoo/portage/pull/574
Signed-off-by: Michał Górny 
---
 bin/check-implicit-pointer-usage.py   | 25 ++
 bin/chmod-lite.py | 11 ++---
 bin/dohtml.py | 11 ++---
 bin/doins.py  |  7 ++-
 bin/ebuild| 15 +-
 bin/filter-bash-environment.py| 11 ++---
 bin/install.py| 19 ---
 bin/pid-ns-init   | 10 ++--
 bin/xattr-helper.py   | 32 +++-
 lib/_emerge/DependencyArg.py  | 10 
 lib/_emerge/JobStatusDisplay.py   |  4 +-
 lib/_emerge/Package.py|  8 ---
 lib/_emerge/PackageVirtualDbapi.py|  3 --
 lib/_emerge/SequentialTaskQueue.py|  3 --
 lib/_emerge/TaskSequence.py   |  3 --
 lib/_emerge/UseFlagDisplay.py |  8 ---
 lib/_emerge/UserQuery.py  | 17 +++
 lib/_emerge/actions.py|  2 -
 lib/_emerge/resolver/DbapiProvidesIndex.py|  3 --
 lib/_emerge/resolver/output_helpers.py| 16 --
 lib/_emerge/resolver/slot_collision.py|  8 ---
 lib/portage/__init__.py   | 49 +++
 lib/portage/_emirrordist/Config.py|  4 --
 lib/portage/_selinux.py   | 12 -
 lib/portage/_sets/base.py |  3 --
 lib/portage/dbapi/porttree.py |  7 +--
 lib/portage/dep/__init__.py   | 18 +--
 lib/portage/dep/soname/SonameAtom.py  |  8 ---
 lib/portage/elog/mod_save_summary.py  |  2 -
 lib/portage/elog/mod_syslog.py|  4 --
 lib/portage/exception.py  | 45 -
 lib/portage/mail.py   | 49 +++
 lib/portage/manifest.py   |  7 ---
 lib/portage/output.py |  4 +-
 lib/portage/process.py| 16 +-
 lib/portage/proxy/objectproxy.py  |  6 ---
 lib/portage/repository/config.py  |  6 ---
 .../tests/unicode/test_string_format.py   | 44 -
 lib/portage/util/_ShelveUnicodeWrapper.py | 45 -
 39 files changed, 102 insertions(+), 453 deletions(-)
 delete mode 100644 lib/portage/util/_ShelveUnicodeWrapper.py

diff --git a/bin/check-implicit-pointer-usage.py 
b/bin/check-implicit-pointer-usage.py
index a49db8107..868e4b3c8 100755
--- a/bin/check-implicit-pointer-usage.py
+++ b/bin/check-implicit-pointer-usage.py
@@ -33,22 +33,10 @@ pointer_pattern = (
 + r"|"
 + r"cast to pointer from integer of different size)")
 
-if sys.hexversion < 0x300:
-# Use encoded byte strings in python-2.x, since the python ebuilds are
-# known to remove the encodings module when USE=build is enabled (thus
-# disabling unicode decoding/encoding). The portage module has a
-# workaround for this, but currently we don't import that here since we
-# don't want to trigger potential sandbox violations due to stale pyc
-# files for the portage module.
-unicode_quote_open = '\xE2\x80\x98'
-unicode_quote_close = '\xE2\x80\x99'
-def write(msg):
-sys.stdout.write(msg)
-else:
-unicode_quote_open = '\u2018'
-unicode_quote_close = '\u2019'
-def write(msg):
-sys.stdout.buffer.write(msg.encode('utf_8', 'backslashreplace'))
+unicode_quote_open = '\u2018'
+unicode_quote_close = '\u2019'
+def write(msg):
+sys.stdout.buffer.write(msg.encode('utf_8', 'backslashreplace'))
 
 pointer_pattern = re.compile(pointer_pattern)
 
@@ -57,10 +45,7 @@ last_implicit_linenum = -1
 last_implicit_func = ""
 
 while True:
-if sys.hexversion >= 0x300:
-line = sys.stdin.buffer.readline().decode('utf_8', 'replace')
-else:
-line = sys.stdin.readline()
+line = sys.stdin.buffer.readline().decode('utf_8', 'replace')
 if not line:
 break
 # translate unicode open/close quotes to ascii ones
diff --git a/bin/chmod-lite.py b/bin/chmod-lite.py
index 177be7eab..7fe743ed1 100755
--- a/bin/chmod-lite.py
+++ b/bin/chmod-lite.py
@@ -12,12 +12,11 @@ os.chdir(os.environ["__PORTAGE_HELPER_CWD"])
 
 def main(files):
 
-   if sys.hexversion >= 0x300:
-   # We can't trust that the filesystem encoding (locale dependent)
-   # correctly matches the arguments, so use surrogateescape to
-   # pass through the original argv bytes for Python 3.
-   fs_encoding = sys.getfilesystemencoding()
-   files = [x.encode(fs_encoding, 'surrogateescape') for x in 
files]
+   # We can't tru

[gentoo-portage-dev] [PATCH] Eliminate basestring/long/_unicode py3 compat

2020-07-15 Thread Michał Górny
Replace basestring and _unicode hacks with str, and long with int.

Signed-off-by: Michał Górny 
---
 bin/egencache |  7 +--
 lib/_emerge/BinpkgFetcher.py  |  5 +-
 lib/_emerge/BlockerCache.py   | 19 ++-
 lib/_emerge/BlockerDB.py  |  4 +-
 lib/_emerge/FakeVartree.py| 11 +---
 lib/_emerge/JobStatusDisplay.py   |  7 +--
 lib/_emerge/Package.py| 27 -
 lib/_emerge/Scheduler.py  |  5 +-
 lib/_emerge/actions.py| 21 +++
 lib/_emerge/create_world_atom.py  |  8 +--
 lib/_emerge/depgraph.py   | 15 ++---
 lib/_emerge/main.py   |  4 +-
 lib/_emerge/resolver/output.py| 26 -
 lib/_emerge/resolver/output_helpers.py|  6 +-
 lib/_emerge/resolver/slot_collision.py|  6 +-
 lib/portage/__init__.py   | 12 ++--
 lib/portage/_emirrordist/MirrorDistTask.py|  7 +--
 lib/portage/_emirrordist/main.py  |  9 +--
 lib/portage/_sets/base.py |  7 +--
 lib/portage/cache/flat_hash.py|  5 +-
 lib/portage/cache/fs_template.py  |  7 +--
 lib/portage/cache/index/pkg_desc_index.py | 10 +---
 lib/portage/cache/metadata.py |  8 +--
 lib/portage/cache/sqlite.py   |  7 +--
 lib/portage/cache/template.py | 19 ++-
 lib/portage/cvstree.py|  3 -
 lib/portage/dbapi/bintree.py  | 55 ---
 lib/portage/dbapi/porttree.py |  6 +-
 lib/portage/dbapi/vartree.py  | 33 +--
 lib/portage/dep/__init__.py   | 50 +++--
 lib/portage/eclass_cache.py   | 10 +---
 lib/portage/elog/__init__.py  |  5 +-
 lib/portage/elog/mod_echo.py  |  5 +-
 lib/portage/elog/mod_syslog.py|  5 +-
 lib/portage/emaint/modules/binhost/binhost.py |  9 +--
 lib/portage/emaint/modules/sync/sync.py   |  9 +--
 lib/portage/exception.py  |  9 +--
 lib/portage/getbinpkg.py  |  5 +-
 lib/portage/locks.py  | 17 +++---
 lib/portage/mail.py   |  7 +--
 lib/portage/manifest.py   | 12 +---
 lib/portage/package/ebuild/config.py  | 11 ++--
 lib/portage/package/ebuild/doebuild.py|  8 +--
 .../package/ebuild/getmaskingstatus.py|  7 +--
 lib/portage/process.py|  5 +-
 lib/portage/proxy/lazyimport.py   |  7 +--
 lib/portage/repository/config.py  |  5 +-
 lib/portage/sync/getaddrinfo_validate.py  |  6 +-
 lib/portage/sync/modules/rsync/rsync.py   |  7 +--
 lib/portage/tests/dep/test_match_from_list.py |  7 +--
 .../tests/resolver/ResolverPlayground.py  |  9 +--
 .../tests/unicode/test_string_format.py   |  7 +--
 lib/portage/update.py | 16 ++
 lib/portage/util/__init__.py  |  8 +--
 lib/portage/util/_dyn_libs/LinkageMapELF.py   |  6 +-
 .../util/_dyn_libs/PreservedLibsRegistry.py   |  7 +--
 lib/portage/util/_urlopen.py  |  9 +--
 lib/portage/util/changelog.py | 10 ++--
 lib/portage/util/compression_probe.py |  4 +-
 lib/portage/util/configparser.py  |  9 +--
 lib/portage/util/env_update.py|  7 +--
 lib/portage/util/install_mask.py  |  9 +--
 lib/portage/versions.py   | 17 ++
 lib/portage/xml/metadata.py   | 12 ++--
 repoman/lib/repoman/__init__.py   |  7 +--
 repoman/lib/repoman/main.py   |  2 -
 repoman/lib/repoman/metadata.py   |  5 --
 .../modules/scan/metadata/ebuild_metadata.py  |  5 +-
 repoman/lib/repoman/utilities.py  |  3 -
 69 files changed, 227 insertions(+), 490 deletions(-)

diff --git a/bin/egencache b/bin/egencache
index d172319f8..0847d73a7 100755
--- a/bin/egencache
+++ b/bin/egencache
@@ -75,9 +75,6 @@ else:
else:
from portage.xml.metadata import parse_metadata_use
 
-if sys.hexversion >= 0x300:
-   # pylint: disable=W0622
-   long = int
 
 def parse_args(args):
usage = "egencache [options]  ... [atom] ..."
@@ -344,7 +341,7 @@ class GenCache(object):
max_mtime = ec_hash.mtime
if max_mtime == sc.mtime:
max_mtime += 1
-   max_mtime = long(max_mtime)
+   max_mtime = int(max_mtime)
try:
os.utime(ebuild_hash.location, 
(max_mtime, max_mtime))
e

[gentoo-portage-dev] [PATCH] Remove unnecessary time.monotonic() compat

2020-07-14 Thread Michał Górny
time.monotonic() is available since py3.3, so there's no need for
the compat anymore.

Signed-off-by: Michał Górny 
---
 lib/portage/dbapi/vartree.py |  7 ++--
 lib/portage/tests/util/futures/test_retry.py |  8 ++---
 lib/portage/util/_eventloop/EventLoop.py |  6 ++--
 lib/portage/util/monotonic.py| 34 
 4 files changed, 10 insertions(+), 45 deletions(-)
 delete mode 100644 lib/portage/util/monotonic.py

diff --git a/lib/portage/dbapi/vartree.py b/lib/portage/dbapi/vartree.py
index 3687b471b..80ca0ab80 100644
--- a/lib/portage/dbapi/vartree.py
+++ b/lib/portage/dbapi/vartree.py
@@ -1,4 +1,4 @@
-# Copyright 1998-2019 Gentoo Authors
+# Copyright 1998-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__ import division, unicode_literals
@@ -36,7 +36,6 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.util.install_mask:install_mask_dir,InstallMask,_raise_exc',
'portage.util.listdir:dircache,listdir',
'portage.util.movefile:movefile',
-   'portage.util.monotonic:monotonic',
'portage.util.path:first_existing,iter_parents',
'portage.util.writeable_check:get_ro_checker',
'portage.util._xattr:xattr',
@@ -3616,7 +3615,7 @@ class dblink(object):
symlink_collisions = []
destroot = self.settings['ROOT']
totfiles = len(file_list) + len(symlink_list)
-   previous = monotonic()
+   previous = time.monotonic()
progress_shown = False
report_interval = 1.7  # seconds
falign = len("%d" % totfiles)
@@ -3625,7 +3624,7 @@ class dblink(object):
for i, (f, f_type) in enumerate(chain(
((f, "reg") for f in file_list),
((f, "sym") for f in symlink_list))):
-   current = monotonic()
+   current = time.monotonic()
if current - previous > report_interval:
showMessage(_("%3d%% done,  %*d files 
remaining ...\n") %
(i * 100 / totfiles, 
falign, totfiles - i))
diff --git a/lib/portage/tests/util/futures/test_retry.py 
b/lib/portage/tests/util/futures/test_retry.py
index 7a1e76280..4530bba83 100644
--- a/lib/portage/tests/util/futures/test_retry.py
+++ b/lib/portage/tests/util/futures/test_retry.py
@@ -1,4 +1,4 @@
-# Copyright 2018 Gentoo Foundation
+# Copyright 2018-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 try:
@@ -12,6 +12,7 @@ except ImportError:
import dummy_threading as threading
 
 import sys
+import time
 
 from portage.tests import TestCase
 from portage.util._eventloop.global_event_loop import global_event_loop
@@ -19,7 +20,6 @@ from portage.util.backoff import RandomExponentialBackoff
 from portage.util.futures import asyncio
 from portage.util.futures.retry import retry
 from portage.util.futures.executor.fork import ForkExecutor
-from portage.util.monotonic import monotonic
 
 
 class SucceedLaterException(Exception):
@@ -31,12 +31,12 @@ class SucceedLater(object):
A callable object that succeeds some duration of time has passed.
"""
def __init__(self, duration):
-   self._succeed_time = monotonic() + duration
+   self._succeed_time = time.monotonic() + duration
 
def __call__(self):
loop = global_event_loop()
result = loop.create_future()
-   remaining = self._succeed_time - monotonic()
+   remaining = self._succeed_time - time.monotonic()
if remaining > 0:
loop.call_soon_threadsafe(lambda: None if result.done() 
else
result.set_exception(SucceedLaterException(
diff --git a/lib/portage/util/_eventloop/EventLoop.py 
b/lib/portage/util/_eventloop/EventLoop.py
index ffd12cff9..a3bea97aa 100644
--- a/lib/portage/util/_eventloop/EventLoop.py
+++ b/lib/portage/util/_eventloop/EventLoop.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2018 Gentoo Foundation
+# Copyright 1999-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__ import division
@@ -11,6 +11,7 @@ import os
 import select
 import signal
 import sys
+import time
 import traceback
 
 try:
@@ -37,7 +38,6 @@ portage.proxy.lazyimport.lazyimport(globals(),
 )
 
 from portage.util import writemsg_level
-from portage.util.monotonic import monotonic
 from ..SlotObject import SlotObject
 from .PollConstants import PollConstants
 from .PollSelectAdapter import PollSelectAdapter
@@ -887,7 +887,7 @@ class EventLoop(

Re: [gentoo-portage-dev] [PATCH] travis.yml: drop python 2.7 (bug 731114)

2020-07-06 Thread Michał Górny
On Mon, 2020-07-06 at 11:42 -0700, Zac Medico wrote:
> It should be pretty safe to drop support for python2.7 at this point.
> 

We should probably also change the trove classifier to ... Python :: 3
:: Only

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] Speeding up Tree Verification

2020-07-01 Thread Michał Górny
On Tue, 2020-06-30 at 16:51 -0500, Sid Spry wrote:
> On Tue, Jun 30, 2020, at 2:29 PM, Michał Górny wrote:
> > On Tue, 2020-06-30 at 12:50 -0500, Sid Spry wrote:
> > > On Tue, Jun 30, 2020, at 2:28 AM, Michał Górny wrote:
> > > > Dnia June 30, 2020 2:13:43 AM UTC, Sid Spry  napisał(a):
> > > > > Hello,
> > > > > 
> > > > > I have some runnable pseudocode outlining a faster tree verification
> > > > > algorithm.
> > > > > Before I create patches I'd like to see if there is any guidance on
> > > > > making the
> > > > > changes as unobtrusive as possible. If the radical change in algorithm
> > > > > is
> > > > > acceptable I can work on adding the changes.
> > > > > 
> > > > > Instead of composing any kind of structured data out of the portage
> > > > > tree my
> > > > > algorithm just lists all files and then optionally batches them out to
> > > > > threads.
> > > > > There is a noticeable speedup by eliding the tree traversal operations
> > > > > which
> > > > > can be seen when running the algorithm with a single thread and
> > > > > comparing it to
> > > > > the current algorithm in gemato (which should still be discussed
> > > > > here?).
> > > > 
> > > > Without reading the code: does your algorithm correctly detect 
> > > > extraneous files?
> > > > 
> > > 
> > > Yes and no.
> > > 
> > > I am not sure why this is necessary. If the file does not appear in a 
> > > manifest it is
> > > ignored. It makes the most sense to me to put the burden of not including
> > > untracked files on the publisher. If the user puts an untracked file into 
> > > the tree it
> > > will be ignored to no consequence; the authored files don't refer to it, 
> > > after all.
> > 
> > This is necessary because a malicious third party can MITM you an rsync
> > tree with extraneous files (say, -r1 baselayout ebuild) that do horrible
> > things on your system.  If you don't reject files not in Manifest, you
> > open a huge security hole.
> > 
> 
> Ok, I will refer to https://www.gentoo.org/glep/glep-0074.html and implement 
> the
> checks in detail, but will still need to spend some time looking for the best 
> place
> to insert the code.
> 
> I think it best to address this from two fronts. On one hand rejecting extra 
> files
> seems to have immediate benefit but the larger issue is portage exposing
> untracked potentially malicious files to the user.

I've pushed two branches with two exploits I could think of.  Please
test your tools against them.  In both cases, the directory should be
rejected without any ill effects.

https://github.com/mgorny/glep74-test-cases

> 
> Has anything like a verity loopback filesystem been explored? It might reduce
> duplication of work.

I don't understand what a 'verity loopback filesystem' is.  Could you elaborate?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] Speeding up Tree Verification

2020-06-30 Thread Michał Górny
On Tue, 2020-06-30 at 12:50 -0500, Sid Spry wrote:
> On Tue, Jun 30, 2020, at 2:28 AM, Michał Górny wrote:
> > Dnia June 30, 2020 2:13:43 AM UTC, Sid Spry  napisał(a):
> > > Hello,
> > > 
> > > I have some runnable pseudocode outlining a faster tree verification
> > > algorithm.
> > > Before I create patches I'd like to see if there is any guidance on
> > > making the
> > > changes as unobtrusive as possible. If the radical change in algorithm
> > > is
> > > acceptable I can work on adding the changes.
> > > 
> > > Instead of composing any kind of structured data out of the portage
> > > tree my
> > > algorithm just lists all files and then optionally batches them out to
> > > threads.
> > > There is a noticeable speedup by eliding the tree traversal operations
> > > which
> > > can be seen when running the algorithm with a single thread and
> > > comparing it to
> > > the current algorithm in gemato (which should still be discussed
> > > here?).
> > 
> > Without reading the code: does your algorithm correctly detect extraneous 
> > files?
> > 
> 
> Yes and no.
> 
> I am not sure why this is necessary. If the file does not appear in a 
> manifest it is
> ignored. It makes the most sense to me to put the burden of not including
> untracked files on the publisher. If the user puts an untracked file into the 
> tree it
> will be ignored to no consequence; the authored files don't refer to it, 
> after all.

This is necessary because a malicious third party can MITM you an rsync
tree with extraneous files (say, -r1 baselayout ebuild) that do horrible
things on your system.  If you don't reject files not in Manifest, you
open a huge security hole.

> But it would be easy enough to build a second list of all files and compare 
> it to
> the list of files built from the manifests. If there are extras an error can 
> be
> generated. This is actually the first test I did on my manifest parsing code. 
> I tried
> to see if my tracked files roughly matched the total files in tree. That can 
> be
> repurposed for this check.
> 
> > > Some simple tests like counting all objects traversed and verified
> > > returns the
> > > same(ish). Once it is put into portage it could be tested in detail.
> > > 
> > > There is also my partial attempt at removing the brittle interface to
> > > GnuPG
> > > (it's not as if the current code is badly designed, just that parsing
> > > the
> > > output of GnuPG directly is likely not the best idea).
> > 
> > The 'brittle interface' is well-defined machine-readable output.
> > 
> 
> Ok. I was aware there was a machine interface, but the classes that manipulate
> a temporary GPG home seemed like not the best solution. I guess that is all
> due to GPG assuming everything is in ~/.gnupg and keeping its state as a
> directory structure.

A temporary home directory guarantees that user configuration does not
affect the verification result.

> 
> > > Needs gemato, dnspython, and requests. Slightly better than random code
> > > because
> > > I took inspiration from the existing gemato classes.
> > 
> > The code makes a lot of brittle assumptions about the structure. The 
> > GLEP was specifically designed to avoid that and let us adjust the 
> > structure in the future to meet our needs.
> > 
> 
> These same assumptions are built into the code that operates on the
> tree structure. If the GLEP were changed the existing code would also
> potentially need changing. This code just uses the structure in a different
> way.
> 

The code that predates the GLEP, yes.  It will eventually be changed to
be more flexible, especially when we can assume that we start removing
backwards compatibility.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] Speeding up Tree Verification

2020-06-30 Thread Michał Górny
>> glsa.build_manifest_list()
>> glsa.parse_manifests()
>> 
>> start = timeit.default_timer()
>> glsa.verify_manifests()
>> end = timeit.default_timer()
>> pprint(end - start)
>> 
>> # Handled by checking for None.
>> #no_ignore = filter(lambda x: x._tag != 'ignore',
>glsa_manifest_results)
>> 
>> 
>> #pprint(glsa._manifest_results)
>> real_files = [x for x in filter(lambda x: x is not None,
>glsa._manifest_results)]
>> #pprint(real_files)
>> pprint(len(glsa._manifest_results))
>> pprint(len(real_files))
>> 
>> all_files = []
>> for path, dirs, files in os.walk('/var/db/repos/gentoo'):
>> pass
>> 
>> return 0
>> 
>> if __name__ == '__main__':
>> sys.exit(main())
>> ```
>> 
>> ```python (wkd.py, likely unneeded but I didn't want to redo these
>files yet)
>> #!/usr/bin/env python3
>> import sys, hashlib
>> import dns
>> from dns import (
>> name, query, dnssec,
>> message, resolver, rdatatype
>> )
>> import shutil, requests
>> 
>> def check_domain_signature(domain: str) -> bool:
>> response = dns.resolver.query(domain, dns.rdatatype.NS)
>> nsname = response.rrset[0]
>> response = dns.resolver.query(str(nsname), dns.rdatatype.A)
>> nsaddr = response.rrset[0].to_text()
>> 
>> # DNSKEY
>> request = dns.message.make_query(domain,
>> dns.rdatatype.DNSKEY, want_dnssec=True)
>> response = dns.query.udp(request, nsaddr)
>> if response.rcode() != 0:
>> raise Exception('Unable to request dnskey.')
>> 
>> answer = response.answer
>> if len(answer) != 2:
>> raise Exception('Malformed answer to dnskey query.')
>> 
>> name = dns.name.from_text(domain)
>> try:
>> dns.dnssec.validate(answer[0], answer[1], {name: answer[0]})
>> except dns.dnssec.ValidationFailure as exc:
>> # Validation failed. The raise causes python to abort with
>status 1.
>> #raise exc
>> return False
>> except AttributeError as exc:
>> # Validation may have failed; DNSKEY missing signer
>attribute. dig may report
>> # domain as valid.
>> #
>> # TODO: Additional state where subdomain of valid domain may
>fail with 3 nested
>> # KeyErrors. Avoid temptation to wildcard catch. Safer to put
>in process?
>> #raise exc
>> return False
>> else:
>> return True
>> 
>> def hash_localpart(incoming: bytes) -> str:
>> '''Z-base32 the localpart of an e-mail address
>> 
>>
>https://tools.ietf.org/html/draft-koch-openpgp-webkey-service-08#section-3.1
>> describes why this is needed.
>> 
>> See https://tools.ietf.org/html/rfc6189#section-5.1.6 for a
>> description of the z-base32 scheme.
>> '''
>> zb32 = "ybndrfg8ejkmcpqxot1uwisza345h769"
>> 
>> b = hashlib.sha1(incoming).digest()
>> ret = ""
>> assert(len(b) * 8 == 160)
>> for i in range(0, 160, 5):
>> byte = i // 8
>> offset = i - byte * 8
>> # offset | bits remaining in k+1 | right-shift k+1
>> # 3 | 0 | x
>> # 4 | 1 | 7
>> # 5 | 2 | 6
>> # 6 | 3 | 5
>> # 7 | 4 | 4
>> if offset < 4:
>> n = (b[byte] >> (3 - offset))
>> else:
>> n = (b[byte] << (offset - 3)) + (b[byte + 1] >> (11 -
>offset))
>> 
>> ret += zb32[n & 0b1]
>> return ret
>> 
>> def build_web_key_uri(address: str) -> str:
>> local, remote = address.split('@')
>> local = hash_localpart(local.encode('utf-8'))
>> return 'https://' + remote + '/.well-known/openpgpkey/hu/' + \
>> local
>> 
>> def stream_to_file(uri: str, fname: str) -> None:
>> with requests.get(uri, verify=True, stream=True) as r:
>> from pprint import pprint
>> pprint(r.headers)
>> with open(fname, 'wb') as f:
>> shutil.copyfileobj(r.raw, f)
>> ```
>> 


--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] Speeding up Tree Verification

2020-06-30 Thread Michał Górny
est, nsaddr)
>if response.rcode() != 0:
>raise Exception('Unable to request dnskey.')
>
>answer = response.answer
>if len(answer) != 2:
>raise Exception('Malformed answer to dnskey query.')
>
>name = dns.name.from_text(domain)
>try:
>dns.dnssec.validate(answer[0], answer[1], {name: answer[0]})
>except dns.dnssec.ValidationFailure as exc:
>   # Validation failed. The raise causes python to abort with status 1.
>#raise exc
>return False
>except AttributeError as exc:
># Validation may have failed; DNSKEY missing signer attribute. dig may
>report
># domain as valid.
>#
># TODO: Additional state where subdomain of valid domain may fail with
>3 nested
># KeyErrors. Avoid temptation to wildcard catch. Safer to put in
>process?
>#raise exc
>return False
>else:
>return True
>
>def hash_localpart(incoming: bytes) -> str:
>'''Z-base32 the localpart of an e-mail address
>
>https://tools.ietf.org/html/draft-koch-openpgp-webkey-service-08#section-3.1
>describes why this is needed.
>
>See https://tools.ietf.org/html/rfc6189#section-5.1.6 for a
>description of the z-base32 scheme.
>'''
>zb32 = "ybndrfg8ejkmcpqxot1uwisza345h769"
>
>b = hashlib.sha1(incoming).digest()
>ret = ""
>assert(len(b) * 8 == 160)
>for i in range(0, 160, 5):
>byte = i // 8
>offset = i - byte * 8
># offset | bits remaining in k+1 | right-shift k+1
># 3 | 0 | x
># 4 | 1 | 7
># 5 | 2 | 6
># 6 | 3 | 5
># 7 | 4 | 4
>if offset < 4:
>n = (b[byte] >> (3 - offset))
>else:
> n = (b[byte] << (offset - 3)) + (b[byte + 1] >> (11 - offset))
>
>ret += zb32[n & 0b1]
>return ret
>
>def build_web_key_uri(address: str) -> str:
>local, remote = address.split('@')
>local = hash_localpart(local.encode('utf-8'))
>return 'https://' + remote + '/.well-known/openpgpkey/hu/' + \
>local
>
>def stream_to_file(uri: str, fname: str) -> None:
>with requests.get(uri, verify=True, stream=True) as r:
>from pprint import pprint
>pprint(r.headers)
>with open(fname, 'wb') as f:
>shutil.copyfileobj(r.raw, f)
>```


--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] Add caching to a few commonly used functions

2020-06-27 Thread Michał Górny
Dnia June 28, 2020 3:42:33 AM UTC, Zac Medico  napisał(a):
>On 6/27/20 8:12 PM, Michał Górny wrote:
>> Dnia June 28, 2020 3:00:00 AM UTC, Zac Medico 
>napisał(a):
>>> On 6/26/20 11:34 PM, Chun-Yu Shei wrote:
>>>> Hi,
>>>>
>>>> I was recently interested in whether portage could be speed up,
>since
>>>> dependency resolution can sometimes take a while on slower
>machines.
>>>> After generating some flame graphs with cProfile and vmprof, I
>found
>>> 3
>>>> functions which seem to be called extremely frequently with the
>same
>>>> arguments: catpkgsplit, use_reduce, and match_from_list.  In the
>>> first
>>>> two cases, it was simple to cache the results in dicts, while
>>>> match_from_list was a bit trickier, since it seems to be a
>>> requirement
>>>> that it return actual entries from the input "candidate_list".  I
>>> also
>>>> ran into some test failures if I did the caching after the
>>>> mydep.unevaluated_atom.use and mydep.repo checks towards the end of
>>> the
>>>> function, so the caching is only done up to just before that point.
>>>>
>>>> The catpkgsplit change seems to definitely be safe, and I'm pretty
>>> sure
>>>> the use_reduce one is too, since anything that could possibly
>change
>>> the
>>>> result is hashed.  I'm a bit less certain about the match_from_list
>>> one,
>>>> although all tests are passing.
>>>>
>>>> With all 3 patches together, "emerge -uDvpU --with-bdeps=y @world"
>>>> speeds up from 43.53 seconds to 30.96 sec -- a 40.6% speedup. 
>>> "emerge
>>>> -ep @world" is just a tiny bit faster, going from 18.69 to 18.22
>sec
>>>> (2.5% improvement).  Since the upgrade case is far more common,
>this
>>>> would really help in daily use, and it shaves about 30 seconds off
>>>> the time you have to wait to get to the [Yes/No] prompt (from ~90s
>to
>>>> 60s) on my old Sandy Bridge laptop when performing normal upgrades.
>>>>
>>>> Hopefully, at least some of these patches can be incorporated, and
>>> please
>>>> let me know if any changes are necessary.
>>>>
>>>> Thanks,
>>>> Chun-Yu
>>>
>>> Using global variables for caches like these causes a form of memory
>>> leak for use cases involving long-running processes that need to
>work
>>> with many different repositories (and perhaps multiple versions of
>>> those
>>> repositories).
>>>
>>> There are at least a couple of different strategies that we can use
>to
>>> avoid this form of memory leak:
>>>
>>> 1) Limit the scope of the caches so that they have some sort of
>garbage
>>> collection life cycle. For example, it would be natural for the
>>> depgraph
>>> class to have a local cache of use_reduce results, so that the cache
>>> can
>>> be garbage collected along with the depgraph.
>>>
>>> 2) Eliminate redundant calls. For example, redundant calls to
>>> catpkgslit
>>> can be avoided by constructing more _pkg_str instances, since
>>> catpkgsplit is able to return early when its argument happens to be
>a
>>> _pkg_str instance.
>> 
>> I think the weak stuff from the standard library might also be
>helpful.
>> 
>> --
>> Best regards, 
>> Michał Górny
>> 
>
>Hmm, maybe weak global caches are an option?

It would probably be necessary to add hit/miss counter and compare results 
before and after.

--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] Add caching to a few commonly used functions

2020-06-27 Thread Michał Górny
Dnia June 28, 2020 3:00:00 AM UTC, Zac Medico  napisał(a):
>On 6/26/20 11:34 PM, Chun-Yu Shei wrote:
>> Hi,
>> 
>> I was recently interested in whether portage could be speed up, since
>> dependency resolution can sometimes take a while on slower machines.
>> After generating some flame graphs with cProfile and vmprof, I found
>3
>> functions which seem to be called extremely frequently with the same
>> arguments: catpkgsplit, use_reduce, and match_from_list.  In the
>first
>> two cases, it was simple to cache the results in dicts, while
>> match_from_list was a bit trickier, since it seems to be a
>requirement
>> that it return actual entries from the input "candidate_list".  I
>also
>> ran into some test failures if I did the caching after the
>> mydep.unevaluated_atom.use and mydep.repo checks towards the end of
>the
>> function, so the caching is only done up to just before that point.
>> 
>> The catpkgsplit change seems to definitely be safe, and I'm pretty
>sure
>> the use_reduce one is too, since anything that could possibly change
>the
>> result is hashed.  I'm a bit less certain about the match_from_list
>one,
>> although all tests are passing.
>> 
>> With all 3 patches together, "emerge -uDvpU --with-bdeps=y @world"
>> speeds up from 43.53 seconds to 30.96 sec -- a 40.6% speedup. 
>"emerge
>> -ep @world" is just a tiny bit faster, going from 18.69 to 18.22 sec
>> (2.5% improvement).  Since the upgrade case is far more common, this
>> would really help in daily use, and it shaves about 30 seconds off
>> the time you have to wait to get to the [Yes/No] prompt (from ~90s to
>> 60s) on my old Sandy Bridge laptop when performing normal upgrades.
>> 
>> Hopefully, at least some of these patches can be incorporated, and
>please
>> let me know if any changes are necessary.
>> 
>> Thanks,
>> Chun-Yu
>
>Using global variables for caches like these causes a form of memory
>leak for use cases involving long-running processes that need to work
>with many different repositories (and perhaps multiple versions of
>those
>repositories).
>
>There are at least a couple of different strategies that we can use to
>avoid this form of memory leak:
>
>1) Limit the scope of the caches so that they have some sort of garbage
>collection life cycle. For example, it would be natural for the
>depgraph
>class to have a local cache of use_reduce results, so that the cache
>can
>be garbage collected along with the depgraph.
>
>2) Eliminate redundant calls. For example, redundant calls to
>catpkgslit
>can be avoided by constructing more _pkg_str instances, since
>catpkgsplit is able to return early when its argument happens to be a
>_pkg_str instance.

I think the weak stuff from the standard library might also be helpful.

--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] [PATCH 1/3] Add caching to catpkgsplit function

2020-06-27 Thread Michał Górny
Dnia June 27, 2020 6:34:13 AM UTC, Chun-Yu Shei  napisał(a):
>According to cProfile, catpkgsplit is called up to 1-5.5 million times
>during "emerge -uDvpU --with-bdeps=y @world". Adding a dict to cache
>its
>results reduces the time for this command from 43.53 -> 41.53 seconds
>--
>a 4.8% speedup.


Not saying caching is wrong for an interim solution but this is the kind of 
function where refactoring may yield even more gain.


>---
> lib/portage/versions.py | 7 +++
> 1 file changed, 7 insertions(+)
>
>diff --git a/lib/portage/versions.py b/lib/portage/versions.py
>index 0c21373cc..ffec316ce 100644
>--- a/lib/portage/versions.py
>+++ b/lib/portage/versions.py
>@@ -312,6 +312,7 @@ def _pkgsplit(mypkg, eapi=None):
> 
> _cat_re = re.compile('^%s$' % _cat, re.UNICODE)
> _missing_cat = 'null'
>+_catpkgsplit_cache = {}
> 
> def catpkgsplit(mydata, silent=1, eapi=None):
>   """
>@@ -331,6 +332,11 @@ def catpkgsplit(mydata, silent=1, eapi=None):
>   return mydata.cpv_split
>   except AttributeError:
>   pass
>+
>+  cache_entry = _catpkgsplit_cache.get(mydata)
>+  if cache_entry is not None:
>+  return cache_entry
>+
>   mysplit = mydata.split('/', 1)
>   p_split = None
>   if len(mysplit) == 1:
>@@ -343,6 +349,7 @@ def catpkgsplit(mydata, silent=1, eapi=None):
>   if not p_split:
>   return None
>   retval = (cat, p_split[0], p_split[1], p_split[2])
>+  _catpkgsplit_cache[mydata] = retval
>   return retval
> 
> class _pkg_str(_unicode):


--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] [PATCH] Use env to find python

2020-06-17 Thread Michał Górny
On Wed, 2020-06-17 at 10:18 +0200, Ulrich Mueller wrote:
> > > > > > On Tue, 16 Jun 2020, Michał Górny wrote:
> > '/usr/bin/env python' (with no extra options) is the portable shebang.
> 
> I still think this is a gross hack. You want your script to use the
> correct interpreter (which is in /usr/bin, or the path substituted for
> it at install time), not some random binary which happens to be found
> in the user's ${PATH}.
> 

How do you presume what the correct interpreter is, and why do you
presume that users put 'random binaries' in their PATH?  What if
the user needs to override 'correct interpreter'?  Requiring people to
overwrite executables in /usr/bin sounds like a 'gross hack'.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] erroneous behavior in 2-style USE dependencies?

2020-06-17 Thread Michał Górny
On Tue, 2020-06-16 at 23:07 +, Michael Lienhardt wrote:
> Dear all,
> 
> My bad for not noticing it sooner, but when there is a dependency like 
> ">=sys-fs/udev-208-r1:0/0[static-libs?]" (that occurs in 
> virtual/libgudev-215-r3),
>  since 'static-libs' is not a use flags of sys-fs/udev-242, that cpv is 
> silently not considered during dependency solving by emerge.
> However, the PMS states:
>  - it is an error for a use dependency to be applied to an ebuild which does 
> not have the flag in question in IUSE_REFERENCEABLE
> 

This is a bit like undefined behavior.  PMS says such a thing shouldn't
happen (i.e. the ebuild is wrong) but does not force specific error
handling.  You could reject the ebuild entirely or reject dependencies
that don't have the flag in question (even if it's disabled).  You could
also pretend it's 'static-libs(-)?' but that would be bad if the default
was supposed to be '(+)'.

> This is related to the tool I'm working on: should my tool allow this 
> behavior, or fail like it is currently doing (I guess the former)?
> 

That depends on what the tool is doing.  I suppose you probably don't
need very strict behavior there.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Use env to find python

2020-06-16 Thread Michał Górny
On Tue, 2020-06-16 at 15:19 -0400, Mike Gilbert wrote:
> On Tue, Jun 16, 2020 at 1:55 PM Zac Medico  wrote:
> > On 6/16/20 10:46 AM, Mike Gilbert wrote:
> > > On Tue, Jun 16, 2020 at 1:45 PM Mike Gilbert  wrote:
> > > > On Mon, Jun 15, 2020 at 9:39 AM Sid Spry  wrote:
> > > > > On Mon, Jun 15, 2020, at 2:36 AM, Ulrich Mueller wrote:
> > > > > > But we know that it is in /usr/bin, so why add yet another 
> > > > > > indirection?
> > > > > > 
> > > > > > Attachments:
> > > > > > * signature.asc
> > > > > 
> > > > > Ah, sorry -- I forgot to note this here. If you wish to support 
> > > > > prefix it is possible it may not be in /usr/bin. Granted I am not 
> > > > > sure if the prefix stage3 I was using is old enough to be broken in 
> > > > > some way, but adding this would prevent future breakage.
> > > > 
> > > > The portage ebuild and the python distutils module already take care
> > > > of updating shebangs at install time.
> > > 
> > > I suppose your patch might be useful if you are trying to run portage
> > > from a git checkout on a prefix system.
> > > 
> > 
> > So, given that the ebuild updates shebangs automatically, should't we
> > optimize the default shebangs to be as flexible as possible?
> 
> Yes, that makes sense.
> 
> However, we should test to make sure that distutils is smart enough to
> parse that "/usr/bin/env -S python" string and replace it with
> version-specific python shebang.
> 

'/usr/bin/env python' (with no extra options) is the portable shebang.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] config.environ: delay export of A and AA (bug 720180)

2020-05-26 Thread Michał Górny
On Mon, 2020-05-25 at 21:31 -0700, Zac Medico wrote:
> Since variables like A and AA can contain extremely large values which
> may trigger E2BIG errors during attempts to execute subprocesses, delay
> export until the last moment, and unexport when appropriate.
> 

Please don't.  This will only hide the problem from developers who will
unknowingly commit broken ebuilds and cause users of alternative package
managers to suffer.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] process: Provide libc fallback for sethostname() on PyPy

2020-05-17 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 lib/portage/process.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/portage/process.py b/lib/portage/process.py
index 79052b608..ceb454030 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -715,7 +715,14 @@ def _exec(binary, mycommand, opt_name, fd_pipes,
if unshare_net:
# use 'localhost' to 
avoid hostname resolution problems
try:
-   
socket.sethostname('localhost')
+   # pypy3 does 
not implement socket.sethostname()
+   new_hostname = 
b'localhost'
+   if 
hasattr(socket, 'sethostname'):
+   
socket.sethostname(new_hostname)
+   else:
+   if 
libc.sethostname(new_hostname, len(new_hostname)) != 0:
+   
errno_value = ctypes.get_errno()
+   
raise OSError(errno_value, os.strerror(errno_value))
except Exception as e:

writemsg("Unable to set hostname: %s (for FEATURES=\"network-sandbox\")\n" % (
e,),
-- 
2.26.2




Re: [gentoo-portage-dev] [PATCH] Enable FEATURES=parallel-install by default (bug 715110)

2020-05-13 Thread Michał Górny
On Tue, 2020-05-12 at 13:18 -0700, Zac Medico wrote:
> On 5/12/20 12:28 PM, Michał Górny wrote:
> > W dniu wto, 12.05.2020 o godzinie 10∶05 -0700, użytkownik Zac Medico
> > napisał:
> > > On 5/12/20 3:39 AM, Michał Górny wrote:
> > > > I'm sorry but I was asking of a more high-level implications.
> > > > 
> > > > I presume that this means that more than files of more than one
> > > > package
> > > > can be merged simultaneously.  However:
> > > 
> > > No, an exclusive lock must be held on vardbapi._fs_lock for this.
> > > This
> > > is currently required at least to guarantee that access to the config
> > > memory file is serialized (config memory is the thing that emerge
> > > --noconfmem disables, but --noconfmem does not disable this lock).
> > > 
> > > We assume that it's probably not worthwhile to try to merge files for
> > > more than one package at a time, since that would cause them to
> > > compete
> > > for IO bandwidth.
> > > 
> > > > 1. Are collisions handled correctly then?  i.e. if you start
> > > > installing
> > > > A, and then B, and the two packages collide will portage fail
> > > > before
> > > > starting to install any file from B?
> > > 
> > > There are no guarantees here. However, the risk is minimal, since
> > > it's
> > > unlikely that a file collision of this sort would occur. file
> > > collisions
> > > are a QA problem that is generally detected and corrected log before
> > > we
> > > would encounter a collision of this sort.
> > > 
> > > > 2. Are preinst/postinst phases called simultaneously or serialized?
> > > 
> > > They're serialized.
> > 
> > Now I'm lost here.  Could you try to explain to me, without getting
> > into the deep technicalities, how parallel-install achieves better
> > speed or at doing what is non-parallel-install so slow?
> 
> It allows preinst/postinst/prerm/postrm phases to run for one package
> while files are concurrently being merged or unmerged for another
> package. This makes it possible to approach saturation of IO bandwidth.

Doesn't this imply that programs run in these phases could fail if
Portage is simultaneously replacing their files?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Enable FEATURES=parallel-install by default (bug 715110)

2020-05-12 Thread Michał Górny
W dniu wto, 12.05.2020 o godzinie 10∶05 -0700, użytkownik Zac Medico
napisał:
> On 5/12/20 3:39 AM, Michał Górny wrote:
> > I'm sorry but I was asking of a more high-level implications.
> > 
> > I presume that this means that more than files of more than one
> > package
> > can be merged simultaneously.  However:
> 
> No, an exclusive lock must be held on vardbapi._fs_lock for this.
> This
> is currently required at least to guarantee that access to the config
> memory file is serialized (config memory is the thing that emerge
> --noconfmem disables, but --noconfmem does not disable this lock).
> 
> We assume that it's probably not worthwhile to try to merge files for
> more than one package at a time, since that would cause them to
> compete
> for IO bandwidth.
> 
> > 1. Are collisions handled correctly then?  i.e. if you start
> > installing
> > A, and then B, and the two packages collide will portage fail
> > before
> > starting to install any file from B?
> 
> There are no guarantees here. However, the risk is minimal, since
> it's
> unlikely that a file collision of this sort would occur. file
> collisions
> are a QA problem that is generally detected and corrected log before
> we
> would encounter a collision of this sort.
> 
> > 2. Are preinst/postinst phases called simultaneously or serialized?
> 
> They're serialized.

Now I'm lost here.  Could you try to explain to me, without getting
into the deep technicalities, how parallel-install achieves better
speed or at doing what is non-parallel-install so slow?

-- 
Best regards,
Michał Górny





Re: [gentoo-portage-dev] [PATCH] Enable FEATURES=parallel-install by default (bug 715110)

2020-05-12 Thread Michał Górny
W dniu wto, 12.05.2020 o godzinie 01∶40 -0700, użytkownik Zac Medico
napisał:
> On 5/11/20 10:54 PM, Michał Górny wrote:
> > W dniu nie, 10.05.2020 o godzinie 21∶32 -0700, użytkownik Zac
> > Medico
> > napisał:
> > > The feature enables finer grained locks for install operations,
> > > and
> > > everyone agrees that it's safe to enable by default.
> > 
> > Who's 'everyone' and where's their analysis of the problem?
> > The manpage doesn't really help understand what this does, exactly.
> 
> Before parallel-install there was just one big lock, so only one
> package
> slot could enter the merge/unmerge state at a given time.
> 
> With parallel install, there are a few finer-grained locks that come
> into play. [...]

I'm sorry but I was asking of a more high-level implications.

I presume that this means that more than files of more than one package
can be merged simultaneously.  However:

1. Are collisions handled correctly then?  i.e. if you start installing
A, and then B, and the two packages collide will portage fail before
starting to install any file from B?

2. Are preinst/postinst phases called simultaneously or serialized?

-- 
Best regards,
Michał Górny





Re: [gentoo-portage-dev] [PATCH] Enable FEATURES=parallel-install by default (bug 715110)

2020-05-11 Thread Michał Górny
W dniu nie, 10.05.2020 o godzinie 21∶32 -0700, użytkownik Zac Medico
napisał:
> The feature enables finer grained locks for install operations, and
> everyone agrees that it's safe to enable by default.

Who's 'everyone' and where's their analysis of the problem?
The manpage doesn't really help understand what this does, exactly.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Change BINPKG_COMPRESS default from bzip2 to xz

2020-04-27 Thread Michał Górny
On Sun, 2020-04-26 at 18:23 -0400, Michael Orlitzky wrote:
> On 4/26/20 3:25 PM, Ulrich Mueller wrote:
> > > > > > > On Sun, 26 Apr 2020, Michael Orlitzky wrote:
> > > Fuel for the fire:
> > >  * https://www.nongnu.org/lzip/lzip_benchmark.html
> > >  * https://www.nongnu.org/lzip/xz_inadequate.html
> > 
> > Yep. That's why lzip is the dominant compression format now, and nobody
> > is using xz any more.
> > 
> 
> If you believed that argument, you would be using Windows =P
> 
> Dude has GRAPHS, it must be true.
> 

It is true but nobody cares.  People managed to shove a lot of unused
code into the kernel and nobody cares.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Change BINPKG_COMPRESS default from bzip2 to xz

2020-04-26 Thread Michał Górny
On Sun, 2020-04-26 at 09:55 -0700, Matt Turner wrote:
> Bug: https://bugs.gentoo.org/715108
> Signed-off-by: Matt Turner 
> ---
> Strawman patch. Bikeshed away.
> 

xz is generally slow and doesn't do parallel good.  If we want to change
this, we should go for something cool like zstd that scales better.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] Need backup mentor for FUSE-based sandbox project

2020-04-26 Thread Michał Górny
On Sat, 2020-04-25 at 21:40 -0700, Alec Warner wrote:
> On Thu, Apr 23, 2020 at 12:09 PM Michał Górny  wrote:
> 
> > Hi, everyone.
> > 
> > It seems that we *urgently* (read: in 6 days) need to find backup
> > mentors for this year's GSoC projects.  I'm mentoring the project to
> > develop a FUSE-based sandbox alternative that's going to work reliably
> > with more packages than the LD_PRELOAD hack [1].
> > 
> > Would anyone volunteer to be the backup maintainer for this?
> > 
> > [1]
> > https://summerofcode.withgoogle.com/dashboard/organization/5749981018849280/proposal/5572241732927488/
> 
> I get a 403 for that.
> 
> https://wiki.gentoo.org/wiki/Google_Summer_of_Code/2020/Ideas#FUSE-powered_sandbox
> is
> your writeup; I assume the above is supposed to be a link to the student's
> proposal?

Yes and yes.  I suppose idea of stable links didn't reach Google yet. 
However, I should have noted that we have a backup mentor now but
there's no harm in having a second backup ;-).

> 
> 
> > 
> > --
> > Best regards,
> > Michał Górny
> > 
> > 

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] Need backup mentor for FUSE-based sandbox project

2020-04-23 Thread Michał Górny
Hi, everyone.

It seems that we *urgently* (read: in 6 days) need to find backup
mentors for this year's GSoC projects.  I'm mentoring the project to
develop a FUSE-based sandbox alternative that's going to work reliably
with more packages than the LD_PRELOAD hack [1].

Would anyone volunteer to be the backup maintainer for this?

[1] 
https://summerofcode.withgoogle.com/dashboard/organization/5749981018849280/proposal/5572241732927488/

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH v2] process: Unshare UTS namespace, and set hostname to 'localhost'

2020-03-28 Thread Michał Górny
On Sat, 2020-03-28 at 11:16 -0700, Zac Medico wrote:
> On 3/28/20 12:13 AM, Michał Górny wrote:
> > Use UTS namespace to override hostname when network-sandbox is enabled.
> > Set it to 'localhost' as that has a better chance of being present
> > in /etc/hosts.  This fixes tests in some packages that try to connect
> > to localhost via hostname obtained using gethostname(), e.g. docker-py,
> > and suffer resolution problems due to the system hostname not being
> > defined in /etc/hosts.
> > ---
> >  lib/portage/process.py | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/portage/process.py b/lib/portage/process.py
> > index c1fc4bcf6..c48503208 100644
> > --- a/lib/portage/process.py
> > +++ b/lib/portage/process.py
> > @@ -348,12 +348,14 @@ def spawn(mycommand, env=None, opt_name=None, 
> > fd_pipes=None, returnpid=False,
> > if unshare_net or unshare_ipc or unshare_mount or unshare_pid:
> > # from /usr/include/bits/sched.h
> > CLONE_NEWNS = 0x0002
> > +   CLONE_NEWUTS = 0x0400
> > CLONE_NEWIPC = 0x0800
> > CLONE_NEWPID = 0x2000
> > CLONE_NEWNET = 0x4000
> >  
> > if unshare_net:
> > -   unshare_flags |= CLONE_NEWNET
> > +   # UTS namespace to override hostname
> > +   unshare_flags |= CLONE_NEWNET | CLONE_NEWUTS
> > if unshare_ipc:
> > unshare_flags |= CLONE_NEWIPC
> > if unshare_mount:
> > @@ -704,6 +706,13 @@ def _exec(binary, mycommand, opt_name, fd_pipes,
> > 
> > noiselevel=-1)
> > os._exit(1)
> > if unshare_net:
> > +   # use 'localhost' to 
> > avoid hostname resolution problems
> > +   try:
> > +   
> > socket.sethostname('localhost')
> > +   except Exception as e:
> > +   
> > writemsg("Unable to set hostname: %s (for FEATURES=\"network-sandbox\")\n" 
> > % (
> > +   e),
> 
> Existing code uses (e,) in cases like this, in order to wrap the
> exception in a tuple, preventing ambiguity in python2 where exceptions
> may behave like tuples. If you don't include the comma, then the
> parenthesis do nothing here, but these days the string formatting
> appears to work correctly with python2.7 either way.

Oops, I meant to leave it there.  I'll fix and push.

> 
> > +       
> > noiselevel=-1)
> > 
> > _configure_loopback_interface()
> > except AttributeError:
> > # unshare() not supported by libc
> > 
> 
> Looks good. Please merge.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH v2] process: Unshare UTS namespace, and set hostname to 'localhost'

2020-03-28 Thread Michał Górny
Use UTS namespace to override hostname when network-sandbox is enabled.
Set it to 'localhost' as that has a better chance of being present
in /etc/hosts.  This fixes tests in some packages that try to connect
to localhost via hostname obtained using gethostname(), e.g. docker-py,
and suffer resolution problems due to the system hostname not being
defined in /etc/hosts.
---
 lib/portage/process.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/portage/process.py b/lib/portage/process.py
index c1fc4bcf6..c48503208 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -348,12 +348,14 @@ def spawn(mycommand, env=None, opt_name=None, 
fd_pipes=None, returnpid=False,
if unshare_net or unshare_ipc or unshare_mount or unshare_pid:
# from /usr/include/bits/sched.h
CLONE_NEWNS = 0x0002
+   CLONE_NEWUTS = 0x0400
CLONE_NEWIPC = 0x0800
CLONE_NEWPID = 0x2000
CLONE_NEWNET = 0x4000
 
if unshare_net:
-   unshare_flags |= CLONE_NEWNET
+   # UTS namespace to override hostname
+   unshare_flags |= CLONE_NEWNET | CLONE_NEWUTS
if unshare_ipc:
unshare_flags |= CLONE_NEWIPC
if unshare_mount:
@@ -704,6 +706,13 @@ def _exec(binary, mycommand, opt_name, fd_pipes,

noiselevel=-1)
os._exit(1)
if unshare_net:
+   # use 'localhost' to 
avoid hostname resolution problems
+   try:
+   
socket.sethostname('localhost')
+   except Exception as e:
+   
writemsg("Unable to set hostname: %s (for FEATURES=\"network-sandbox\")\n" % (
+   e),
+   
noiselevel=-1)

_configure_loopback_interface()
except AttributeError:
# unshare() not supported by libc
-- 
2.26.0




[gentoo-portage-dev] [PATCH] process: Unshare UTS namespace, and set hostname to 'localhost'

2020-03-27 Thread Michał Górny
Use UTS namespace to override hostname when network-sandbox is enabled.
Set it to 'localhost' as that has a better chance of being present
in /etc/hosts.  This fixes tests in some packages that try to connect
to localhost via hostname obtained using gethostname(), e.g. docker-py,
and suffer resolution problems due to the system hostname not being
defined in /etc/hosts.
---
 lib/portage/process.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/portage/process.py b/lib/portage/process.py
index c1fc4bcf6..0f9789acb 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -348,12 +348,14 @@ def spawn(mycommand, env=None, opt_name=None, 
fd_pipes=None, returnpid=False,
if unshare_net or unshare_ipc or unshare_mount or unshare_pid:
# from /usr/include/bits/sched.h
CLONE_NEWNS = 0x0002
+   CLONE_NEWUTS = 0x0400
CLONE_NEWIPC = 0x0800
CLONE_NEWPID = 0x2000
CLONE_NEWNET = 0x4000
 
if unshare_net:
-   unshare_flags |= CLONE_NEWNET
+   # UTS namespace to override hostname
+   unshare_flags |= CLONE_NEWNET | CLONE_NEWUTS
if unshare_ipc:
unshare_flags |= CLONE_NEWIPC
if unshare_mount:
@@ -704,6 +706,8 @@ def _exec(binary, mycommand, opt_name, fd_pipes,

noiselevel=-1)
os._exit(1)
if unshare_net:
+   # use 'localhost' to 
avoid hostname resolution problems
+   
socket.sethostname('localhost')

_configure_loopback_interface()
except AttributeError:
# unshare() not supported by libc
-- 
2.26.0




Re: [gentoo-portage-dev] [PATCH] repoman: remove check for addpredict

2020-03-05 Thread Michał Górny
On Thu, 2020-03-05 at 15:30 -0500, Mike Gilbert wrote:
> This function has not been deprecated, and developers generally have a
> good reason for using it. A repoman warning for this is just noise.
> 
> No reason was given when this check was added in 2010, and it was requested
> by a developer who has since retired.
> 

If you want to remove this class of warnings, I'd suggest removing
the one for -j1 as well.  Same argument.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] repoman: Restore eclass deprecations lost in rewrite

2020-02-26 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 repoman/lib/repoman/modules/linechecks/deprecated/inherit.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py 
b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
index 9cef086da..f307d46fd 100644
--- a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
+++ b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
@@ -11,6 +11,8 @@ class InheritDeprecated(LineCheck):
 
# deprecated eclass : new eclass (False if no new eclass)
deprecated_eclasses = {
+   "autotools-multilib": "multilib-minimal",
+   "autotools-utils": False,
"base": False,
"bash-completion": "bash-completion-r1",
"boost-utils": False,
@@ -18,8 +20,10 @@ class InheritDeprecated(LineCheck):
"confutils": False,
"distutils": "distutils-r1",
"epatch": "(eapply since EAPI 6)",
+   "fdo-mime": "xdg-utils",
"games": False,
"gems": "ruby-fakegem",
+   "git-2": "git-r3",
"gpe": False,
"gst-plugins-bad": "gstreamer",
"gst-plugins-base": "gstreamer",
@@ -33,6 +37,7 @@ class InheritDeprecated(LineCheck):
"user": "GLEP 81",
"versionator": "eapi7-ver (built-in since EAPI 7)",
"x-modular": "xorg-2",
+   "xfconf": False,
}
 
_inherit_re = re.compile(r'^\s*inherit\s(.*)$')
-- 
2.25.1




[gentoo-portage-dev] [PATCH] Clean up remnants of eapi5-hdepend, HDEPEND and targetroot

2019-12-26 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 bin/eapi.sh   | 46 +--
 bin/ebuild.sh | 23 +++---
 bin/phase-functions.sh|  4 +-
 doc/package/ebuild.docbook|  1 -
 doc/package/ebuild/eapi/5-hdepend.docbook | 32 -
 doc/portage.docbook   |  1 -
 lib/_emerge/Package.py|  6 +--
 lib/_emerge/actions.py|  1 -
 lib/_emerge/depgraph.py   | 12 +
 lib/portage/__init__.py   |  2 +-
 lib/portage/cache/metadata.py |  2 +-
 lib/portage/dbapi/bintree.py  |  7 ++-
 lib/portage/dbapi/porttree.py |  2 +-
 lib/portage/dbapi/vartree.py  |  2 +-
 lib/portage/dep/_slot_operator.py |  3 --
 lib/portage/eapi.py   | 18 ++--
 .../ebuild/_config/special_env_vars.py|  2 +-
 lib/portage/package/ebuild/config.py  | 13 +-
 man/emerge.1  | 10 ++--
 repoman/cnf/repository/linechecks.yaml|  1 -
 .../modules/linechecks/patches/patches.py |  2 +-
 21 files changed, 53 insertions(+), 137 deletions(-)
 delete mode 100644 doc/package/ebuild/eapi/5-hdepend.docbook

diff --git a/bin/eapi.sh b/bin/eapi.sh
index 4eafc1c87..29dfb008c 100644
--- a/bin/eapi.sh
+++ b/bin/eapi.sh
@@ -31,19 +31,15 @@ ___eapi_has_prefix_variables() {
 }
 
 ___eapi_has_BROOT() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ 
]]
 }
 
 ___eapi_has_SYSROOT() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
-}
-
-___eapi_has_HDEPEND() {
-   [[ ${1-${EAPI-0}} =~ ^(5-hdepend)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ 
]]
 }
 
 ___eapi_has_BDEPEND() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ 
]]
 }
 
 ___eapi_has_RDEPEND_DEPEND_fallback() {
@@ -65,11 +61,11 @@ ___eapi_has_dosed() {
 }
 
 ___eapi_has_einstall() {
-   [[ ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_dohtml() {
-   [[ ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
+   [[ ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ ]]
 }
 
 ___eapi_has_dohtml_deprecated() {
@@ -77,7 +73,7 @@ ___eapi_has_dohtml_deprecated() {
 }
 
 ___eapi_has_dolib_libopts() {
-   [[ ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
+   [[ ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ ]]
 }
 
 ___eapi_has_docompress() {
@@ -85,7 +81,7 @@ ___eapi_has_docompress() {
 }
 
 ___eapi_has_dostrip() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress|6)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress|6)$ 
]]
 }
 
 ___eapi_has_nonfatal() {
@@ -101,23 +97,23 @@ ___eapi_has_usex() {
 }
 
 ___eapi_has_get_libdir() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_einstalldocs() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_eapply() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_eapply_user() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_in_iuse() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_has_version_functions() {
@@ -167,7 +163,7 @@ ___eapi_unpack_supports_xz() {
 }
 
 ___eapi_unpack_supports_txz() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0}} =~ ^(0|1|2|3|4|4-python|4-slot-abi|5|5-progress)$ ]]
 }
 
 ___eapi_econf_passes_--disable-dependency-tracking() {
@@ -179,7 +175,7 @@ ___eapi_econf_passes_--disable-silent-rules() {
 }
 
 ___eapi_econf_passes_--docdir_and_--htmldir() {
-   [[ ! ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+   [[ ! ${1-${EAPI-0

Re: [gentoo-portage-dev] Re: RFC: [QA] notice with 'failed' seds [was PATCH: eapply drop -s option]

2019-12-16 Thread Michał Górny
On Sun, 2019-12-15 at 13:29 -0800, Zac Medico wrote:
> On 12/13/19 2:12 PM, Michael 'veremitz' Everitt wrote:
> > On 13/12/19 20:36, Michał Górny wrote [excerpted]:
> > > Is this really an argument for or *against* it?  Developers are entirely
> > > capable of keeping seds that do nothing for years, as well as patches
> > > that -- while apparently applying correctly -- are entirely meaningless.
> > 
> > 
> > I think there is some merit in some kind of feedback when sed's are doing
> > nothing, although how feasible it is to generate any useful feedback I
> > can't say. I wouldn't say it needs to explicitly fail or make lots of
> > noise, just an info message that could prompt some further investigation.
> > 
> 
> It's possible to implement a sed wrapper that detects file arguments for
> -i/--in-place mode, and compares file content before and after the sed call.
> 
> There are also ways to make sed exit with an error but that won't be as
> easy to use as a sed wrapper:
> 
> https://stackoverflow.com/questions/15965073/return-code-of-sed-for-no-match/15966279

Don't forget that there could be valid cases for sed not changing
a file.  Not to mention corner cases where a working replacement results
in no change, e.g.:

  # yuck!
  sed -i -e "s^-O2^${CFLAGS}^" ...

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH v2] eapply: Output verbosely only if patch fails to apply with -F0

2019-12-14 Thread Michał Górny
On Fri, 2019-12-13 at 23:49 +0100, Ulrich Mueller wrote:
> > > > > > On Fri, 13 Dec 2019, Mike Gilbert wrote:
> > > > It also triggers pointless bug reports. Please remove this.
> > > 
> > > I don't like that eqawarn either (see above).
> > > 
> > > OTOH, users shouldn't normally have "qa" in PORTAGE_ELOG_CLASSES,
> > > so they won't see the warning?
> > Here's a bug report filed by a user, which is what prompted me to
> > reply on this thread in the first place.
> > https://bugs.gentoo.org/702608
> 
> Well then, trivial patch included below.
> 
> 
> From 81000b32d330a1cc41a4541f7e4264918eb7e6c5 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ulrich=20M=C3=BCller?= 
> Date: Fri, 13 Dec 2019 23:41:23 +0100
> Subject: [PATCH] eapply: Drop QA warning for fuzz factor.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This didn't add any information beyond what is already present in the
> output of patch. Developers will know how to interpret its output, and
> users won't see the warning anyway with the standard configuration.
> 
> Signed-off-by: Ulrich Müller 
> ---
>  bin/phase-helpers.sh | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
> index b5691bd70..020862ba0 100644
> --- a/bin/phase-helpers.sh
> +++ b/bin/phase-helpers.sh
> @@ -1004,8 +1004,6 @@ if ___eapi_has_eapply; then
>   if ${patch_cmd} "${all_opts[@]}" --dry-run -s -F0 \
>   < "${f}" &>/dev/null; then
>   all_opts+=( -s -F0 )
> - else
> - eqawarn "${f}: patch failed to apply 
> without a fuzz factor, please rebase"
>   fi
>  
>   ${patch_cmd} "${all_opts[@]}" < "${f}"

Actually, I added that because of your comment that people should be
rebasing patches rather than removing context.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-14 Thread Michał Górny
On Fri, 2019-12-13 at 17:15 -0500, Mike Gilbert wrote:
> On Fri, Dec 13, 2019 at 4:42 PM Michał Górny  wrote:
> > On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
> > > On Fri, Dec 13, 2019 at 3:36 PM Michał Górny  wrote:
> > > > Just like 'many of the proposals lately', developers are going to be
> > > > the ones disabling it (because they don't care), and users will be the
> > > > ones enabling it (because they do care), just to learn that developers
> > > > don't care and go complaining to the mailing lists that users dare
> > > > report issues they don't care about.
> > > 
> > > I care if the patch is actually broken, which the warning doesn't
> > > really tell me. It's just not a very reliable indicator, and will
> > > produce false-positives frequently.
> > > 
> > 
> > You can also take less context into the patch and use -F0.  Then you'll
> > have the same effect, no warnings to bother you and no pretending that
> > the patch applies when it doesn't.
> 
> That really doesn't help me. My point is that I don't want to touch
> the patch unless it is actually necessary to do so.
> 

Then make patches with -U0.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-13 Thread Michał Górny
On Fri, 2019-12-13 at 21:56 +, Michael 'veremitz' Everitt wrote:
> On 13/12/19 21:42, Michał Górny wrote:
> > On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
> > > On Fri, Dec 13, 2019 at 3:36 PM Michał Górny  wrote:
> > > > Just like 'many of the proposals lately', developers are going to be
> > > > the ones disabling it (because they don't care), and users will be the
> > > > ones enabling it (because they do care), just to learn that developers
> > > > don't care and go complaining to the mailing lists that users dare
> > > > report issues they don't care about.
> > > I care if the patch is actually broken, which the warning doesn't
> > > really tell me. It's just not a very reliable indicator, and will
> > > produce false-positives frequently.
> > > 
> > You can also take less context into the patch and use -F0.  Then you'll
> > have the same effect, no warnings to bother you and no pretending that
> > the patch applies when it doesn't.
> > 
> Is there any mileage in having a similar scheme to which we already apply
> '-p' increments to the -F variable?
> eg.
> 1) attempt patch with -F0
> 2) if (1) fails, attempt with -F2/3 & display 'yellow' warning & QA notice

That is precisely what my patch does and what people are complaining
about.

> 3) if (2) fails, attempt with, say, -F10 & display big fat 'red' warning
> and QA notice

That makes no sense as it exceeds context provided in most patches.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-13 Thread Michał Górny
On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
> On Fri, Dec 13, 2019 at 3:36 PM Michał Górny  wrote:
> > Just like 'many of the proposals lately', developers are going to be
> > the ones disabling it (because they don't care), and users will be the
> > ones enabling it (because they do care), just to learn that developers
> > don't care and go complaining to the mailing lists that users dare
> > report issues they don't care about.
> 
> I care if the patch is actually broken, which the warning doesn't
> really tell me. It's just not a very reliable indicator, and will
> produce false-positives frequently.
> 

You can also take less context into the patch and use -F0.  Then you'll
have the same effect, no warnings to bother you and no pretending that
the patch applies when it doesn't.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-13 Thread Michał Górny
On Fri, 2019-12-13 at 21:31 +0100, Fabian Groffen wrote:
> On 13-12-2019 14:24:33 -0500, Michael Orlitzky wrote:
> > On 12/13/19 9:28 AM, Fabian Groffen wrote:
> > > We are providing those patches, maybe.  In reality very often the
> > > patches originate from somewhere else though.  And you don't want to
> > > have to respin all of those just because.  At least that's what I feel.
> > > 
> > 
> > Just because... the context changed? A new "!" in a line of context can
> > be the difference between letting someone log in with the right password
> > and letting them log in with the wrong password. You should at least
> > have to stop and verify that the patch does what you think it does when
> > it "gains" fuzz. And at that point, git diff will give you a clean
> > version of it.
> 
> Counter argument is that we've been doing this for decades, and always
> relied on maintainers to check the contents of their patches, without
> problems.  We didn't introduce a predictable random number generator or
> something.

Is this really an argument for or *against* it?  Developers are entirely
capable of keeping seds that do nothing for years, as well as patches
that -- while apparently applying correctly -- are entirely meaningless.
Should I remind you that epatch was entirely capable of creating
meaningless files by randomly applying the wrong patch level?

> Your very specific example just illustrates the niche this proposal is
> targetting.
> 
> As with many of the proposals lately, they just seem to aim at more work
> for individual maintainers, with a very low gain ratio.
> Better, allow this to be a FEATURE, or whatever, that devs should enable,
> but don't spit this in the user's face by default.
> 

Just like 'many of the proposals lately', developers are going to be
the ones disabling it (because they don't care), and users will be the
ones enabling it (because they do care), just to learn that developers
don't care and go complaining to the mailing lists that users dare
report issues they don't care about.


-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-13 Thread Michał Górny
On Fri, 2019-12-13 at 09:06 -0500, Mike Gilbert wrote:
> On Fri, Dec 13, 2019 at 8:52 AM Michał Górny  wrote:
> > On Fri, 2019-12-13 at 08:47 -0500, Mike Gilbert wrote:
> > > On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller  wrote:
> > > > > > > > > On Thu, 12 Dec 2019, Mike Gilbert wrote:
> > > > > I think this should be reverted. It causes too much noise, and
> > > > > "solves" a problem only very rarely.
> > > > 
> > > > Now, how many lines of output does this typically produce, compared
> > > > to the total size of a typical build log? Especially with mgorny's
> > > > subsequent modification, which suppresses the output unless the patch
> > > > doesn't apply cleanly.
> > > 
> > > In most cases, I would be inclined to simply ignore the patch output
> > > since there's really no need for me to take any action on it.
> > > 
> > > On the other hand, it makes it more difficult to quickly identify the
> > > list of patches being applied if there is junk output in the middle of
> > > the list.
> > > 
> > > > It was also suggested that we add -F0 in EAPI 8, but that would break
> > > > the build in those cases that are producing extra output now. I don't
> > > > think that would be preferable.
> > > 
> > > I am opposed to including such a change in EAPI 8. It would make
> > > ebuild maintenance more difficult for everyone, and I don't think the
> > > potential benefit is worth it.
> > 
> > ...and why do we consider it correct to apply patches when the context
> > doesn't match?  If our only goal is to make things 'easier' for
> > 'everyone', then we could just pass -F and ignore all the context.
> > 
> > Though I don't understand why include any context in the first place if
> > you don't care about it matching.  Sounds like a waste of space to me!
> 
> The patch command defaults to -F2. If that makes no sense, why is it
> the upstream default?
> 

You should ask upstream, not me.  But if I were to guess, the answer
would be because patch(1) is used by random people trying to apply
random patches they've found somewhere.  We on the other hand are
applying patches that *we* are supposed to provide.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] eapply: Drop -s option for patch.

2019-12-13 Thread Michał Górny
On Fri, 2019-12-13 at 08:47 -0500, Mike Gilbert wrote:
> On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller  wrote:
> > > > > > > On Thu, 12 Dec 2019, Mike Gilbert wrote:
> > > I think this should be reverted. It causes too much noise, and
> > > "solves" a problem only very rarely.
> > 
> > Now, how many lines of output does this typically produce, compared
> > to the total size of a typical build log? Especially with mgorny's
> > subsequent modification, which suppresses the output unless the patch
> > doesn't apply cleanly.
> 
> In most cases, I would be inclined to simply ignore the patch output
> since there's really no need for me to take any action on it.
> 
> On the other hand, it makes it more difficult to quickly identify the
> list of patches being applied if there is junk output in the middle of
> the list.
> 
> > It was also suggested that we add -F0 in EAPI 8, but that would break
> > the build in those cases that are producing extra output now. I don't
> > think that would be preferable.
> 
> I am opposed to including such a change in EAPI 8. It would make
> ebuild maintenance more difficult for everyone, and I don't think the
> potential benefit is worth it.

...and why do we consider it correct to apply patches when the context
doesn't match?  If our only goal is to make things 'easier' for
'everyone', then we could just pass -F and ignore all the context.

Though I don't understand why include any context in the first place if
you don't care about it matching.  Sounds like a waste of space to me!

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH v2] eapply: Output verbosely only if patch fails to apply with -F0

2019-11-27 Thread Michał Górny
On Wed, 2019-11-27 at 23:49 +0100, Ulrich Mueller wrote:
> > > > > > On Wed, 27 Nov 2019, Michael Orlitzky wrote:
> > This now disagrees with the PMS algorithm, doesn't it?
> 
> The difference is that there is now a QA warning for something that is
> perfectly within the spec. Maybe the extra verboseness would be enough,
> but the eqawarn line should be omitted? It doesn't provide any info that
> isn't already present in the output of patch itself.
> 

It helps people understand why some patches throw a wall of text while
others don't.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH v2] eapply: Output verbosely only if patch fails to apply with -F0

2019-11-27 Thread Michał Górny
12d0c48ad disabled silent output for eapply, in order to obtain fuzz
factors in build logs.  However, this also causes eapply to report all
patched files which can make logs unreadable when there are no fuzz
factors to be reported.  Instead, use verbose output only when applying
the patch with -F0 fails.

To achieve that, attempt to apply each patch with -F0 --dry-run first.
If this succeeds, just silently apply the patch for real.  If it
doesn't, output an explicit eqawarn that the patch does not apply
cleanly and retry with the default fuzz factor and verbose output.
Non-silenced output applies both to successful application with fuzz
and to failure.

Signed-off-by: Michał Górny 
---
 bin/phase-helpers.sh | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Changes in v2:
- added original path to output

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index 60f8d3243..b5691bd70 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -995,8 +995,20 @@ if ___eapi_has_eapply; then
# -f to avoid interactivity
# -g0 to guarantee no VCS interaction
# --no-backup-if-mismatch not to pollute the sources
-   ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
-   "${patch_options[@]}" < "${f}"
+   local all_opts=(
+   -p1 -f -g0 --no-backup-if-mismatch
+   "${patch_options[@]}"
+   )
+   # try applying with -F0 first, output a verbose warning
+   # if fuzz factor is necessary
+   if ${patch_cmd} "${all_opts[@]}" --dry-run -s -F0 \
+   < "${f}" &>/dev/null; then
+   all_opts+=( -s -F0 )
+   else
+   eqawarn "${f}: patch failed to apply 
without a fuzz factor, please rebase"
+   fi
+
+   ${patch_cmd} "${all_opts[@]}" < "${f}"
failed=${?}
if ! eend "${failed}"; then
__helpers_die "patch -p1 ${patch_options[*]} 
failed with ${f}"
-- 
2.24.0




Re: [gentoo-portage-dev] [PATCH] eapply: Output verbosely only if patch fails to apply with -F0

2019-11-27 Thread Michał Górny
On Wed, 2019-11-27 at 11:13 -0800, Zac Medico wrote:
> On 11/27/19 10:54 AM, Michał Górny wrote:
> > 12d0c48ad disabled silent output for eapply, in order to obtain fuzz
> > factors in build logs.  However, this also causes eapply to report all
> > patched files which can make logs unreadable when there are no fuzz
> > factors to be reported.  Instead, use verbose output only when applying
> > the patch with -F0 fails.
> > 
> > To achieve that, attempt to apply each patch with -F0 --dry-run first.
> > If this succeeds, just silently apply the patch for real.  If it
> > doesn't, output an explicit eqawarn that the patch does not apply
> > cleanly and retry with the default fuzz factor and verbose output.
> > Non-silenced output applies both to successful application with fuzz
> > and to failure.
> > 
> > Signed-off-by: Michał Górny 
> > ---
> >  bin/phase-helpers.sh | 16 ++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
> > index 60f8d3243..6f9b52678 100644
> > --- a/bin/phase-helpers.sh
> > +++ b/bin/phase-helpers.sh
> > @@ -995,8 +995,20 @@ if ___eapi_has_eapply; then
> > # -f to avoid interactivity
> > # -g0 to guarantee no VCS interaction
> > # --no-backup-if-mismatch not to pollute the sources
> > -   ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
> > -   "${patch_options[@]}" < "${f}"
> > +   local all_opts=(
> > +   -p1 -f -g0 --no-backup-if-mismatch
> > +   "${patch_options[@]}"
> > +   )
> > +   # try applying with -F0 first, output a verbose warning
> > +   # if fuzz factor is necessary
> > +   if ${patch_cmd} "${all_opts[@]}" --dry-run -s -F0 \
> > +   < "${f}" &>/dev/null; then
> > +   all_opts+=( -s -F0 )
> > +   else
> > +   eqawarn "patch failed to apply without a 
> > fuzz factor, please rebase"
> 
> The earlier ebegin call only outputs the basename, so there could be
> some confusion about the origin of the patch. This function is also
> called by eapply_user, it's possible that people could get confused and
> report bugs about fuzz in their local patches. Other than that it looks
> good.
> 

Ok, I will add a full path.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] eapply: Output verbosely only if patch fails to apply with -F0

2019-11-27 Thread Michał Górny
12d0c48ad disabled silent output for eapply, in order to obtain fuzz
factors in build logs.  However, this also causes eapply to report all
patched files which can make logs unreadable when there are no fuzz
factors to be reported.  Instead, use verbose output only when applying
the patch with -F0 fails.

To achieve that, attempt to apply each patch with -F0 --dry-run first.
If this succeeds, just silently apply the patch for real.  If it
doesn't, output an explicit eqawarn that the patch does not apply
cleanly and retry with the default fuzz factor and verbose output.
Non-silenced output applies both to successful application with fuzz
and to failure.

Signed-off-by: Michał Górny 
---
 bin/phase-helpers.sh | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index 60f8d3243..6f9b52678 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -995,8 +995,20 @@ if ___eapi_has_eapply; then
# -f to avoid interactivity
# -g0 to guarantee no VCS interaction
# --no-backup-if-mismatch not to pollute the sources
-   ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
-   "${patch_options[@]}" < "${f}"
+   local all_opts=(
+   -p1 -f -g0 --no-backup-if-mismatch
+   "${patch_options[@]}"
+   )
+   # try applying with -F0 first, output a verbose warning
+   # if fuzz factor is necessary
+   if ${patch_cmd} "${all_opts[@]}" --dry-run -s -F0 \
+   < "${f}" &>/dev/null; then
+   all_opts+=( -s -F0 )
+   else
+   eqawarn "patch failed to apply without a 
fuzz factor, please rebase"
+   fi
+
+   ${patch_cmd} "${all_opts[@]}" < "${f}"
failed=${?}
if ! eend "${failed}"; then
__helpers_die "patch -p1 ${patch_options[*]} 
failed with ${f}"
-- 
2.24.0




Re: [gentoo-portage-dev] SLOT move of sys-libs/ncurses-5.9-r5(0/5) to sys-libs/ncurses-compat-5.9-r5(5/5) ?

2019-11-14 Thread Michał Górny
On Thu, 2019-11-14 at 13:01 +, Joakim Tjernlund wrote:
> Is there way to move the above with current portage ?
> 

No.  Slot moves work only within a package.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [r...@gentoo.org: Cron /usr/local/bin/pidlock -s sync-distfiles /usr/bin/timeout -k 2h 1h /usr/local/bin/mastermirror/sync-distfiles.sh]

2019-11-05 Thread Michał Górny
On Tue, 2019-11-05 at 18:13 +, Robin H. Johnson wrote:
> Can Portage handle this error more gracefully please?
> 

Is it still happening though?  I thought the Portage upgrade fixed it.

> The symlink test file is there to verify mirror behavior, so we don't
> want to delete it either.

I think it's fine to delete it now.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: remove check that bans libtool files and static libs from /

2019-11-04 Thread Michał Górny
On Mon, 2019-11-04 at 04:15 -0600, William Hubbs wrote:
> I also don't like your tone in your response to Zac merging the patch.
> 
> https://archives.gentoo.org/gentoo-portage-dev/message/1abfd0499e514b7d6b70b709e9e3ae18
> 
> If I say out here that since I'm a council member I'm above you and zac
> should listen to me and apply the patch is that appropriate? I imagine
> not, so I feel the same way about you bringing your qa membership into
> the discussion.
> In my opinion, all that kind of thing leads to is people becoming angry.
> 
> I'm going to ask you to close https://bugs.gentoo.org/699254. I honestly
> do not feel that this is an appropriate way to deal with this situation.
> 

Excuse me but are you serious?  So first you choose to claim that
something is not policy because you don't see it stamped.  Then you
demand that QA doesn't vote on stamping it because... why exactly? 
Because it's 'not an appropriate way', apparently.

So what's the appropriate way?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: remove check that bans libtool files and static libs from /

2019-11-03 Thread Michał Górny
On Sun, 2019-11-03 at 15:26 -0600, William Hubbs wrote:
> On Sun, Nov 03, 2019 at 12:18:40PM -0800, Zac Medico wrote:
> > On 11/3/19 11:53 AM, Michał Górny wrote:
> > > On Sun, 2019-11-03 at 11:49 -0800, Zac Medico wrote:
> > > > On 10/27/19 10:40 AM, William Hubbs wrote:
> > > > > Most upstreams and build systems do not make this distinction, so this
> > > > > causes unnecessary hacks in ebuilds.
> > > > > 
> > > > > Signed-off-by: William Hubbs 
> > > > > ---
> > > > >  bin/install-qa-check.d/80libraries | 10 --
> > > > >  1 file changed, 10 deletions(-)
> > > > > 
> > > > > diff --git a/bin/install-qa-check.d/80libraries 
> > > > > b/bin/install-qa-check.d/80libraries
> > > > > index d1d2c4fdd..e59369bf6 100644
> > > > > --- a/bin/install-qa-check.d/80libraries
> > > > > +++ b/bin/install-qa-check.d/80libraries
> > > > > @@ -152,16 +152,6 @@ lib_check() {
> > > > >   done
> > > > >   [[ ${abort} == "yes" ]] && die "add those ldscripts"
> > > > >  
> > > > > - # Make sure people don't store libtool files or static libs in 
> > > > > /lib
> > > > > - f=$(ls "${ED%/}"/lib*/*.{a,la} 2>/dev/null)
> > > > > - if [[ -n ${f} ]] ; then
> > > > > - __vecho -ne '\n'
> > > > > - eqawarn "QA Notice: Excessive files found in the / 
> > > > > partition"
> > > > > - eqawarn "${f}"
> > > > > - __vecho -ne '\n'
> > > > > - die "static archives (*.a) and libtool library files 
> > > > > (*.la) belong in /usr/lib*, not /lib*"
> > > > > - fi
> > > > > -
> > > > >   # Verify that the libtool files don't contain bogus $D entries.
> > > > >   local abort=no gentoo_bug=no always_overflow=no
> > > > >   for a in "${ED%/}"/usr/lib*/*.la ; do
> > > > > 
> > > > 
> > > > Merged. Thanks!
> > > > 
> > > > https://gitweb.gentoo.org/proj/portage.git/commit/?id=498900e5e51460502d8271f409a4c614a021613b
> > > > 
> > > 
> > > Please revert this.  I should point out that this has been vetoed
> > > by a QA member, and is currently subject to QA vote.  Therefore, I
> > > believe you shouldn't be making rash decisions based on patches
> > > submitted by a single developer.  Especially given that so far nobody
> > > else has voiced his opinion either way, so it's 1:1.
> 
> You being a qa member doesn't have a lot to do with this mgorny. you
> know there was no official policy when I posted this, and as far as I
> know there is not one now.
> 

That is a really poor argument.  Something that's respected for 10+
years and reported as QA violation is a standing policy as far as I'm
concerned.  Just because it isn't backed by a formally stamped policy
(at least as far as we know -- maybe it was actually stamped somewhere
in the past?) doesn't mean you it's fine for one person to change it ad-
hoc because it stands in his way.

I should point that I'm very concerned that you're pushing this forward
even though:

1) I've objected to the change itself,

2) I've pointed out that it's been sent to the wrong mailing list,
and that this explicitly prevents a number of developers from even
knowing that this is happening,

3) removing it provides a way for regressions that can have major impact
on users and that involve much effort in reverting that.

So if I send a revert patch afterwards, and you object, should the patch
be accepted because only one person objected?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: remove check that bans libtool files and static libs from /

2019-11-03 Thread Michał Górny
On Sun, 2019-11-03 at 11:49 -0800, Zac Medico wrote:
> On 10/27/19 10:40 AM, William Hubbs wrote:
> > Most upstreams and build systems do not make this distinction, so this
> > causes unnecessary hacks in ebuilds.
> > 
> > Signed-off-by: William Hubbs 
> > ---
> >  bin/install-qa-check.d/80libraries | 10 --
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/bin/install-qa-check.d/80libraries 
> > b/bin/install-qa-check.d/80libraries
> > index d1d2c4fdd..e59369bf6 100644
> > --- a/bin/install-qa-check.d/80libraries
> > +++ b/bin/install-qa-check.d/80libraries
> > @@ -152,16 +152,6 @@ lib_check() {
> > done
> > [[ ${abort} == "yes" ]] && die "add those ldscripts"
> >  
> > -   # Make sure people don't store libtool files or static libs in /lib
> > -   f=$(ls "${ED%/}"/lib*/*.{a,la} 2>/dev/null)
> > -   if [[ -n ${f} ]] ; then
> > -   __vecho -ne '\n'
> > -   eqawarn "QA Notice: Excessive files found in the / partition"
> > -   eqawarn "${f}"
> > -   __vecho -ne '\n'
> > -   die "static archives (*.a) and libtool library files (*.la) 
> > belong in /usr/lib*, not /lib*"
> > -   fi
> > -
> > # Verify that the libtool files don't contain bogus $D entries.
> > local abort=no gentoo_bug=no always_overflow=no
> > for a in "${ED%/}"/usr/lib*/*.la ; do
> > 
> 
> Merged. Thanks!
> 
> https://gitweb.gentoo.org/proj/portage.git/commit/?id=498900e5e51460502d8271f409a4c614a021613b
> 

Please revert this.  I should point out that this has been vetoed
by a QA member, and is currently subject to QA vote.  Therefore, I
believe you shouldn't be making rash decisions based on patches
submitted by a single developer.  Especially given that so far nobody
else has voiced his opinion either way, so it's 1:1.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: remove check that bans libtool files and static libs from /

2019-10-27 Thread Michał Górny
On Sun, 2019-10-27 at 13:49 -0500, William Hubbs wrote:
> On Sun, Oct 27, 2019 at 06:58:00PM +0100, Michał Górny wrote:
> > On Sun, 2019-10-27 at 12:40 -0500, William Hubbs wrote:
> > > Most upstreams and build systems do not make this distinction, so this
> > > causes unnecessary hacks in ebuilds.
> > > 
> > 
> > The hacks aren't 'unnecessary'.  There is a very good reason that files
> > that are used *purely at build time* don't land in /.  That reason is
> > disk space.  Even if people nowadays are forced to use initramfs with
> > separate /usr, it doesn't mean you should just let their rootfs fill up
> > with useless files.
> 
> The useless files argument really holds no water with me. We install
> many files on / that are useless in one situation or another.
> Some examples are logrotate files when logrotate isn't installed,

Do we really install logrotate files to /?

> systemd units for openrc systems and openrc init scripts for systemd
> systems.

Those files are at least needed in some scenarios.  You're talking about
files that are never used without /usr mounted.  And that can't be used
because they depend on headers in /usr/include.

> 
> Talk to me about useless files on / after we put all of these, and
> possibly others I can't think of,  behind use flags.
> 
> > Do you have any *real* argument?  Because 'unnecessary hack' is
> > basically your feeling of ebuild aesthetics.  My aesthetics is more
> > worried about useless clutter in /lib*.  FHS agrees with me, as you
> > yourself admitted yesterday.
> 
> Any downstream hack means that we are being lazy and not reporting the
> bug upstream and asking them to fix it.
> 
> This particular issue is not a big deal to any other distro and has
> never been. Shouldn't we try to get upstreams to do this if it is so
> important?
> 

Because 'any other distro' usually means a binary distro that moves
files around after installing.  And splits static libraries into
separate packages.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: remove check that bans libtool files and static libs from /

2019-10-27 Thread Michał Górny
On Sun, 2019-10-27 at 12:40 -0500, William Hubbs wrote:
> Most upstreams and build systems do not make this distinction, so this
> causes unnecessary hacks in ebuilds.
> 

The hacks aren't 'unnecessary'.  There is a very good reason that files
that are used *purely at build time* don't land in /.  That reason is
disk space.  Even if people nowadays are forced to use initramfs with
separate /usr, it doesn't mean you should just let their rootfs fill up
with useless files.

Do you have any *real* argument?  Because 'unnecessary hack' is
basically your feeling of ebuild aesthetics.  My aesthetics is more
worried about useless clutter in /lib*.  FHS agrees with me, as you
yourself admitted yesterday.

So why do you believe we should introduce this regression?  And why are
you trying to sneak it past most of the developers via gentoo-portage-
dev instead of gentoo-dev?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] make.globals: Change FETCHCOMMAND_RSYNC to --copy-links

2019-10-21 Thread Michał Górny
On Mon, 2019-10-21 at 17:25 +0200, Ulrich Mueller wrote:
> > > > > > On Mon, 21 Oct 2019, Michał Górny wrote:
> > This also removes most of the other options that are irrelevant or even
> > undesirable to distfile fetching, that is:
> > - '-r' since we always fetch a single file, so recursive operation is
> >   unnecessary
> > - '-p', '-o', '-g' since we want to apply our permissions and ownership
> >   for distfiles rather than copying the one from mirrors,
> > - '-D' since we do not expect any devices or specials in distfiles.
> 
> That certainly makes sense, but I don't see it in the actual patch?

Those were the options implied by '-a'.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] make.globals: Change FETCHCOMMAND_RSYNC to --copy-links

2019-10-21 Thread Michał Górny
Change FETCHCOMMAND_RSYNC to use '-Lt' over '-a'.  Notably, this
replaces --links with --copy-links option, i.e. makes rsync copy
underlying files when symlinks are met.  This is important since
we do not transfer symlink targets, therefore '-l' ends up creating
dangling symlinks.

This also removes most of the other options that are irrelevant or even
undesirable to distfile fetching, that is:

- '-r' since we always fetch a single file, so recursive operation is
  unnecessary
- '-p', '-o', '-g' since we want to apply our permissions and ownership
  for distfiles rather than copying the one from mirrors,
- '-D' since we do not expect any devices or specials in distfiles.

Copying timestamps is preserved in case it's helpful in determining
whether files need to be refetched.

Bug: https://bugs.gentoo.org/698046
Signed-off-by: Michał Górny 
---
 cnf/make.globals | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cnf/make.globals b/cnf/make.globals
index 9eeb7a01e..50511e812 100644
--- a/cnf/make.globals
+++ b/cnf/make.globals
@@ -38,8 +38,8 @@ PORTAGE_TMPDIR="/var/tmp"
 FETCHCOMMAND="wget -t 3 -T 60 --passive-ftp -O \"\${DISTDIR}/\${FILE}\" 
\"\${URI}\""
 RESUMECOMMAND="wget -c -t 3 -T 60 --passive-ftp -O \"\${DISTDIR}/\${FILE}\" 
\"\${URI}\""
 
-FETCHCOMMAND_RSYNC="rsync -avP \"\${URI}\" \"\${DISTDIR}/\${FILE}\""
-RESUMECOMMAND_RSYNC="rsync -avP \"\${URI}\" \"\${DISTDIR}/\${FILE}\""
+FETCHCOMMAND_RSYNC="rsync -LtvP \"\${URI}\" \"\${DISTDIR}/\${FILE}\""
+RESUMECOMMAND_RSYNC="rsync -LtvP \"\${URI}\" \"\${DISTDIR}/\${FILE}\""
 
 # NOTE: rsync will evaluate quotes embedded inside PORTAGE_SSH_OPTS
 FETCHCOMMAND_SSH="bash -c \"x=\\\${2#ssh://} ; host=\\\${x%%/*} ; 
port=\\\${host##*:} ; host=\\\${host%:*} ; [[ \\\${host} = \\\${port} ]] && 
port= ; exec rsync --rsh=\\\"ssh \\\${port:+-p\\\${port}} \\\${3}\\\" -avP 
\\\"\\\${host}:/\\\${x#*/}\\\" \\\"\\\$1\\\"\" rsync \"\${DISTDIR}/\${FILE}\" 
\"\${URI}\" \"\${PORTAGE_SSH_OPTS}\""
-- 
2.23.0




Re: [gentoo-portage-dev] [PATCH 1/2] fetch: Use real os.walk() to avoid unicode issues with Portage

2019-10-21 Thread Michał Górny
On Mon, 2019-10-21 at 02:10 -0700, Zac Medico wrote:
> On 10/21/19 1:43 AM, Michał Górny wrote:
> > Use real os.walk() when getting filenames for FlatLayout.  Unlike
> > the wrapped Portage module, it return str output for str path parameter,
> > so we don't have to recode it back and forth.
> > 
> > Signed-off-by: Michał Górny 
> > ---
> >  lib/portage/package/ebuild/fetch.py | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/portage/package/ebuild/fetch.py 
> > b/lib/portage/package/ebuild/fetch.py
> > index cedf12b19..be277f1a3 100644
> > --- a/lib/portage/package/ebuild/fetch.py
> > +++ b/lib/portage/package/ebuild/fetch.py
> > @@ -11,6 +11,7 @@ import io
> >  import itertools
> >  import json
> >  import logging
> > +import os as real_os
> >  import random
> >  import re
> >  import stat
> > @@ -270,7 +271,7 @@ class FlatLayout(object):
> > return filename
> >  
> > def get_filenames(self, distdir):
> > -   for dirpath, dirnames, filenames in os.walk(distdir,
> > +   for dirpath, dirnames, filenames in real_os.walk(distdir,
> > onerror=_raise_exc):
> > return iter(filenames)
> >  
> > 
> 
> The real_os.walk will trigger UnicodeEncodeError if distdir can't be
> encoded with sys.getfilesystemencoding(). It's an edge case, but
> generally I prefer to handle it.
> 
> We can continue to use portage.os for the os.walk call, and turn
> get_filenames into a generator method like this:
> 
> for filename in filenames:
> try:
> yield portage._unicode_decode(filename, errors='strict')
> except UnicodeDecodeError:
> # Ignore it. Distfiles names must have valid UTF8 encoding.
> pass

Since you've already written it, could you commit it?  I don't wish to
have my name on the implicit module overrides hackery I don't approve
of.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH 2/2] emirrordist: Pass path from DeletionIterator to DeletionTask

2019-10-21 Thread Michał Górny
Since DeletionIterator needs to stat the distfile and therefore find
one working path for it, pass it to DeletionTask instead of recomputing
it there.  This also fixes wrongly assuming that first layout will
always be correct.

Bug: https://bugs.gentoo.org/697890
Signed-off-by: Michał Górny 
---
 lib/portage/_emirrordist/DeletionIterator.py |  2 ++
 lib/portage/_emirrordist/DeletionTask.py | 14 +-
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/portage/_emirrordist/DeletionIterator.py 
b/lib/portage/_emirrordist/DeletionIterator.py
index 5c193911a..3cbff2c3a 100644
--- a/lib/portage/_emirrordist/DeletionIterator.py
+++ b/lib/portage/_emirrordist/DeletionIterator.py
@@ -72,6 +72,7 @@ class DeletionIterator(object):
 
yield DeletionTask(background=True,
distfile=filename,
+   distfile_path=path,
config=self._config)
 
else:
@@ -85,6 +86,7 @@ class DeletionIterator(object):
 
yield 
DeletionTask(background=True,
distfile=filename,
+   distfile_path=path,
config=self._config)
 
if deletion_db is not None:
diff --git a/lib/portage/_emirrordist/DeletionTask.py 
b/lib/portage/_emirrordist/DeletionTask.py
index a4bb29419..4e9c26ca2 100644
--- a/lib/portage/_emirrordist/DeletionTask.py
+++ b/lib/portage/_emirrordist/DeletionTask.py
@@ -10,14 +10,9 @@ from _emerge.CompositeTask import CompositeTask
 
 class DeletionTask(CompositeTask):
 
-   __slots__ = ('distfile', 'config')
+   __slots__ = ('distfile', 'distfile_path', 'config')
 
def _start(self):
-
-   distfile_path = os.path.join(
-   self.config.options.distfiles,
-   self.config.layouts[0].get_path(self.distfile))
-
if self.config.options.recycle_dir is not None:
recycle_path = os.path.join(
self.config.options.recycle_dir, self.distfile)
@@ -29,7 +24,8 @@ class DeletionTask(CompositeTask):
"distfiles to recycle") % self.distfile)
try:
# note: distfile_path can be a symlink 
here
-   
os.rename(os.path.realpath(distfile_path), recycle_path)
+   
os.rename(os.path.realpath(self.distfile_path),
+   recycle_path)
except OSError as e:
if e.errno != errno.EXDEV:
logging.error(("rename %s from 
distfiles to "
@@ -40,7 +36,7 @@ class DeletionTask(CompositeTask):
return
 
self._start_task(
-   FileCopier(src_path=distfile_path,
+   FileCopier(src_path=self.distfile_path,
dest_path=recycle_path,
background=False),
self._recycle_copier_exit)
@@ -55,7 +51,7 @@ class DeletionTask(CompositeTask):
logging.debug(("delete '%s' from "
"distfiles") % self.distfile)
try:
-   os.unlink(distfile_path)
+   os.unlink(self.distfile_path)
except OSError as e:
if e.errno not in (errno.ENOENT, errno.ESTALE):
logging.error("%s unlink failed in 
distfiles: %s" %
-- 
2.23.0




[gentoo-portage-dev] [PATCH 1/2] fetch: Use real os.walk() to avoid unicode issues with Portage

2019-10-21 Thread Michał Górny
Use real os.walk() when getting filenames for FlatLayout.  Unlike
the wrapped Portage module, it return str output for str path parameter,
so we don't have to recode it back and forth.

Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index cedf12b19..be277f1a3 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -11,6 +11,7 @@ import io
 import itertools
 import json
 import logging
+import os as real_os
 import random
 import re
 import stat
@@ -270,7 +271,7 @@ class FlatLayout(object):
return filename
 
def get_filenames(self, distdir):
-   for dirpath, dirnames, filenames in os.walk(distdir,
+   for dirpath, dirnames, filenames in real_os.walk(distdir,
onerror=_raise_exc):
return iter(filenames)
 
-- 
2.23.0




[gentoo-portage-dev] [PATCH v2] emirrordist: Clean dangling symlinks up

2019-10-21 Thread Michał Górny
Bug: https://bugs.gentoo.org/697906
Signed-off-by: Michał Górny 
---
 lib/portage/_emirrordist/DeletionIterator.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/portage/_emirrordist/DeletionIterator.py 
b/lib/portage/_emirrordist/DeletionIterator.py
index dab6eaea2..5c193911a 100644
--- a/lib/portage/_emirrordist/DeletionIterator.py
+++ b/lib/portage/_emirrordist/DeletionIterator.py
@@ -27,11 +27,16 @@ class DeletionIterator(object):
# require at least one successful stat()
exceptions = []
for layout in reversed(self._config.layouts):
+   path = os.path.join(distdir, 
layout.get_path(filename))
try:
-   st = os.stat(
-   os.path.join(distdir, 
layout.get_path(filename)))
+   st = os.stat(path)
except OSError as e:
-   exceptions.append(e)
+   # is it a dangling symlink?
+   try:
+   if os.path.islink(path):
+   os.unlink(path)
+   except OSError as e:
+   exceptions.append(e)
else:
if stat.S_ISREG(st.st_mode):
break
-- 
2.23.0




Re: [gentoo-portage-dev] [PATCH] emirrordist: Clean dangling symlinks up

2019-10-21 Thread Michał Górny
On Mon, 2019-10-21 at 00:44 -0700, Zac Medico wrote:
> On 10/20/19 3:23 AM, Michał Górny wrote:
> > Bug: https://bugs.gentoo.org/697906
> > Signed-off-by: Michał Górny 
> > ---
> >  lib/portage/_emirrordist/DeletionIterator.py | 21 +---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/portage/_emirrordist/DeletionIterator.py 
> > b/lib/portage/_emirrordist/DeletionIterator.py
> > index dab6eaea2..6bc0fd09f 100644
> > --- a/lib/portage/_emirrordist/DeletionIterator.py
> > +++ b/lib/portage/_emirrordist/DeletionIterator.py
> > @@ -27,16 +27,31 @@ class DeletionIterator(object):
> > # require at least one successful stat()
> > exceptions = []
> > for layout in reversed(self._config.layouts):
> > +   path = os.path.join(distdir, 
> > layout.get_path(filename))
> > try:
> > -   st = os.stat(
> > -   os.path.join(distdir, 
> > layout.get_path(filename)))
> > +   st = os.stat(path)
> > except OSError as e:
> > -   exceptions.append(e)
> > +   # is it a dangling symlink?
> > +   try:
> > +   if os.path.islink(path):
> > +   os.unlink(path)
> > +   except OSError as e:
> > +   exceptions.append(e)
> > else:
> > if stat.S_ISREG(st.st_mode):
> > break
> 
> How about if we remove the above break so that we can eliminate the
> lstat loop below?

Oops, I forgot to remove it from my patch.  The 'break' is fine since we
need this dangling symlink elimination only if none of the files exist. 
If they do, then symlink removal post distfile removal will take care of
them.

> 
> 
> > else:
> > if exceptions:
> > +   # check for dangling symlinks
> > +   for layout in self._config.layouts:
> > +   path = os.path.join(distdir, 
> > layout.get_path(filename))
> > +   try:
> > +   st = os.lstat(path)
> > +   if 
> > stat.S_ISLNK(st.st_mode):
> > +   os.unlink(path)
> > +   except OSError as e:
> > +   pass
> > +
> > logging.error("stat failed on '%s' in 
> > distfiles: %s\n" %
> > (filename, '; '.join(str(x) for 
> > x in exceptions)))
> > continue
> > 
> 
> Looks good except that it would be nice to eliminate the second loop.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] emirrordist: Clean dangling symlinks up

2019-10-20 Thread Michał Górny
Bug: https://bugs.gentoo.org/697906
Signed-off-by: Michał Górny 
---
 lib/portage/_emirrordist/DeletionIterator.py | 21 +---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/lib/portage/_emirrordist/DeletionIterator.py 
b/lib/portage/_emirrordist/DeletionIterator.py
index dab6eaea2..6bc0fd09f 100644
--- a/lib/portage/_emirrordist/DeletionIterator.py
+++ b/lib/portage/_emirrordist/DeletionIterator.py
@@ -27,16 +27,31 @@ class DeletionIterator(object):
# require at least one successful stat()
exceptions = []
for layout in reversed(self._config.layouts):
+   path = os.path.join(distdir, 
layout.get_path(filename))
try:
-   st = os.stat(
-   os.path.join(distdir, 
layout.get_path(filename)))
+   st = os.stat(path)
except OSError as e:
-   exceptions.append(e)
+   # is it a dangling symlink?
+   try:
+   if os.path.islink(path):
+   os.unlink(path)
+   except OSError as e:
+   exceptions.append(e)
else:
if stat.S_ISREG(st.st_mode):
break
else:
if exceptions:
+   # check for dangling symlinks
+   for layout in self._config.layouts:
+   path = os.path.join(distdir, 
layout.get_path(filename))
+   try:
+   st = os.lstat(path)
+   if 
stat.S_ISLNK(st.st_mode):
+   os.unlink(path)
+   except OSError as e:
+   pass
+
logging.error("stat failed on '%s' in 
distfiles: %s\n" %
(filename, '; '.join(str(x) for 
x in exceptions)))
continue
-- 
2.23.0




[gentoo-portage-dev] [PATCH] emirrordist: Report all stat() errors instead of first one

2019-10-20 Thread Michał Górny
When DeletionIterator fails, report all stat() errors.  Reporting
just the first one results in confusing logs, suggesting that one
of the location did not exist but the other existed and was removed.
---
 lib/portage/_emirrordist/DeletionIterator.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/portage/_emirrordist/DeletionIterator.py 
b/lib/portage/_emirrordist/DeletionIterator.py
index cd773b3c8..dab6eaea2 100644
--- a/lib/portage/_emirrordist/DeletionIterator.py
+++ b/lib/portage/_emirrordist/DeletionIterator.py
@@ -25,20 +25,20 @@ class DeletionIterator(object):
distfiles_set.update(layout.get_filenames(distdir))
for filename in distfiles_set:
# require at least one successful stat()
-   first_exception = None
+   exceptions = []
for layout in reversed(self._config.layouts):
try:
st = os.stat(
os.path.join(distdir, 
layout.get_path(filename)))
except OSError as e:
-   first_exception = e
+   exceptions.append(e)
else:
if stat.S_ISREG(st.st_mode):
break
else:
-   if first_exception is not None:
+   if exceptions:
logging.error("stat failed on '%s' in 
distfiles: %s\n" %
-   (filename, first_exception))
+   (filename, '; '.join(str(x) for 
x in exceptions)))
continue
 
if filename in file_owners:
-- 
2.23.0




[gentoo-portage-dev] [PATCH] fetch: Use distfile fetching method to get layout.conf

2019-10-18 Thread Michał Górny
Rewrite the layout.conf getter to reuse the standard fetch() method
rather than using urlopen().  While at it, fix negative cache elision
to apply to memory cache as well (and not get written to disk if next
mirror was fine).

Most importantly, this ensures that we respect FETCHCOMMAND while
fetching layout.conf, and so layout.conf is fetched the same way normal
distfiles are.  With some uncommon configurations, the previous disjoint
logic might have resulted in one of the fetches failing while the other
succeeded.

This also adds some nice verbosity.  If mirror connection takes a while,
the user sees that rather than having Portage wait silently.

Bug: https://bugs.gentoo.org/697566
Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 36 +++--
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index debab38a2..599c496ee 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -382,7 +382,7 @@ class MirrorLayoutConfig(object):
return ret
 
 
-def get_mirror_url(mirror_url, filename, cache_path=None):
+def get_mirror_url(mirror_url, filename, mysettings, cache_path=None):
"""
Get correct fetch URL for a given file, accounting for mirror
layout configuration.
@@ -408,23 +408,25 @@ def get_mirror_url(mirror_url, filename, cache_path=None):
if ts >= time.time() - 86400:
mirror_conf.deserialize(data)
else:
+   tmpfile = '.layout.conf.%d' % time.time()
try:
-   f = urlopen(mirror_url + '/distfiles/layout.conf')
-   try:
-   data = io.StringIO(f.read().decode('utf8'))
-   finally:
-   f.close()
-
-   mirror_conf.read_from_file(data)
+   if fetch({tmpfile: (mirror_url + 
'/distfiles/layout.conf',)},
+   mysettings, fetchonly=1, try_mirrors=0):
+   tmpfile = os.path.join(mysettings['DISTDIR'], 
tmpfile)
+   try:
+   mirror_conf.read_from_file(tmpfile)
+   finally:
+   os.unlink(tmpfile)
+   else:
+   raise IOError()
except (ConfigParserError, IOError, UnicodeDecodeError):
-   # Do not cache negative results.
-   cache_path = None
-
-   cache[mirror_url] = (time.time(), mirror_conf.serialize())
-   if cache_path is not None:
-   f = atomic_ofstream(cache_path, 'w')
-   json.dump(cache, f)
-   f.close()
+   pass
+   else:
+   cache[mirror_url] = (time.time(), 
mirror_conf.serialize())
+   if cache_path is not None:
+   f = atomic_ofstream(cache_path, 'w')
+   json.dump(cache, f)
+   f.close()
 
return (mirror_url + "/distfiles/" +

mirror_conf.get_best_supported_layout().get_path(filename))
@@ -619,7 +621,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
mirror_cache = None
for l in locations:
filedict[myfile].append(functools.partial(
-   get_mirror_url, l, myfile, 
mirror_cache))
+   get_mirror_url, l, myfile, mysettings, 
mirror_cache))
if myuri is None:
continue
if myuri[:9]=="mirror://":
-- 
2.23.0




Re: [gentoo-portage-dev] [PATCH] emirrordist: Delete potential symlinks for all layouts

2019-10-17 Thread Michał Górny
On Thu, 2019-10-17 at 11:02 -0700, Zac Medico wrote:
> On 10/17/19 5:09 AM, Michał Górny wrote:
> > Unlink for all distfile layouts rather than for all but primary.  This
> > wrongly assumed that the primary layout will always be removed
> > as a result of earlier code.  However, the rename() call correctly
> > assumes that the primary layout may be an old symlink, and operates
> > on realpath().  Therefore, the real file from a secondary layout may
> > be renamed, leaving dangling symlink.
> > 
> > Bug: https://bugs.gentoo.org/697906
> > Signed-off-by: Michał Górny 
> > ---
> >  lib/portage/_emirrordist/DeletionTask.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/portage/_emirrordist/DeletionTask.py 
> > b/lib/portage/_emirrordist/DeletionTask.py
> > index db5ac5ffb..a4bb29419 100644
> > --- a/lib/portage/_emirrordist/DeletionTask.py
> > +++ b/lib/portage/_emirrordist/DeletionTask.py
> > @@ -102,7 +102,7 @@ class DeletionTask(CompositeTask):
> >  
> > def _delete_links(self):
> > success = True
> > -   for layout in self.config.layouts[1:]:
> > +   for layout in self.config.layouts:
> > distfile_path = os.path.join(
> > self.config.options.distfiles,
> >         layout.get_path(self.distfile))
> > 
> 
> Looks good. Please merge.

Done.  Could you make a bugfix release with this, please?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] emirrordist: Delete potential symlinks for all layouts

2019-10-17 Thread Michał Górny
Unlink for all distfile layouts rather than for all but primary.  This
wrongly assumed that the primary layout will always be removed
as a result of earlier code.  However, the rename() call correctly
assumes that the primary layout may be an old symlink, and operates
on realpath().  Therefore, the real file from a secondary layout may
be renamed, leaving dangling symlink.

Bug: https://bugs.gentoo.org/697906
Signed-off-by: Michał Górny 
---
 lib/portage/_emirrordist/DeletionTask.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/portage/_emirrordist/DeletionTask.py 
b/lib/portage/_emirrordist/DeletionTask.py
index db5ac5ffb..a4bb29419 100644
--- a/lib/portage/_emirrordist/DeletionTask.py
+++ b/lib/portage/_emirrordist/DeletionTask.py
@@ -102,7 +102,7 @@ class DeletionTask(CompositeTask):
 
def _delete_links(self):
success = True
-   for layout in self.config.layouts[1:]:
+   for layout in self.config.layouts:
distfile_path = os.path.join(
self.config.options.distfiles,
layout.get_path(self.distfile))
-- 
2.23.0




[gentoo-portage-dev] [PATCH v2] fetch: Make FlatLayout.get_filenames() not return directories

2019-10-13 Thread Michał Górny
Stop returning directories from FlatLayout.get_filenames().  This causes
emirrordist to wrongly presume directories created by new layout
to be distfiles, and causes some noisy errors.

Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 5d0bc7355..cd204b755 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -35,6 +35,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.util:atomic_ofstream',
'portage.util.configparser:SafeConfigParser,read_configs,' +
'ConfigParserError',
+   'portage.util.install_mask:_raise_exc',
'portage.util._urlopen:urlopen',
 )
 
@@ -269,7 +270,9 @@ class FlatLayout(object):
return filename
 
def get_filenames(self, distdir):
-   return iter(os.listdir(distdir))
+   for dirpath, dirnames, filenames in os.walk(distdir,
+   onerror=_raise_exc):
+   return iter(filenames)
 
@staticmethod
def verify_args(args):
-- 
2.23.0




[gentoo-portage-dev] [PATCH] fetch: Make FlatLayout.get_filenames() not return directories

2019-10-13 Thread Michał Górny
Stop returning directories from FlatLayout.get_filenames().  This causes
emirrordist to wrongly presume directories created by new layout
to be distfiles, and causes some noisy errors.

Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 5d0bc7355..793402137 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -269,7 +269,8 @@ class FlatLayout(object):
return filename
 
def get_filenames(self, distdir):
-   return iter(os.listdir(distdir))
+   for dirpath, dirnames, filenames in os.walk(distdir):
+   return iter(filenames)
 
@staticmethod
def verify_args(args):
-- 
2.23.0




[gentoo-portage-dev] [PATCH] man/emirrordist.1: document new options

2019-10-12 Thread Michał Górny
Signed-off-by: Michał Górny 
---
 man/emirrordist.1 | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/man/emirrordist.1 b/man/emirrordist.1
index cdd66cbdb..45108ef8c 100644
--- a/man/emirrordist.1
+++ b/man/emirrordist.1
@@ -119,6 +119,16 @@ Directory for individual fetch logs.
 Specifies a file containing a list of files to whitelist, one per line,
 # prefixed lines ignored. Use this option multiple times in order to
 specify multiple whitelists.
+.TP
+\fB\-\-symlinks\fR
+Use symbolic links to link distfiles to the primary layout from other
+specified layouts. If not specified, hard links are used instead.
+.TP
+\fB\-\-layout\-conf\fR=\fIFILE\fR
+Specifies alternate mirror \fIlayout.conf\fR file to use in order
+to select the desired distfile layout. If not specified,
+\fIlayout.conf\fR in \fB\-\-distfiles\fR directory will be used
+if present, otherwise flat layout will be assumed.
 .SH "REPORTING BUGS"
 Please report bugs via https://bugs.gentoo.org/
 .SH "THANKS"
-- 
2.23.0




[gentoo-portage-dev] [PATCH] emirrordist: Support specifying custom --layout-conf

2019-10-11 Thread Michał Górny
Support specifying custom layout.conf file to use.  This makes it
possible to start propagating new mirror layout without exposing it
to users.

Signed-off-by: Michał Górny 
---
 lib/portage/_emirrordist/Config.py | 6 --
 lib/portage/_emirrordist/main.py   | 6 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/portage/_emirrordist/Config.py 
b/lib/portage/_emirrordist/Config.py
index ace3b0a6a..c1f59f725 100644
--- a/lib/portage/_emirrordist/Config.py
+++ b/lib/portage/_emirrordist/Config.py
@@ -74,8 +74,10 @@ class Config(object):
options.deletion_db, 'deletion')
 
self.layout_conf = MirrorLayoutConfig()
-   self.layout_conf.read_from_file(
-   os.path.join(self.distfiles, 'layout.conf'))
+   if options.layout_conf is None:
+   options.layout_conf = os.path.join(self.distfiles,
+   'layout.conf')
+   self.layout_conf.read_from_file(options.layout_conf)
self.layouts = self.layout_conf.get_all_layouts()
 
def _open_log(self, log_desc, log_path, mode):
diff --git a/lib/portage/_emirrordist/main.py b/lib/portage/_emirrordist/main.py
index 0ae45ab6f..ce0c2929f 100644
--- a/lib/portage/_emirrordist/main.py
+++ b/lib/portage/_emirrordist/main.py
@@ -193,6 +193,12 @@ common_options = (
"distfiles between layouts",
"action"   : "store_true"
},
+   {
+   "longopt"  : "--layout-conf",
+   "help" : "specifies layout.conf file to use instead of "
+   "the one present in the distfiles directory",
+   "metavar"  : "FILE"
+   },
 )
 
 def parse_args(args):
-- 
2.23.0




[gentoo-portage-dev] [PATCH v3] fetch: Support GLEP 75 mirror structure

2019-10-04 Thread Michał Górny
Add a support for the subset of GLEP 75 needed by Gentoo Infra.  This
includes fetching and parsing layout.conf, and support for flat layout
and filename-hash layout with cutoffs being multiplies of 4.

Bug: https://bugs.gentoo.org/646898
Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py| 158 -
 lib/portage/tests/ebuild/test_fetch.py |  94 ++-
 2 files changed, 247 insertions(+), 5 deletions(-)

Chages in v3:
- mirrors are evaluated lazily (i.e. Portage doesn't fetch layouts
  for all mirrors prematurely),
- garbage layout.conf is handled gracefully,
- cache updates are done atomically,
- layout.conf argument verification has been moved to invidual classes,
- a few unit and integration tests have been added.

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 227bf45ae..fa250535f 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -6,13 +6,17 @@ from __future__ import print_function
 __all__ = ['fetch']
 
 import errno
+import functools
 import io
+import itertools
+import json
 import logging
 import random
 import re
 import stat
 import sys
 import tempfile
+import time
 
 from collections import OrderedDict
 
@@ -27,14 +31,19 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.package.ebuild.doebuild:doebuild_environment,' + \
'_doebuild_spawn',
'portage.package.ebuild.prepare_build_dirs:prepare_build_dirs',
+   'portage.util:atomic_ofstream',
+   'portage.util.configparser:SafeConfigParser,read_configs,' +
+   'NoOptionError,ConfigParserError',
+   'portage.util._urlopen:urlopen',
 )
 
 from portage import os, selinux, shutil, _encodings, \
_movefile, _shell_quote, _unicode_encode
 from portage.checksum import (get_valid_checksum_keys, perform_md5, verify_all,
-   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter)
+   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter,
+   checksum_str)
 from portage.const import BASH_BINARY, CUSTOM_MIRRORS_FILE, \
-   GLOBAL_CONFIG_PATH
+   GLOBAL_CONFIG_PATH, CACHE_PATH
 from portage.data import portage_gid, portage_uid, secpass, userpriv_groups
 from portage.exception import FileNotFound, OperationNotPermitted, \
PortageException, TryAgain
@@ -253,6 +262,144 @@ _size_suffix_map = {
'Y' : 80,
 }
 
+
+class FlatLayout(object):
+   def get_path(self, filename):
+   return filename
+
+   @staticmethod
+   def verify_args(args):
+   return len(args) == 1
+
+
+class FilenameHashLayout(object):
+   def __init__(self, algo, cutoffs):
+   self.algo = algo
+   self.cutoffs = [int(x) for x in cutoffs.split(':')]
+
+   def get_path(self, filename):
+   fnhash = checksum_str(filename.encode('utf8'), self.algo)
+   ret = ''
+   for c in self.cutoffs:
+   assert c % 4 == 0
+   c = c // 4
+   ret += fnhash[:c] + '/'
+   fnhash = fnhash[c:]
+   return ret + filename
+
+   @staticmethod
+   def verify_args(args):
+   if len(args) != 3:
+   return False
+   if args[1] not in get_valid_checksum_keys():
+   return False
+   # argsidate cutoffs
+   for c in args[2].split(':'):
+   try:
+   c = int(c)
+   except ValueError:
+   break
+   else:
+   if c % 4 != 0:
+   break
+   else:
+   return True
+   return False
+
+
+class MirrorLayoutConfig(object):
+   """
+   Class to read layout.conf from a mirror.
+   """
+
+   def __init__(self):
+   self.structure = ()
+
+   def read_from_file(self, f):
+   cp = SafeConfigParser()
+   read_configs(cp, [f])
+   vals = []
+   for i in itertools.count():
+   try:
+   vals.append(tuple(cp.get('structure', '%d' % 
i).split()))
+   except NoOptionError:
+   break
+   self.structure = tuple(vals)
+
+   def serialize(self):
+   return self.structure
+
+   def deserialize(self, data):
+   self.structure = data
+
+   @staticmethod
+   def validate_structure(val):
+   if val[0] == 'flat':
+   return FlatLayout.verify_args(val)
+   if val[0] == 'filename-hash':
+   return FilenameHashLayout.verify_args(val)
+   return False
+
+   def 

Re: [gentoo-portage-dev] [PATCH v2] fetch: Support GLEP 75 mirror structure

2019-10-03 Thread Michał Górny
On Thu, 2019-10-03 at 22:01 -0700, Alec Warner wrote:
> On Thu, Oct 3, 2019 at 9:37 AM Michał Górny  wrote:
> 
> > Add a support for the subset of GLEP 75 needed by Gentoo Infra.  This
> > includes fetching and parsing layout.conf, and support for flat layout
> > and filename-hash layout with cutoffs being multiplies of 4.
> > 
> > Bug: https://bugs.gentoo.org/646898
> > Signed-off-by: Michał Górny 
> > ---
> >  lib/portage/package/ebuild/fetch.py | 139 +++-
> >  1 file changed, 135 insertions(+), 4 deletions(-)
> > 
> > Changes in v2: switched to a more classy layout to make the code
> > reusable in emirrordist.
> > 
> > diff --git a/lib/portage/package/ebuild/fetch.py
> > b/lib/portage/package/ebuild/fetch.py
> > index 227bf45ae..18e3d390a 100644
> > --- a/lib/portage/package/ebuild/fetch.py
> > +++ b/lib/portage/package/ebuild/fetch.py
> > @@ -7,12 +7,15 @@ __all__ = ['fetch']
> > 
> >  import errno
> >  import io
> > +import itertools
> > +import json
> >  import logging
> >  import random
> >  import re
> >  import stat
> >  import sys
> >  import tempfile
> > +import time
> > 
> >  from collections import OrderedDict
> > 
> > @@ -27,14 +30,17 @@ portage.proxy.lazyimport.lazyimport(globals(),
> > 'portage.package.ebuild.doebuild:doebuild_environment,' + \
> > '_doebuild_spawn',
> > 'portage.package.ebuild.prepare_build_dirs:prepare_build_dirs',
> > +
> >  'portage.util.configparser:SafeConfigParser,read_configs,NoOptionError',
> > +   'portage.util._urlopen:urlopen',
> >  )
> > 
> >  from portage import os, selinux, shutil, _encodings, \
> > _movefile, _shell_quote, _unicode_encode
> >  from portage.checksum import (get_valid_checksum_keys, perform_md5,
> > verify_all,
> > -   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter)
> > +   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter,
> > +   checksum_str)
> >  from portage.const import BASH_BINARY, CUSTOM_MIRRORS_FILE, \
> > -   GLOBAL_CONFIG_PATH
> > +   GLOBAL_CONFIG_PATH, CACHE_PATH
> >  from portage.data import portage_gid, portage_uid, secpass,
> > userpriv_groups
> >  from portage.exception import FileNotFound, OperationNotPermitted, \
> > PortageException, TryAgain
> > @@ -253,6 +259,130 @@ _size_suffix_map = {
> > 'Y' : 80,
> >  }
> > 
> > +
> > +class FlatLayout(object):
> > +   def get_path(self, filename):
> > +   return filename
> > +
> > +
> > +class FilenameHashLayout(object):
> > +   def __init__(self, algo, cutoffs):
> > +   self.algo = algo
> > +   self.cutoffs = [int(x) for x in cutoffs.split(':')]
> > +
> > +   def get_path(self, filename):
> > +   fnhash = checksum_str(filename.encode('utf8'), self.algo)
> > +   ret = ''
> > +   for c in self.cutoffs:
> > +   assert c % 4 == 0
> > 
> 
> I'm not quite sure what this assert is doing. I'm not super in favor of
> asserts (I'd rather see an exception like raise FooError("..."), but if you
> are going to use it please use something like:
> 
> assert c %4 == 0, "Some description of why we put this assert here so if it
> fires we can do something useful."

It's already checked in validate_structure().  Maybe I could indirect it
to this class to make things clearer.

> 
> +   c = c // 4
> > +   ret += fnhash[:c] + '/'
> > +   fnhash = fnhash[c:]
> > +   return ret + filename
> > +
> > +
> > +class MirrorLayoutConfig(object):
> > +   """
> > +   Class to read layout.conf from a mirror.
> > +   """
> > +
> > +   def __init__(self):
> > +   self.structure = ()
> > +
> > +   def read_from_file(self, f):
> > +   cp = SafeConfigParser()
> > +   read_configs(cp, [f])
> > +   vals = []
> > +   for i in itertools.count():
> > +   try:
> > +   vals.append(tuple(cp.get('structure', '%d'
> > % i).split()))
> > +   except NoOptionError:
> > +   break
> > +   self.structure = tuple(vals)
> > +
>

Re: [gentoo-portage-dev] [PATCH] fetch: Support GLEP 75 mirror structure

2019-10-03 Thread Michał Górny
On Thu, 2019-10-03 at 21:58 -0700, Alec Warner wrote:
> On Thu, Oct 3, 2019 at 7:52 AM Michał Górny  wrote:
> 
> > Add a support for the subset of GLEP 75 needed by Gentoo Infra.  This
> > includes fetching and parsing layout.conf, and support for flat layout
> > and filename-hash layout with cutoffs being multiplies of 4.
> > 
> > Bug: https://bugs.gentoo.org/646898
> > Signed-off-by: Michał Górny 
> > ---
> >  lib/portage/package/ebuild/fetch.py | 113 +++-
> >  1 file changed, 109 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/portage/package/ebuild/fetch.py
> > b/lib/portage/package/ebuild/fetch.py
> > index 227bf45ae..692efcc01 100644
> > --- a/lib/portage/package/ebuild/fetch.py
> > +++ b/lib/portage/package/ebuild/fetch.py
> > @@ -7,12 +7,15 @@ __all__ = ['fetch']
> > 
> >  import errno
> >  import io
> > +import itertools
> > +import json
> >  import logging
> >  import random
> >  import re
> >  import stat
> >  import sys
> >  import tempfile
> > +import time
> > 
> >  from collections import OrderedDict
> > 
> > @@ -27,14 +30,17 @@ portage.proxy.lazyimport.lazyimport(globals(),
> > 'portage.package.ebuild.doebuild:doebuild_environment,' + \
> > '_doebuild_spawn',
> > 'portage.package.ebuild.prepare_build_dirs:prepare_build_dirs',
> > +
> >  'portage.util.configparser:SafeConfigParser,read_configs,NoOptionError',
> > +   'portage.util._urlopen:urlopen',
> >  )
> > 
> >  from portage import os, selinux, shutil, _encodings, \
> > _movefile, _shell_quote, _unicode_encode
> >  from portage.checksum import (get_valid_checksum_keys, perform_md5,
> > verify_all,
> > -   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter)
> > +   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter,
> > +   checksum_str)
> >  from portage.const import BASH_BINARY, CUSTOM_MIRRORS_FILE, \
> > -   GLOBAL_CONFIG_PATH
> > +   GLOBAL_CONFIG_PATH, CACHE_PATH
> >  from portage.data import portage_gid, portage_uid, secpass,
> > userpriv_groups
> >  from portage.exception import FileNotFound, OperationNotPermitted, \
> > PortageException, TryAgain
> > @@ -253,6 +259,104 @@ _size_suffix_map = {
> > 'Y' : 80,
> >  }
> > 
> > +
> > +def filename_hash_path(filename, algo, cutoffs):
> > +   """
> > +   Get directory path for filename in filename-hash mirror structure.
> > +
> > +   @param filename: Filename to fetch
> > +   @param algo: Hash algorithm
> > +   @param cutoffs: Cutoff values (n:n...)
> > +   @return: Directory path
> > +   """
> > +
> > +   fnhash = checksum_str(filename.encode('utf8'), algo)
> > +   ret = ''
> > +   for c in cutoffs.split(':'):
> > +   c = int(c) // 4
> > +   ret += fnhash[:c] + '/'
> > 
> 
> When making a path, please use os.path.join()

This is URL, not a path.

> 
> 
> > +   fnhash = fnhash[c:]
> > +   return ret
> > +
> > +
> > +def get_mirror_url(mirror_url, filename, eroot):
> > +   """
> > +   Get correct fetch URL for a given file, accounting for mirror
> > +   layout configuration.
> > +
> > +   @param mirror_url: Base URL to the mirror (without '/distfiles')
> > +   @param filename: Filename to fetch
> > +   @param eroot: EROOT to use for the cache file
> > +   @return: Full URL to fetch
> > +   """
> > 
> +
> > +   cache_file = os.path.join(eroot, CACHE_PATH,
> > 'mirror-metadata.json')
> > +   try:
> > +   with open(cache_file, 'r') as f:
> > +   cache = json.load(f)
> > +   except (IOError, ValueError):
> > +   cache = {}
> > 
> 
> I'm a bit worried that we are opening this cache file off of disk every
> time we call get_mirror_url(). Can we just cache the contents in memory
> between calls; or even better pass the cache in as argument rather than it
> be contained in get_mirror_url?

We could but this is no bottleneck.  That's premature optimization,
the way I see it.

> 
> 
> > +
> > +   ts, layout = cache.get(mirror_url, (0, None))
> > +   # refresh at least daily
> > +   if ts < time.time() - 86400:
> > +   # t

[gentoo-portage-dev] [PATCH v2] fetch: Support GLEP 75 mirror structure

2019-10-03 Thread Michał Górny
Add a support for the subset of GLEP 75 needed by Gentoo Infra.  This
includes fetching and parsing layout.conf, and support for flat layout
and filename-hash layout with cutoffs being multiplies of 4.

Bug: https://bugs.gentoo.org/646898
Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 139 +++-
 1 file changed, 135 insertions(+), 4 deletions(-)

Changes in v2: switched to a more classy layout to make the code
reusable in emirrordist.

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 227bf45ae..18e3d390a 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -7,12 +7,15 @@ __all__ = ['fetch']
 
 import errno
 import io
+import itertools
+import json
 import logging
 import random
 import re
 import stat
 import sys
 import tempfile
+import time
 
 from collections import OrderedDict
 
@@ -27,14 +30,17 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.package.ebuild.doebuild:doebuild_environment,' + \
'_doebuild_spawn',
'portage.package.ebuild.prepare_build_dirs:prepare_build_dirs',
+   'portage.util.configparser:SafeConfigParser,read_configs,NoOptionError',
+   'portage.util._urlopen:urlopen',
 )
 
 from portage import os, selinux, shutil, _encodings, \
_movefile, _shell_quote, _unicode_encode
 from portage.checksum import (get_valid_checksum_keys, perform_md5, verify_all,
-   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter)
+   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter,
+   checksum_str)
 from portage.const import BASH_BINARY, CUSTOM_MIRRORS_FILE, \
-   GLOBAL_CONFIG_PATH
+   GLOBAL_CONFIG_PATH, CACHE_PATH
 from portage.data import portage_gid, portage_uid, secpass, userpriv_groups
 from portage.exception import FileNotFound, OperationNotPermitted, \
PortageException, TryAgain
@@ -253,6 +259,130 @@ _size_suffix_map = {
'Y' : 80,
 }
 
+
+class FlatLayout(object):
+   def get_path(self, filename):
+   return filename
+
+
+class FilenameHashLayout(object):
+   def __init__(self, algo, cutoffs):
+   self.algo = algo
+   self.cutoffs = [int(x) for x in cutoffs.split(':')]
+
+   def get_path(self, filename):
+   fnhash = checksum_str(filename.encode('utf8'), self.algo)
+   ret = ''
+   for c in self.cutoffs:
+   assert c % 4 == 0
+   c = c // 4
+   ret += fnhash[:c] + '/'
+   fnhash = fnhash[c:]
+   return ret + filename
+
+
+class MirrorLayoutConfig(object):
+   """
+   Class to read layout.conf from a mirror.
+   """
+
+   def __init__(self):
+   self.structure = ()
+
+   def read_from_file(self, f):
+   cp = SafeConfigParser()
+   read_configs(cp, [f])
+   vals = []
+   for i in itertools.count():
+   try:
+   vals.append(tuple(cp.get('structure', '%d' % 
i).split()))
+   except NoOptionError:
+   break
+   self.structure = tuple(vals)
+
+   def serialize(self):
+   return self.structure
+
+   def deserialize(self, data):
+   self.structure = data
+
+   @staticmethod
+   def validate_structure(val):
+   if val == ('flat',):
+   return True
+   if val[0] == 'filename-hash' and len(val) == 3:
+   if val[1] not in get_valid_checksum_keys():
+   return False
+   # validate cutoffs
+   for c in val[2].split(':'):
+   try:
+   c = int(c)
+   except ValueError:
+   break
+   else:
+   if c % 4 != 0:
+   break
+   else:
+   return True
+   return False
+   return False
+
+   def get_best_supported_layout(self):
+   for val in self.structure:
+   if self.validate_structure(val):
+   if val[0] == 'flat':
+   return FlatLayout()
+   elif val[0] == 'filename-hash':
+   return FilenameHashLayout(val[1], 
val[2])
+   else:
+   # fallback
+   return FlatLayout()
+
+
+def get_mirror_url(mirror_url, filename, eroot):
+   """
+   Get correct fetch URL for a giv

[gentoo-portage-dev] [PATCH] fetch: Support GLEP 75 mirror structure

2019-10-03 Thread Michał Górny
Add a support for the subset of GLEP 75 needed by Gentoo Infra.  This
includes fetching and parsing layout.conf, and support for flat layout
and filename-hash layout with cutoffs being multiplies of 4.

Bug: https://bugs.gentoo.org/646898
Signed-off-by: Michał Górny 
---
 lib/portage/package/ebuild/fetch.py | 113 +++-
 1 file changed, 109 insertions(+), 4 deletions(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 227bf45ae..692efcc01 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -7,12 +7,15 @@ __all__ = ['fetch']
 
 import errno
 import io
+import itertools
+import json
 import logging
 import random
 import re
 import stat
 import sys
 import tempfile
+import time
 
 from collections import OrderedDict
 
@@ -27,14 +30,17 @@ portage.proxy.lazyimport.lazyimport(globals(),
'portage.package.ebuild.doebuild:doebuild_environment,' + \
'_doebuild_spawn',
'portage.package.ebuild.prepare_build_dirs:prepare_build_dirs',
+   'portage.util.configparser:SafeConfigParser,read_configs,NoOptionError',
+   'portage.util._urlopen:urlopen',
 )
 
 from portage import os, selinux, shutil, _encodings, \
_movefile, _shell_quote, _unicode_encode
 from portage.checksum import (get_valid_checksum_keys, perform_md5, verify_all,
-   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter)
+   _filter_unaccelarated_hashes, _hash_filter, _apply_hash_filter,
+   checksum_str)
 from portage.const import BASH_BINARY, CUSTOM_MIRRORS_FILE, \
-   GLOBAL_CONFIG_PATH
+   GLOBAL_CONFIG_PATH, CACHE_PATH
 from portage.data import portage_gid, portage_uid, secpass, userpriv_groups
 from portage.exception import FileNotFound, OperationNotPermitted, \
PortageException, TryAgain
@@ -253,6 +259,104 @@ _size_suffix_map = {
'Y' : 80,
 }
 
+
+def filename_hash_path(filename, algo, cutoffs):
+   """
+   Get directory path for filename in filename-hash mirror structure.
+
+   @param filename: Filename to fetch
+   @param algo: Hash algorithm
+   @param cutoffs: Cutoff values (n:n...)
+   @return: Directory path
+   """
+
+   fnhash = checksum_str(filename.encode('utf8'), algo)
+   ret = ''
+   for c in cutoffs.split(':'):
+   c = int(c) // 4
+   ret += fnhash[:c] + '/'
+   fnhash = fnhash[c:]
+   return ret
+
+
+def get_mirror_url(mirror_url, filename, eroot):
+   """
+   Get correct fetch URL for a given file, accounting for mirror
+   layout configuration.
+
+   @param mirror_url: Base URL to the mirror (without '/distfiles')
+   @param filename: Filename to fetch
+   @param eroot: EROOT to use for the cache file
+   @return: Full URL to fetch
+   """
+
+   cache_file = os.path.join(eroot, CACHE_PATH, 'mirror-metadata.json')
+   try:
+   with open(cache_file, 'r') as f:
+   cache = json.load(f)
+   except (IOError, ValueError):
+   cache = {}
+
+   ts, layout = cache.get(mirror_url, (0, None))
+   # refresh at least daily
+   if ts < time.time() - 86400:
+   # the default
+   layout = ('flat',)
+
+   try:
+   f = urlopen(mirror_url + '/distfiles/layout.conf')
+   try:
+   data = io.StringIO(f.read().decode('utf8'))
+   finally:
+   f.close()
+   cp = SafeConfigParser()
+   read_configs(cp, [data])
+
+   for i in itertools.count():
+   try:
+   val = tuple(cp.get('structure', '%d' % 
i).split())
+   if val == ('flat',):
+   pass
+   elif val[0] == 'filename-hash' and 
len(val) == 3:
+   if val[1] not in 
get_valid_checksum_keys():
+   continue
+   # validate cutoffs
+   cutoffs_good = False
+   for c in val[2].split(':'):
+   try:
+   c = int(c)
+   except ValueError:
+   break
+   else:
+   if c % 4 != 0:
+ 

Re: [gentoo-portage-dev] [PATCH] Support PROPERTIES="live".

2019-07-28 Thread Michał Górny
On Sun, 2019-07-28 at 17:21 -0700, Zac Medico wrote:
> On 7/26/19 3:45 PM, Michał Górny wrote:
> > While at it, could you look into making src_unpack() network-sandbox
> > override apply only to ebuilds with PROPERTIES=live?
> 
> There could be another subset of packages that aren't quite "live" but
> they need to fetch something that's immutable which can't be fetched via
> a protocol supported by SRC_URI. Maybe call it
> PROPERTIES="src-fetch-immutable" or something.

Could you name one of those packages, and explain how are they not
'live' if they skip checksum verification?  I'm really against adding
hundreds of weird-named items for the sake of potential use cases
without a real benefit to it.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Support PROPERTIES="live".

2019-07-26 Thread Michał Górny
On Fri, 2019-07-26 at 10:51 +0200, Ulrich Müller wrote:
> Bug: https://bugs.gentoo.org/233589
> Signed-off-by: Ulrich Müller 
> ---
>  cnf/sets/portage.conf |  8 +++-
>  lib/_emerge/EbuildExecuter.py |  4 ++--
>  lib/portage/_sets/__init__.py | 12 +---
>  repoman/lib/repoman/modules/scan/ebuild/ebuild.py |  3 +--
>  4 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/cnf/sets/portage.conf b/cnf/sets/portage.conf
> index ac282d911..ed4c6d9a7 100644
> --- a/cnf/sets/portage.conf
> +++ b/cnf/sets/portage.conf
> @@ -55,9 +55,15 @@ world-candidate = True
>  [preserved-rebuild]
>  class = portage.sets.libs.PreservedLibraryConsumerSet
>  
> -# Installed ebuilds that inherit from known live eclasses.
> +# Installed ebuilds with "live" property.
>  [live-rebuild]
>  class = portage.sets.dbapi.VariableSet
> +variable = PROPERTIES
> +includes = live
> +
> +# Installed ebuilds that inherit from known live eclasses.
> +[deprecated-live-rebuild]
> +class = portage.sets.dbapi.VariableSet
>  variable = INHERITED
>  includes = bzr cvs darcs git git-2 git-r3 golang-vcs mercurial subversion tla
>  
> diff --git a/lib/_emerge/EbuildExecuter.py b/lib/_emerge/EbuildExecuter.py
> index d387b42be..ca9859437 100644
> --- a/lib/_emerge/EbuildExecuter.py
> +++ b/lib/_emerge/EbuildExecuter.py
> @@ -1,4 +1,4 @@
> -# Copyright 1999-2018 Gentoo Foundation
> +# Copyright 1999-2019 Gentoo Authors
>  # Distributed under the terms of the GNU General Public License v2
>  
>  from _emerge.EbuildPhase import EbuildPhase
> @@ -49,7 +49,7 @@ class EbuildExecuter(CompositeTask):
>   phase="unpack", scheduler=self.scheduler,
>   settings=self.settings)
>  
> - if self._live_eclasses.intersection(self.pkg.inherited):
> + if "live" in self.settings.get("PROPERTIES", "").split():
>   # Serialize $DISTDIR access for live ebuilds since
>   # otherwise they can interfere with eachother.
>  
> diff --git a/lib/portage/_sets/__init__.py b/lib/portage/_sets/__init__.py
> index 2c9bf9715..7b81c55e2 100644
> --- a/lib/portage/_sets/__init__.py
> +++ b/lib/portage/_sets/__init__.py
> @@ -1,4 +1,4 @@
> -# Copyright 2007-2014 Gentoo Foundation
> +# Copyright 2007-2019 Gentoo Authors
>  # Distributed under the terms of the GNU General Public License v2
>  
>  from __future__ import print_function
> @@ -121,8 +121,14 @@ class SetConfig(object):
>   parser.remove_section("live-rebuild")
>   parser.add_section("live-rebuild")
>   parser.set("live-rebuild", "class", 
> "portage.sets.dbapi.VariableSet")
> - parser.set("live-rebuild", "variable", "INHERITED")
> - parser.set("live-rebuild", "includes", " 
> ".join(sorted(portage.const.LIVE_ECLASSES)))
> + parser.set("live-rebuild", "variable", "PROPERTIES")
> + parser.set("live-rebuild", "includes", "live")
> +
> + parser.remove_section("deprecated-live-rebuild")
> + parser.add_section("deprecated-live-rebuild")
> + parser.set("deprecated-live-rebuild", "class", 
> "portage.sets.dbapi.VariableSet")
> + parser.set("deprecated-live-rebuild", "variable", "INHERITED")
> + parser.set("deprecated-live-rebuild", "includes", " 
> ".join(sorted(portage.const.LIVE_ECLASSES)))
>  
>   parser.remove_section("module-rebuild")
>   parser.add_section("module-rebuild")
> diff --git a/repoman/lib/repoman/modules/scan/ebuild/ebuild.py 
> b/repoman/lib/repoman/modules/scan/ebuild/ebuild.py
> index d2715bc6e..70011e387 100644
> --- a/repoman/lib/repoman/modules/scan/ebuild/ebuild.py
> +++ b/repoman/lib/repoman/modules/scan/ebuild/ebuild.py
> @@ -12,7 +12,6 @@ from repoman.modules.scan.scanbase import ScanBase
>  # import our initialized portage instance
>  from repoman._portage import portage
>  from portage import os
> -from portage.const import LIVE_ECLASSES
>  from portage.exception import InvalidPackageName
>  
>  pv_toolong_re = re.compile(r'[0-9]{19,}')
> @@ -110,7 +109,7 @@ class Ebuild(ScanBase):
>   self.metadata = self.pkg._metadata
>   self.eapi = self.metadata["EAPI"]
>   self.inherited = self.pkg.inherited
> - self.live_ebuild = LIVE_ECLASSES.intersection(self.inherited)
> + self.live_ebuild = "live" in self.metadata["PROPERTIES"].split()
>   self.keywords = self.metadata["KEYWORDS"].split()
>   self.archs = set(kw.lstrip("~") for kw in self.keywords if not 
> kw.startswith("-"))
>   return False

While at it, could you look into making src_unpack() network-sandbox
override apply only to ebuilds with PROPERTIES=live?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [RFC] Adding extra vars to md5-cache, for QA purposes

2019-07-25 Thread Michał Górny
On Thu, 2019-07-25 at 12:57 -0700, Zac Medico wrote:
> On 7/25/19 5:20 AM, Michał Górny wrote:
> > Hi,
> > 
> > TL;DR: I'd like to make it possible for ebuilds to define additional
> > variables that will be stored in md5-cache.  This would be useful for CI
> > and other tooling that right now has to parse ebuilds for other data.
> > 
> > 
> > The idea is to add a new incremental ebuild/eclass variable (technical
> > name: QA_EXTRA_CACHE_VARS) that would define additional data to be
> > stored in cache.  For example, python*-r1 eclasses would define
> > 'PYTHON_COMPAT', acct-user would define 'ACCT_USER_ID', etc.
> > 
> > When regenerating cache, the PM would read this variable, and store
> > the values of all defined variables into md5-cache.  As a result,
> > programs needing those variables can get them straight from cache
> > without having to attempt to run or parse ebuilds (which is both slow
> > and prone to bugs).
> > 
> > This would benefit e.g. gpyutils that right now need to attempt to parse
> > PYTHON_COMPAT from ebuilds.  It would also benefit writing future
> > pkgcheck checks for user/group ID collisions.
> > 
> > 
> > Notes:
> > 
> > - since md5-cache uses key-value format and allows for future
> > extensions, the new values can be added without breaking anything;
> > 
> > - md5-cache is not specified in the PMS, and the whole thing can be
> > implemented without need for EAPI bump,
> > 
> > - I would like to have this implemented consistently both in Portage
> > and pkgcore,
> > 
> > - we will need to clearly define how to dump arrays.
> > 
> > 
> > What do you think?
> 
> Sounds good. Some thoughts:
> 
> * Maybe omit QA from the variable name, since it can be could be
> generally useful for things that are unrelated to QA.
> 
> * In the md5-cache entry, maybe use a common prefix like EXT_ for the
> extra keys in order to distinguish them from normal keys.

Yeah, I was thinking of something like '__ext_foo', or '__ext[foo]'.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [RFC] Adding extra vars to md5-cache, for QA purposes

2019-07-25 Thread Michał Górny
Hi,

TL;DR: I'd like to make it possible for ebuilds to define additional
variables that will be stored in md5-cache.  This would be useful for CI
and other tooling that right now has to parse ebuilds for other data.


The idea is to add a new incremental ebuild/eclass variable (technical
name: QA_EXTRA_CACHE_VARS) that would define additional data to be
stored in cache.  For example, python*-r1 eclasses would define
'PYTHON_COMPAT', acct-user would define 'ACCT_USER_ID', etc.

When regenerating cache, the PM would read this variable, and store
the values of all defined variables into md5-cache.  As a result,
programs needing those variables can get them straight from cache
without having to attempt to run or parse ebuilds (which is both slow
and prone to bugs).

This would benefit e.g. gpyutils that right now need to attempt to parse
PYTHON_COMPAT from ebuilds.  It would also benefit writing future
pkgcheck checks for user/group ID collisions.


Notes:

- since md5-cache uses key-value format and allows for future
extensions, the new values can be added without breaking anything;

- md5-cache is not specified in the PMS, and the whole thing can be
implemented without need for EAPI bump,

- I would like to have this implemented consistently both in Portage
and pkgcore,

- we will need to clearly define how to dump arrays.


What do you think?

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] repoman: add 'user.eclass' to deprecated eclasses

2019-07-24 Thread Michał Górny
Dnia July 24, 2019 6:19:05 AM UTC, Ulrich Mueller  napisał(a):
>>>>>> On Tue, 23 Jul 2019, Michał Górny wrote:
>
>> Make repoman report user.eclass as deprecated by GLEP 81.
>
>I don't understand. user.eclass is inherited by both acct-user.eclass
>and acct-group.eclass, therefore in active use.
>
>Or are you planning to inline its code in the inheriting eclasses?

This doesn't trigger for indirect inherits. Unless I tested wrong.

>
>Ulrich


--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] [PATCH] repoman: add 'user.eclass' to deprecated eclasses

2019-07-23 Thread Michał Górny
On Tue, 2019-07-23 at 13:05 -0700, Zac Medico wrote:
> On 7/23/19 8:00 AM, Michał Górny wrote:
> > Make repoman report user.eclass as deprecated by GLEP 81.
> > 
> > Signed-off-by: Michał Górny 
> > ---
> >  repoman/lib/repoman/modules/linechecks/deprecated/inherit.py | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py 
> > b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
> > index 77ad4f625..361da09b9 100644
> > --- a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
> > +++ b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
> > @@ -30,6 +30,7 @@ class InheritDeprecated(LineCheck):
> > "mono": "mono-env",
> > "python": "python-r1 / python-single-r1 / python-any-r1",
> > "ruby": "ruby-ng",
> > +   "user": "GLEP 81",
> >     "versionator": "eapi7-ver (built-in since EAPI 7)",
> > "x-modular": "xorg-2",
> > }
> > 
> 
> Looks good, please merge.

Merged, thanks.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH] repoman: add 'user.eclass' to deprecated eclasses

2019-07-23 Thread Michał Górny
Make repoman report user.eclass as deprecated by GLEP 81.

Signed-off-by: Michał Górny 
---
 repoman/lib/repoman/modules/linechecks/deprecated/inherit.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py 
b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
index 77ad4f625..361da09b9 100644
--- a/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
+++ b/repoman/lib/repoman/modules/linechecks/deprecated/inherit.py
@@ -30,6 +30,7 @@ class InheritDeprecated(LineCheck):
"mono": "mono-env",
"python": "python-r1 / python-single-r1 / python-any-r1",
"ruby": "ruby-ng",
+   "user": "GLEP 81",
"versionator": "eapi7-ver (built-in since EAPI 7)",
"x-modular": "xorg-2",
}
-- 
2.22.0




[gentoo-portage-dev] [PATCH 1/2] sync: Split key refresh into explicit WKD/keyserver phases

2019-07-04 Thread Michał Górny
Split key refresh into two parts: first try to refresh the key via WKD,
then via keyservers, rather than using the combined function that is
less explicit.  This ensures that users are correctly informed whether
keyservers are actually used, and therefore whether they may be subject
to SKS poisoning attacks.  Furthermore, it skips WKD from retry loop.

Signed-off-by: Michał Górny 
---
 lib/portage/sync/syncbase.py | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/portage/sync/syncbase.py b/lib/portage/sync/syncbase.py
index d15bb6d14..46644d68e 100644
--- a/lib/portage/sync/syncbase.py
+++ b/lib/portage/sync/syncbase.py
@@ -252,11 +252,17 @@ class SyncBase(object):
@type openpgp_env: gemato.openpgp.OpenPGPEnvironment
"""
out = portage.output.EOutput(quiet=('--quiet' in 
self.options['emerge_config'].opts))
+   out.ebegin('Refreshing keys via WKD')
+   if openpgp_env.refresh_keys_wkd():
+   out.eend(0)
+   return
+   out.eend(1)
+
out.ebegin('Refreshing keys from keyserver{}'.format(
('' if self.repo.sync_openpgp_keyserver is None else ' 
' + self.repo.sync_openpgp_keyserver)))
retry_decorator = self._key_refresh_retry_decorator()
if retry_decorator is None:
-   
openpgp_env.refresh_keys(keyserver=self.repo.sync_openpgp_keyserver)
+   
openpgp_env.refresh_keys_keyserver(keyserver=self.repo.sync_openpgp_keyserver)
else:
def noisy_refresh_keys():
"""
@@ -264,7 +270,7 @@ class SyncBase(object):
errors, display errors as soon as they occur.
"""
try:
-   
openpgp_env.refresh_keys(keyserver=self.repo.sync_openpgp_keyserver)
+   
openpgp_env.refresh_keys_keyserver(keyserver=self.repo.sync_openpgp_keyserver)
except Exception as e:
writemsg_level("%s\n" % (e,),
level=logging.ERROR, 
noiselevel=-1)
-- 
2.22.0




[gentoo-portage-dev] [PATCH 2/2] repos.conf: Default to using Gentoo keyservers

2019-07-04 Thread Michał Górny
Default to using hkps://keys.gentoo.org which are guaranteed to hold
the newest copies of Gentoo keys, are secured against key poisoning
and are more reliable than SKS.

Signed-off-by: Michał Górny 
---
 cnf/repos.conf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cnf/repos.conf b/cnf/repos.conf
index e84840bf2..2d73b3e35 100644
--- a/cnf/repos.conf
+++ b/cnf/repos.conf
@@ -10,6 +10,7 @@ sync-rsync-verify-jobs = 1
 sync-rsync-verify-metamanifest = yes
 sync-rsync-verify-max-age = 24
 sync-openpgp-key-path = /usr/share/openpgp-keys/gentoo-release.asc
+sync-openpgp-keyserver = hkps://keys.gentoo.org
 sync-openpgp-key-refresh-retry-count = 40
 sync-openpgp-key-refresh-retry-overall-timeout = 1200
 sync-openpgp-key-refresh-retry-delay-exp-base = 2
-- 
2.22.0




[gentoo-portage-dev] [PATCH] [repoman] Allow empty LICENSE in acct-* packages

2019-06-20 Thread Michał Górny
Since acct-* packages do not create any files (except for keepfile
for the home directory), it is perfectly normal for the LICENSE
to be empty.
---
 repoman/lib/repoman/modules/scan/metadata/ebuild_metadata.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/repoman/lib/repoman/modules/scan/metadata/ebuild_metadata.py 
b/repoman/lib/repoman/modules/scan/metadata/ebuild_metadata.py
index 4c35bbc12..2edf8f7f2 100644
--- a/repoman/lib/repoman/modules/scan/metadata/ebuild_metadata.py
+++ b/repoman/lib/repoman/modules/scan/metadata/ebuild_metadata.py
@@ -39,8 +39,8 @@ class EbuildMetadata(ScanBase):
ebuild = kwargs.get('ebuild').get()
for pos, missing_var in 
enumerate(self.repo_settings.qadata.missingvars):
if not ebuild.metadata.get(missing_var):
-   if kwargs.get('catdir') == "virtual" and \
-   missing_var in ("HOMEPAGE", "LICENSE"):
+   if (kwargs.get('catdir') in ("acct-group", 
"acct-user", "virtual")
+   and missing_var in ("HOMEPAGE", 
"LICENSE")):
continue
if ebuild.live_ebuild and missing_var == 
"KEYWORDS":
continue
-- 
2.22.0




Re: [gentoo-portage-dev] [PATCH] enable user settable gpg keyservers

2019-02-25 Thread Michał Górny
On Mon, 2019-02-25 at 14:34 -0600, Matthew Thode wrote:
> Allowing users to define which keyservers they update from allows them
> to work around buggy keyservers.  It is also useful for local mirrors
> and / or private keyservers.
> 
> It's likely that I'm setting the default incorrectly and not
> alphabetizing right, but the code works at least...
> 

I'm still wondering why you need to use keyservers instead of WKD which
is much more reliable.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] pid-sandbox: whitelist selected pkg_* phases (bug 673794)

2019-01-08 Thread Michał Górny
Dnia January 8, 2019 4:53:11 AM UTC, Zac Medico  napisał(a):
>Whitelist the same phases that are whitelisted for FEATURES=cgroup,
>since pid-sandbox is less valuable and is likely to have unintended
>consenquences during these phases.
>
>Bug: https://bugs.gentoo.org/673794
>Signed-off-by: Zac Medico 
>---
> lib/_emerge/AbstractEbuildProcess.py   | 9 ++---
> lib/portage/package/ebuild/doebuild.py | 8 ++--
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
>diff --git a/lib/_emerge/AbstractEbuildProcess.py
>b/lib/_emerge/AbstractEbuildProcess.py
>index bda0bd83f..ddf04e9b3 100644
>--- a/lib/_emerge/AbstractEbuildProcess.py
>+++ b/lib/_emerge/AbstractEbuildProcess.py
>@@ -1,4 +1,4 @@
>-# Copyright 1999-2018 Gentoo Foundation
>+# Copyright 1999-2019 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> 
> import errno
>@@ -22,6 +22,10 @@ from portage.util.futures import asyncio
> from portage.util._pty import _create_pty_or_pipe
> from portage.util import apply_secpass_permissions
> 
>+portage.proxy.lazyimport.lazyimport(globals(),
>+  'portage.package.ebuild.doebuild:_global_pid_phases',
>+)
>+
> class AbstractEbuildProcess(SpawnProcess):
> 
>   __slots__ = ('phase', 'settings',) + \
>@@ -30,7 +34,6 @@ class AbstractEbuildProcess(SpawnProcess):
> 
>   _phases_without_builddir = ('clean', 'cleanrm', 'depend', 'help',)
>   _phases_interactive_whitelist = ('config',)
>-  _phases_without_cgroup = ('preinst', 'postinst', 'prerm', 'postrm',
>'config')
> 
>   # Number of milliseconds to allow natural exit of the ebuild
>   # process after it has called the exit command via IPC. It
>@@ -71,7 +74,7 @@ class AbstractEbuildProcess(SpawnProcess):
>   # Check if the cgroup hierarchy is in place. If it's not, mount 
> it.
>   if (os.geteuid() == 0 and platform.system() == 'Linux'
>   and 'cgroup' in self.settings.features
>-  and self.phase not in 
>self._phases_without_cgroup):
>+  and self.phase not in _global_pid_phases):
>   cgroup_root = '/sys/fs/cgroup'
>   cgroup_portage = os.path.join(cgroup_root, 'portage')
> 
>diff --git a/lib/portage/package/ebuild/doebuild.py
>b/lib/portage/package/ebuild/doebuild.py
>index baebb9a27..f11923595 100644
>--- a/lib/portage/package/ebuild/doebuild.py
>+++ b/lib/portage/package/ebuild/doebuild.py
>@@ -1,4 +1,4 @@
>-# Copyright 2010-2018 Gentoo Authors
>+# Copyright 2010-2019 Gentoo Authors
> # Distributed under the terms of the GNU General Public License v2
> 
> from __future__ import unicode_literals
>@@ -110,6 +110,9 @@ _ipc_phases = frozenset([
>   "preinst", "postinst", "prerm", "postrm",
> ])
> 
>+# phases which execute in the global PID namespace
>+_global_pid_phases = frozenset(['preinst', 'postinst', 'prerm',
>'postrm', 'config'])
>+
> # phases in which networking access is allowed
> _networked_phases = frozenset([
>   # for VCS fetching
>@@ -153,7 +156,8 @@ def _doebuild_spawn(phase, settings,
>actionmap=None, **kwargs):
>   kwargs['networked'] = 'network-sandbox' not in settings.features or \
>   phase in _networked_phases or \
>   'network-sandbox' in settings['PORTAGE_RESTRICT'].split()
>-  kwargs['pidns'] = 'pid-sandbox' in settings.features
>+  kwargs['pidns'] = ('pid-sandbox' in settings.features and
>+  phase not in _global_pid_phases)
> 
>   if phase == 'depend':
>   kwargs['droppriv'] = 'userpriv' in settings.features

LGTM, thanks.
--
Best regards, 
Michał Górny



Re: [gentoo-portage-dev] [PATCH] lib/portage/dbapi/vartree: use dynamic report interval in _collision_protect

2019-01-07 Thread Michał Górny
On Mon, 2019-01-07 at 15:31 +0100, Fabian Groffen wrote:
> The reporting of files remaining can look somewhat odd since the report
> interval is hardcoded to be per 1000 objects.  Adjust this interval to
> be regular towards the end.  While at it, report percentage done.
> 
> Output before this patch:
> 
>  * checking 6111 files for package collisions
> 5111 files remaining ...
> 4111 files remaining ...
> 3111 files remaining ...
> 2111 files remaining ...
>  files remaining ...
> 111 files remaining ...
> 
> After:
> 
>  * checking 6158 files for package collisions
> 16% done, 5131 files remaining ...
> 33% done, 4104 files remaining ...
> 50% done, 3077 files remaining ...
> 66% done, 2050 files remaining ...
> 83% done, 1023 files remaining ...
> 

Not saying it's necessary but could we have some final message here?
I find it a bit weird to end at 83%.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] file_copy: handle EUCLEAN from copy_file_range (bug 674332)

2019-01-02 Thread Michał Górny
On Wed, 2019-01-02 at 14:29 -0800, Zac Medico wrote:
> EXT4 can set the errno to EUCLEAN for copy_file_range.
> 
> Bug: https://bugs.gentoo.org/674332
> Signed-off-by: Zac Medico 
> ---
>  src/portage_util_file_copy_reflink_linux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/portage_util_file_copy_reflink_linux.c 
> b/src/portage_util_file_copy_reflink_linux.c
> index 352342c06..1422232a5 100644
> --- a/src/portage_util_file_copy_reflink_linux.c
> +++ b/src/portage_util_file_copy_reflink_linux.c
> @@ -271,7 +271,7 @@ _reflink_linux_file_copy(PyObject *self, PyObject *args)
>  
>  if (copyfunc_ret < 0) {
>  error = errno;
> -if ((errno == EXDEV || errno == ENOSYS || errno == 
> EOPNOTSUPP) &&
> +if ((errno == EXDEV || errno == ENOSYS || errno == 
> EOPNOTSUPP || errno == EUCLEAN) &&
>  copyfunc == cfr_wrapper) {
>  /* Use sendfile instead of copy_file_range for
>   * cross-device copies, or when the copy_file_range

I'm pretty sure EUCLEAN is not portable to non-glibc/Linux systems.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] process.spawn: validate unshare calls (bug 673900)

2018-12-30 Thread Michał Górny
t:
> - # NEWNS = mount namespace
> - flags |= CLONE_NEWNS
> - if unshare_pid:
> - # we also need mount namespace for 
> slave /proc
> - flags |= CLONE_NEWPID | CLONE_NEWNS
> -
>   try:
> - if libc.unshare(flags) != 0:
> + errno_value = 
> _unshare_validator.instance.validate(unshare_flags)
> + if errno_value != 0:
> + writemsg("Unable to unshare: 
> %s\n" % (
> + 
> errno.errorcode.get(errno_value, '?')),
> + noiselevel=-1)
> + raise AttributeError('unshare')
> + if libc.unshare(unshare_flags) != 0:
>   writemsg("Unable to unshare: 
> %s\n" % (
>   
> errno.errorcode.get(ctypes.get_errno(), '?')),
>   noiselevel=-1)
> @@ -626,6 +635,79 @@ def _exec(binary, mycommand, opt_name, fd_pipes,
>   # And switch to the new process.
>   os.execve(binary, myargs, env)
>  
> +
> +class _unshare_validator(object):
> + """
> + In order to prevent failed unshare calls from corrupting the state
> + of an essential process, validate the relevant unshare call in a
> + short-lived subprocess. An unshare call is considered valid if it
> + successfully executes in a short-lived subprocess.
> + """
> +
> + instance = None
> +
> + def __init__(self):
> + self._results = {}
> +
> + def validate(self, flags):
> + """
> + Validate unshare with the given flags. Results are cached.
> +
> + @rtype: int
> + @returns: errno value, or 0 if no error occurred.
> + """
> +
> + if flags == 0:
> + return 0
> +
> + result = self._results.get(flags)
> + if result is not None:
> + return result
> +
> + filename = find_library("c")
> + if filename is None:
> + return errno.ENOTSUP
> +
> + libc = LoadLibrary(filename)
> + if libc is None:
> + return errno.ENOTSUP
> +
> + parent_pipe, subproc_pipe = multiprocessing.Pipe(duplex=False)
> +
> + proc = multiprocessing.Process(
> + target=self._validator_subproc,
> + args=(libc.unshare, flags, subproc_pipe))
> + proc.start()
> + subproc_pipe.close()
> +
> + result = parent_pipe.recv()
> + parent_pipe.close()
> + proc.join()
> +
> + self._results[flags] = result
> + return result
> +
> + def _validator_subproc(self, unshare, flags, subproc_pipe):
> + """
> + Perform validation, and send results to parent process.
> +
> + @param unshare: unshare function
> + @type unshare: callable
> + @param flags: unshare flags
> + @type flags: int
> + @param subproc_pipe: connection to parent process
> + @type subproc_pipe: multiprocessing.Connection
> + """
> + if unshare(flags) != 0:
> + subproc_pipe.send(ctypes.get_errno())
> + else:
> + subproc_pipe.send(0)
> + subproc_pipe.close()
> +
> +
> +_unshare_validator.instance = _unshare_validator()
> +
> +
>  def _setup_pipes(fd_pipes, close_fds=True, inheritable=None):
>   """Setup pipes for a forked process.
>  

Can't we detect the failure in normally spawned ebuild process,
and restart it with unsharing disabled?  Possibly cache the result to
avoid having to restart everything afterwards.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH 0/4] Rename PORT_LOGDIR{,_CLEAN} variables to PORTAGE_LOGDIR{,_CLEAN} Bug 668538

2018-12-17 Thread Michał Górny
On Mon, 2018-12-17 at 21:09 -0500, Alec Warner wrote:
> On Mon, Dec 17, 2018 at 10:51 AM Michał Górny  wrote:
> 
> > On Mon, 2018-12-17 at 15:44 +, M. J. Everitt wrote:
> > > On 17/12/18 12:54, Michał Górny wrote:
> > > > > Not only this, but as noted, unless you know the man pages for
> > 
> > portage and
> > > > > make.conf in order to recite them in your sleep, they are confusing
> > 
> > for
> > > > > users, as they do not necessarily follow an obvious pattern, and it
> > 
> > wasn't
> > > > > until I was attempting to debug something that I noticed that despite
> > > > > believing I had the correct settings in my make.conf (set over a
> > 
> > period of
> > > > > YEARS) they were in fact completely useless, and it wasn't until I
> > 
> > had to
> > > > > spend time with somebody debugging WTF was happening, that this
> > 
> > particular
> > > > > issue even became apparent...
> > > > 
> > > > I don't see how this is an argument for anything.  You have to read
> > > > the manual in order to know that such variable exists and what it
> > 
> > does.
> > > > Or, well, technically you don't since it's provided in
> > 
> > make.conf.example
> > > > already where you only need to uncomment it.
> > > > 
> > > > Either way, the variable name is trivial.  Even if you don't follow
> > > > the usual pattern of uncommenting it from make.conf.example or copying
> > > > from the manual, remembering it for the time needed to retype shoudln't
> > > > be a problem.
> > > > 
> > > > So, is this a solution to a real problem?  Or is it merely a half-
> > > > thought-out partial change that's going to require people to update
> > > > their configuration for no long-term benefit?  And then they will have
> > > > to update it again when someone decides to take another variable for
> > > > a spin.
> > > > 
> > > 
> > > In the case you hadn't noticed, clearly you haven't .. the change is
> > > backwards compatible.. that has already been thought out.
> > > 
> > > But you haven't actually looked at the patch have you, Michal ?
> > > 
> > 
> > I did look at it.  However, that doesn't change what I said.  Being
> > 'backwards compatible' does not change the fact that the old variable
> > becomes deprecated now.  Ergo, users are expected to eventually switch
> > to the new one.
> > 
> 
> So if we rewrote the patches to not deprecate the old one but to support
> both; how would that strike you?
> 

As a horrible idea, violating SPOT rule?

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH 0/4] Rename PORT_LOGDIR{,_CLEAN} variables to PORTAGE_LOGDIR{,_CLEAN} Bug 668538

2018-12-17 Thread Michał Górny
On Mon, 2018-12-17 at 15:44 +, M. J. Everitt wrote:
> On 17/12/18 12:54, Michał Górny wrote:
> > > Not only this, but as noted, unless you know the man pages for portage and
> > > make.conf in order to recite them in your sleep, they are confusing for
> > > users, as they do not necessarily follow an obvious pattern, and it wasn't
> > > until I was attempting to debug something that I noticed that despite
> > > believing I had the correct settings in my make.conf (set over a period of
> > > YEARS) they were in fact completely useless, and it wasn't until I had to
> > > spend time with somebody debugging WTF was happening, that this particular
> > > issue even became apparent...
> > 
> > I don't see how this is an argument for anything.  You have to read
> > the manual in order to know that such variable exists and what it does. 
> > Or, well, technically you don't since it's provided in make.conf.example
> > already where you only need to uncomment it.
> > 
> > Either way, the variable name is trivial.  Even if you don't follow
> > the usual pattern of uncommenting it from make.conf.example or copying
> > from the manual, remembering it for the time needed to retype shoudln't
> > be a problem.
> > 
> > So, is this a solution to a real problem?  Or is it merely a half-
> > thought-out partial change that's going to require people to update
> > their configuration for no long-term benefit?  And then they will have
> > to update it again when someone decides to take another variable for
> > a spin.
> > 
> 
> In the case you hadn't noticed, clearly you haven't .. the change is
> backwards compatible.. that has already been thought out.
> 
> But you haven't actually looked at the patch have you, Michal ?
> 

I did look at it.  However, that doesn't change what I said.  Being
'backwards compatible' does not change the fact that the old variable
becomes deprecated now.  Ergo, users are expected to eventually switch
to the new one.

Even if you don't care beyond changing this now and forgetting about it
afterwards, someone eventually will have to clean up the old variable
and actively force people to update.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH 0/4] Rename PORT_LOGDIR{,_CLEAN} variables to PORTAGE_LOGDIR{,_CLEAN} Bug 668538

2018-12-17 Thread Michał Górny
On Mon, 2018-12-17 at 05:12 +, M. J. Everitt wrote:
> On 15/12/18 08:55, Michał Górny wrote:
> > On Sat, 2018-12-15 at 02:25 +, M. J. Everitt wrote:
> > > This patchset aims to fix potential ambiguity and confusion between older 
> > > PORT_LOG* variables,
> > > and more recent PORTAGE_* variables - often leading to mysteriously 
> > > lacking logging due to
> > > incorrect variable names. Documented in Bug 668538; with thanks to Zac 
> > > for diagnosis, and
> > > solution assistance!
> > > 
> > 
> > Does 'often' actually affect more than one person?  Do you have any
> > evidence to support this?
> > 
> > Given that a lot of Portage variables don't have any prefix or sane
> > names, I dare say this one doesn't especially stand out.
> > 
> 
> Just a thought, but how about you apply your skill and wisdom to reviewing
> the patches, instead of wasting it on writing snide replies?
> Quite radical I know, but whadda ya think?!

You didn't answer my question.  However, given the level of aggression
in your reply, I'm going to presume I've caught you on a blatant lie
and that this problem affects exactly one person, yourself, and you are
making an unnecessary change to bend the world to your mistake.

> As it happens, I was going for consistency here, as that often reflects
> code quality, and you being a keen QA member, I'da thought perhaps you
> might have spotted this!

Are you?  Do you have any evidence to support that?  Because as far as I
can see (and it's even quite visible in your patch), none of
the variables in the group with 'PORT_LOGDIR' in it use 'PORTAGE_'
prefix.  So are you improving consistency in variable naming, or are you
replacing one inconsistency with another?

> Not only this, but as noted, unless you know the man pages for portage and
> make.conf in order to recite them in your sleep, they are confusing for
> users, as they do not necessarily follow an obvious pattern, and it wasn't
> until I was attempting to debug something that I noticed that despite
> believing I had the correct settings in my make.conf (set over a period of
> YEARS) they were in fact completely useless, and it wasn't until I had to
> spend time with somebody debugging WTF was happening, that this particular
> issue even became apparent...

I don't see how this is an argument for anything.  You have to read
the manual in order to know that such variable exists and what it does. 
Or, well, technically you don't since it's provided in make.conf.example
already where you only need to uncomment it.

Either way, the variable name is trivial.  Even if you don't follow
the usual pattern of uncommenting it from make.conf.example or copying
from the manual, remembering it for the time needed to retype shoudln't
be a problem.

So, is this a solution to a real problem?  Or is it merely a half-
thought-out partial change that's going to require people to update
their configuration for no long-term benefit?  And then they will have
to update it again when someone decides to take another variable for
a spin.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH 0/4] Rename PORT_LOGDIR{,_CLEAN} variables to PORTAGE_LOGDIR{,_CLEAN} Bug 668538

2018-12-15 Thread Michał Górny
On Sat, 2018-12-15 at 02:25 +, M. J. Everitt wrote:
> This patchset aims to fix potential ambiguity and confusion between older 
> PORT_LOG* variables,
> and more recent PORTAGE_* variables - often leading to mysteriously lacking 
> logging due to
> incorrect variable names. Documented in Bug 668538; with thanks to Zac for 
> diagnosis, and
> solution assistance!
> 

Does 'often' actually affect more than one person?  Do you have any
evidence to support this?

Given that a lot of Portage variables don't have any prefix or sane
names, I dare say this one doesn't especially stand out.

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part


[gentoo-portage-dev] [PATCH v3 2/3] Support FEATURES=pid-sandbox

2018-11-18 Thread Michał Górny
Supporting using PID namespace in order to isolate the ebuild processes
from host system, and make it possible to kill them all easily
(similarly to cgroups but easier to use).

Bug: https://bugs.gentoo.org/659582
Signed-off-by: Michał Górny 
---
 lib/portage/const.py   |  1 +
 lib/portage/package/ebuild/doebuild.py |  8 +++--
 lib/portage/process.py | 48 +++---
 man/make.conf.5|  7 
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/lib/portage/const.py b/lib/portage/const.py
index e0f93f7cc..ca66bc46e 100644
--- a/lib/portage/const.py
+++ b/lib/portage/const.py
@@ -174,6 +174,7 @@ SUPPORTED_FEATURES   = frozenset([
"notitles",
"parallel-fetch",
"parallel-install",
+   "pid-sandbox",
"prelink-checksums",
"preserve-libs",
"protect-owned",
diff --git a/lib/portage/package/ebuild/doebuild.py 
b/lib/portage/package/ebuild/doebuild.py
index e84a618d2..9917ac82c 100644
--- a/lib/portage/package/ebuild/doebuild.py
+++ b/lib/portage/package/ebuild/doebuild.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Gentoo Foundation
+# Copyright 2010-2018 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__ import unicode_literals
@@ -152,6 +152,7 @@ def _doebuild_spawn(phase, settings, actionmap=None, 
**kwargs):
kwargs['networked'] = 'network-sandbox' not in settings.features or \
phase in _networked_phases or \
'network-sandbox' in settings['PORTAGE_RESTRICT'].split()
+   kwargs['pidns'] = 'pid-sandbox' in settings.features
 
if phase == 'depend':
kwargs['droppriv'] = 'userpriv' in settings.features
@@ -1482,7 +1483,7 @@ def _validate_deps(mysettings, myroot, mydo, mydbapi):
 # XXX Issue: cannot block execution. Deadlock condition.
 def spawn(mystring, mysettings, debug=False, free=False, droppriv=False,
sesandbox=False, fakeroot=False, networked=True, ipc=True,
-   mountns=False, **keywords):
+   mountns=False, pidns=False, **keywords):
"""
Spawn a subprocess with extra portage-specific options.
Optiosn include:
@@ -1518,6 +1519,8 @@ def spawn(mystring, mysettings, debug=False, free=False, 
droppriv=False,
@type ipc: Boolean
@param mountns: Run this command inside mount namespace
@type mountns: Boolean
+   @param pidns: Run this command in isolated PID namespace
+   @type pidns: Boolean
@param keywords: Extra options encoded as a dict, to be passed to spawn
@type keywords: Dictionary
@rtype: Integer
@@ -1551,6 +1554,7 @@ def spawn(mystring, mysettings, debug=False, free=False, 
droppriv=False,
keywords['unshare_net'] = not networked
keywords['unshare_ipc'] = not ipc
keywords['unshare_mount'] = mountns
+   keywords['unshare_pid'] = pidns
 
if not networked and mysettings.get("EBUILD_PHASE") != 
"nofetch" and \
("network-sandbox-proxy" in features or "distcc" in 
features):
diff --git a/lib/portage/process.py b/lib/portage/process.py
index 46868f442..dee126c3c 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -223,7 +223,8 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None, 
returnpid=False,
   uid=None, gid=None, groups=None, umask=None, logfile=None,
   path_lookup=True, pre_exec=None,
   close_fds=(sys.version_info < (3, 4)), unshare_net=False,
-  unshare_ipc=False, unshare_mount=False, cgroup=None):
+  unshare_ipc=False, unshare_mount=False, unshare_pid=False,
+ cgroup=None):
"""
Spawns a given command.

@@ -264,6 +265,8 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None, 
returnpid=False,
@param unshare_mount: If True, mount namespace will be unshared and 
mounts will
be private to the namespace
@type unshare_mount: Boolean
+   @param unshare_pid: If True, PID ns will be unshared from the spawned 
process
+   @type unshare_pid: Boolean
@param cgroup: CGroup path to bind the process to
@type cgroup: String
 
@@ -332,7 +335,7 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None, 
returnpid=False,
# This caches the libc library lookup in the current
# process, so that it's only done once rather than
# for each child process.
-   if unshare_net or unshare_ipc or unshare_mount:
+   if unshare_net or unshare_ipc or unshare_mount or unshare_pid:
find_library("c")
 
# Force instantiation of portage.data.userpriv_groups before the
@@ -348,7 +351,8 @@ def spawn(mycommand, env={},

  1   2   3   4   5   6   7   8   9   >