[gentoo-portage-dev] [PATCH] fetch: add force parameter (bug 697566)

[Zac Medico]

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 <zmedico@g.o>
---
 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