[gentoo-commits] proj/portage:master commit in: lib/portage/tests/ebuild/, lib/portage/package/ebuild/

["Zac Medico" <zmedico@gentoo.org>]

commit:     52bc75a60b84d709712e91c68782f2f207bfce4e
Author:     Zac Medico <zmedico <AT> gentoo <DOT> org>
AuthorDate: Sun Oct 20 00:55:09 2019 +0000
Commit:     Zac Medico <zmedico <AT> gentoo <DOT> org>
CommitDate: Sun Oct 20 08:33:09 2019 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=52bc75a6

fetch: add force parameter (bug 697566)

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
Reviewed-by: Michał Górny <mgorny <AT> gentoo.org>
Signed-off-by: Zac Medico <zmedico <AT> gentoo.org>

 lib/portage/package/ebuild/fetch.py    | 22 +++++++++++++++----
 lib/portage/tests/ebuild/test_fetch.py | 40 +++++++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/lib/portage/package/ebuild/fetch.py b/lib/portage/package/ebuild/fetch.py
index 76e4636c2..05de12740 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,10 +455,23 @@ 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 force: 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.
 	"""
 
+	if force and digests:
+		# Since the force parameter can trigger unnecessary fetch when the
+		# digests match, do not allow force=True when digests are provided.
+		raise PortageException(_('fetch: force=True is not allowed when digests are provided'))
+
 	if not myuris:
 		return 1
 
@@ -878,7 +891,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:
 					# 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 +1055,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..538fb1754 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__'))
+				settings['FETCHCOMMAND'] = '"%s" "%s" "${URI}" "${DISTDIR}" "${FILE}"' % (BASH_BINARY, temp_fetchcommand)
+				settings.features.add('skiprocheck')
+				settings.features.remove('distlocks')
+				os.chmod(settings['DISTDIR'], 0o555)
+				try:
+					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'])