fetch: minimal skiprocheck fixes (bug 220533)

Fix cases here fetch assumes that DISTDIR is writable when it's actually
read-only. This preserves old behavior which allowed users to override
FETCHCOMMAND to fetch files on a remote system, even though DISTDIR is
locally mounted in read-only mode.

Bug: https://bugs.gentoo.org/220533
Fixes: ebbde237d33e ("fetch: atomic downloads (bug 175612)")
Signed-off-by: Zac Medico <zmedico@gentoo.org>

2 files changed, 45 insertions, 15 deletions

diff --git a/lib/portage/package/ebuild/fetch.py b/lib/portage/package/ebuild/fetch.py
index cd204b755..1d5e07260 100644
--- a/lib/portage/package/ebuild/fetch.py
+++ b/lib/portage/package/ebuild/fetch.py
@@ -529,11 +529,12 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 	if listonly or ("distlocks" not in features):
 		use_locks = 0
 
+	distdir_writable = os.access(mysettings["DISTDIR"], os.W_OK)
 	fetch_to_ro = 0
 	if "skiprocheck" in features:
 		fetch_to_ro = 1
 
-	if not os.access(mysettings["DISTDIR"],os.W_OK) and fetch_to_ro:
+	if not distdir_writable and fetch_to_ro:
 		if use_locks:
 			writemsg(colorize("BAD",
 				_("!!! For fetching to a read-only filesystem, "
@@ -613,8 +614,11 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 	for myfile, myuri in file_uri_tuples:
 		if myfile not in filedict:
 			filedict[myfile]=[]
-			mirror_cache = os.path.join(mysettings["DISTDIR"],
-					".mirror-cache.json")
+			if distdir_writable:
+				mirror_cache = os.path.join(mysettings["DISTDIR"],
+						".mirror-cache.json")
+			else:
+				mirror_cache = None
 			for l in locations:
 				filedict[myfile].append(functools.partial(
 					get_mirror_url, l, myfile, mirror_cache))
@@ -790,7 +794,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 				pruned_digests["size"] = size
 
 		myfile_path = os.path.join(mysettings["DISTDIR"], myfile)
-		download_path = myfile_path + _download_suffix
+		download_path = myfile_path if fetch_to_ro else myfile_path + _download_suffix
 		has_space = True
 		has_space_superuser = True
 		file_lock = None
@@ -1058,7 +1062,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 										"File renamed to '%s'\n\n") % \
 										temp_filename, noiselevel=-1)
 							else:
-								_movefile(download_path, myfile_path, mysettings=mysettings)
+								if not fetch_to_ro:
+									_movefile(download_path, myfile_path, mysettings=mysettings)
 								eout = EOutput()
 								eout.quiet = \
 									mysettings.get("PORTAGE_QUIET", None) == "1"
@@ -1177,7 +1182,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 							del e
 							fetched = 0
 						else:
-							if mystat.st_size < fetch_resume_size:
+							if distdir_writable and mystat.st_size < fetch_resume_size:
 								writemsg(_(">>> Deleting distfile with size "
 									"%d (smaller than " "PORTAGE_FETCH_RESU"
 									"ME_MIN_SIZE)\n") % mystat.st_size)
@@ -1315,13 +1320,14 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 										(reason[1], reason[2]), noiselevel=-1)
 									if reason[0] == _("Insufficient data for checksum verification"):
 										return 0
-									temp_filename = \
-										_checksum_failure_temp_file(
-											mysettings, mysettings["DISTDIR"],
-											os.path.basename(download_path))
-									writemsg_stdout(_("Refetching... "
-										"File renamed to '%s'\n\n") % \
-										temp_filename, noiselevel=-1)
+									if distdir_writable:
+										temp_filename = \
+											_checksum_failure_temp_file(
+												mysettings, mysettings["DISTDIR"],
+												os.path.basename(download_path))
+										writemsg_stdout(_("Refetching... "
+											"File renamed to '%s'\n\n") % \
+											temp_filename, noiselevel=-1)
 									fetched=0
 									checksum_failure_count += 1
 									if checksum_failure_count == \
@@ -1338,7 +1344,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 										checksum_failure_max_tries:
 										break
 								else:
