Add a force parameter which forces download even when a file already
exists in DISTDIR (and no digests are available to verify it). This
avoids the need to remove the existing file in advance, which makes
it possible to atomically replace the file and avoid interference
with concurrent processes. This is useful when using FETCHCOMMAND to
fetch a mirror's layout.conf file, for the purposes of bug 697566.

Bug: https://bugs.gentoo.org/697566
Signed-off-by: Zac Medico <zmed...@gentoo.org>
---
 lib/portage/package/ebuild/fetch.py    | 17 ++++++++---
 lib/portage/tests/ebuild/test_fetch.py | 40 ++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/lib/portage/package/ebuild/fetch.py 
b/lib/portage/package/ebuild/fetch.py
index 76e4636c2..e01b4df02 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -432,7 +432,7 @@ def get_mirror_url(mirror_url, filename, cache_path=None):
 
 def fetch(myuris, mysettings, listonly=0, fetchonly=0,
        locks_in_subdir=".locks", use_locks=1, try_mirrors=1, digests=None,
-       allow_missing_digests=True):
+       allow_missing_digests=True, force=False):
        """
        Fetch files to DISTDIR and also verify digests if they are available.
 
@@ -455,6 +455,14 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
        @param allow_missing_digests: Enable fetch even if there are no digests
                available for verification.
        @type allow_missing_digests: bool
+       @param bool: Force download, even when a file already exists in
+               DISTDIR. This is most useful when there are no digests 
available,
+               since otherwise download will be automatically forced if the
+               existing file does not match the available digests. Also, this
+               avoids the need to remove the existing file in advance, which
+               makes it possible to atomically replace the file and avoid
+               interference with concurrent processes.
+       @type force: bool
        @rtype: int
        @return: 1 if successful, 0 otherwise.
        """
@@ -878,7 +886,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
                                eout.quiet = mysettings.get("PORTAGE_QUIET") == 
"1"
                                match, mystat = _check_distfile(
                                        myfile_path, pruned_digests, eout, 
hash_filter=hash_filter)
-                               if match:
+                               if match and not (force and not orig_digests):
                                        # Skip permission adjustment for 
symlinks, since we don't
                                        # want to modify anything outside of 
the primary DISTDIR,
                                        # and symlinks typically point to 
PORTAGE_RO_DISTDIRS.
@@ -1042,10 +1050,11 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
                                                                
os.unlink(download_path)
                                                        except EnvironmentError:
                                                                pass
-                                       elif myfile not in mydigests:
+                                       elif not orig_digests:
                                                # We don't have a digest, but 
the file exists.  We must
                                                # assume that it is fully 
downloaded.
-                                               continue
+                                               if not force:
+                                                       continue
                                        else:
                                                if 
(mydigests[myfile].get("size") is not None
                                                                and 
mystat.st_size < mydigests[myfile]["size"]
diff --git a/lib/portage/tests/ebuild/test_fetch.py 
b/lib/portage/tests/ebuild/test_fetch.py
index f50fea0dd..f4eb0404b 100644
--- a/lib/portage/tests/ebuild/test_fetch.py
+++ b/lib/portage/tests/ebuild/test_fetch.py
@@ -119,10 +119,44 @@ class EbuildFetchTestCase(TestCase):
                                with open(foo_path, 'rb') as f:
                                        self.assertNotEqual(f.read(), 
distfiles['foo'])
 
-                               # Remove the stale file in order to forcefully 
update it.
-                               os.unlink(foo_path)
+                               # Use force=True to update the stale file.
+                               self.assertTrue(bool(run_async(fetch, foo_uri, 
settings, try_mirrors=False, force=True)))
 
-                               self.assertTrue(bool(run_async(fetch, foo_uri, 
settings, try_mirrors=False)))
+                               with open(foo_path, 'rb') as f:
+                                       self.assertEqual(f.read(), 
distfiles['foo'])
+
+                               # Test force=True with FEATURES=skiprocheck, 
using read-only DISTDIR.
+                               # FETCHCOMMAND is set to temporarily chmod +w 
DISTDIR. Note that
+                               # FETCHCOMMAND must perform atomic rename 
itself due to read-only
+                               # DISTDIR.
+                               with open(foo_path, 'wb') as f:
+                                       f.write(b'stale content\n')
+                               orig_fetchcommand = settings['FETCHCOMMAND']
+                               orig_distdir_mode = 
os.stat(settings['DISTDIR']).st_mode
+                               temp_fetchcommand = os.path.join(eubin, 
'fetchcommand')
+                               with open(temp_fetchcommand, 'w') as f:
+                                       f.write("""
+                                       set -e
+                                       URI=$1
+                                       DISTDIR=$2
+                                       FILE=$3
+                                       trap 'chmod a-w "${DISTDIR}"' EXIT
+                                       chmod ug+w "${DISTDIR}"
+                                       %s
+                                       mv -f "${DISTDIR}/${FILE}.__download__" 
"${DISTDIR}/${FILE}"
+                               """ % orig_fetchcommand.replace('${FILE}', 
'${FILE}.__download__'))
+                               try:
+                                       os.chmod(settings['DISTDIR'], 0o555)
+                                       settings['FETCHCOMMAND'] = '"%s" "%s" 
"${URI}" "${DISTDIR}" "${FILE}"' % (BASH_BINARY, temp_fetchcommand)
+                                       settings.features.add('skiprocheck')
+                                       settings.features.remove('distlocks')
+                                       self.assertTrue(bool(run_async(fetch, 
foo_uri, settings, try_mirrors=False, force=True)))
+                               finally:
+                                       settings['FETCHCOMMAND'] = 
orig_fetchcommand
+                                       os.chmod(settings['DISTDIR'], 
orig_distdir_mode)
+                                       settings.features.remove('skiprocheck')
+                                       settings.features.add('distlocks')
+                                       os.unlink(temp_fetchcommand)
 
                                with open(foo_path, 'rb') as f:
                                        self.assertEqual(f.read(), 
distfiles['foo'])
-- 
2.21.0


Reply via email to