pym/portage/package/ebuild/fetch.py: Factor out _get_uris
authorW. Trevor King <wking@tremily.us>
Sun, 19 Jan 2014 02:29:15 +0000 (18:29 -0800)
committerW. Trevor King <wking@tremily.us>
Mon, 20 Jan 2014 02:55:24 +0000 (18:55 -0800)
commit2b4c89a8be60c042d9afd2fb4151b9736afe36ac
tree183fc7453e50c7f0a6cc36ec69c552ca17b1bc9d
parentdeec09e7a8e0f797e55a43d30858413006f34274
pym/portage/package/ebuild/fetch.py: Factor out _get_uris

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

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

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

I don't know if this is useful enough to motivate the uglier
_expandmirror return values, but I'll kick that can down the road for
now.

There was some discussion on the list [1] about the defaults in:

  def _get_uris(uris, settings, custom_mirrors=(), locations=()):

but I prefer this to Alec's floated:

  def _get_uris(uris, settings, custom_mirrors=None, locations=None):
      if not custom_mirrors:
          custom_mirrors = ()
      if not locations:
          locations = ()

Which accomplishes the same thing with less clarity ;).  My default
values are tuples (and not mutable lists) to make it extra-obvious
that we're relying on anything crazy like mutating our default values
;).

There was also discussion about whether the ugly settings object
should be passed to _get_uris, or whether the appropriate settings
should be extracted first and then passed through [2].  I've passed
settings through, because I'd prefer to have the uglyness handled in
helper functions.  The fetch() function is what most folks will care
about, so we want to keep that as clean as possible.  If the helper
functions have to suffer a bit for a cleaner fetch(), so be it.

[1]: http://thread.gmane.org/gmane.linux.gentoo.portage.devel/4002/focus=4041
[2]: http://thread.gmane.org/gmane.linux.gentoo.portage.devel/4002/focus=4042
pym/portage/package/ebuild/fetch.py
pym/portage/tests/ebuild/test_fetch.py