-									_movefile(download_path, myfile_path, mysettings=mysettings)
+									if not fetch_to_ro:
+										_movefile(download_path, myfile_path, mysettings=mysettings)
 									eout = EOutput()
 									eout.quiet = mysettings.get("PORTAGE_QUIET", None) == "1"
 									if digests:
@@ -1349,7 +1356,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
 									break
 					else: # no digests available
 						if not myret:
-							_movefile(download_path, myfile_path, mysettings=mysettings)
+							if not fetch_to_ro:
+								_movefile(download_path, myfile_path, mysettings=mysettings)
 							fetched=2
 							break
 						elif mydigests!=None:
diff --git a/lib/portage/tests/ebuild/test_fetch.py b/lib/portage/tests/ebuild/test_fetch.py
index f2254c468..ac45f8720 100644
--- a/lib/portage/tests/ebuild/test_fetch.py
+++ b/lib/portage/tests/ebuild/test_fetch.py
@@ -9,6 +9,7 @@ import tempfile
 
 import portage
 from portage import shutil, os
+from portage.const import BASH_BINARY
 from portage.tests import TestCase
 from portage.tests.resolver.ResolverPlayground import ResolverPlayground
 from portage.tests.util.test_socks5 import AsyncHTTPServer
@@ -59,15 +60,19 @@ class EbuildFetchTestCase(TestCase):
 
 			playground = ResolverPlayground(ebuilds=ebuilds_subst, distfiles=distfiles)
 			ro_distdir = tempfile.mkdtemp()
+			eubin = os.path.join(playground.eprefix, "usr", "bin")
 			try:
 				fetchcommand = portage.util.shlex_split(playground.settings['FETCHCOMMAND'])
 				fetch_bin = portage.process.find_binary(fetchcommand[0])
 				if fetch_bin is None:
 					self.skipTest('FETCHCOMMAND not found: {}'.format(playground.settings['FETCHCOMMAND']))
+				os.symlink(fetch_bin, os.path.join(eubin, os.path.basename(fetch_bin)))
 				resumecommand = portage.util.shlex_split(playground.settings['RESUMECOMMAND'])
 				resume_bin = portage.process.find_binary(resumecommand[0])
 				if resume_bin is None:
 					self.skipTest('RESUMECOMMAND not found: {}'.format(playground.settings['RESUMECOMMAND']))
+				if resume_bin != fetch_bin:
+					os.symlink(resume_bin, os.path.join(eubin, os.path.basename(resume_bin)))
 				root_config = playground.trees[playground.eroot]['root_config']
 				portdb = root_config.trees["porttree"].dbapi
 				settings = config(clone=playground.settings)
@@ -228,6 +233,23 @@ class EbuildFetchTestCase(TestCase):
 								self.assertEqual(f.read(), distfiles[k])
 					finally:
 						settings['PORTAGE_FETCH_RESUME_MIN_SIZE'] = orig_resume_min_size
+
+					# Test readonly DISTDIR + skiprocheck, with FETCHCOMMAND set to temporarily chmod DISTDIR
+					orig_fetchcommand = settings['FETCHCOMMAND']
+					orig_distdir_mode = os.stat(settings['DISTDIR']).st_mode
+					for k in settings['AA'].split():
+						os.unlink(os.path.join(settings['DISTDIR'], k))
+					try:
+						os.chmod(settings['DISTDIR'], 0o555)
+						settings['FETCHCOMMAND'] = '"%s" -c "chmod ug+w \\"${DISTDIR}\\"; %s; status=\\$?; chmod a-w \\"${DISTDIR}\\"; exit \\$status"' % (BASH_BINARY, orig_fetchcommand.replace('"', '\\"'))
+						settings.features.add('skiprocheck')
+						settings.features.remove('distlocks')
+						self.assertEqual(loop.run_until_complete(async_fetch(pkg, ebuild_path)), 0)
+					finally:
+						settings['FETCHCOMMAND'] = orig_fetchcommand
+						os.chmod(settings['DISTDIR'], orig_distdir_mode)
+						settings.features.remove('skiprocheck')
+						settings.features.add('distlocks')
 			finally:
 				shutil.rmtree(ro_distdir)
 				playground.cleanup()