[PATCH] fetch: Use distfile fetching method to get layout.conf

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

[PATCH] fetch: Use distfile fetching method to get layout.conf

Michał Górny-5
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 <[hidden email]>
---
 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


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fetch: Use distfile fetching method to get layout.conf

Zac Medico-2
On 10/18/19 12:28 AM, Michał Górny wrote:

> 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 <[hidden email]>
> ---
>  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()
We'd need something more like mkstemp for real safety here. However, an
alternative is to use a constant path (containing the mirror name), and
assume that the fetch function updates files atomically. We can avoid
interference with concurrent processes if we let files remain in DISTDIR
(rather than unlink them).

>   try:
> + 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()
--
Thanks,
Zac


signature.asc (1000 bytes) Download Attachment