commit: 6b5889afb1e80bc673ce782e65fc0f49ee7d0908 Author: Zac Medico <zmedico <AT> gentoo <DOT> org> AuthorDate: Sun Oct 13 22:13:18 2019 +0000 Commit: Zac Medico <zmedico <AT> gentoo <DOT> org> CommitDate: Mon Oct 14 19:46:16 2019 +0000 URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=6b5889af
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 <AT> gentoo.org> lib/portage/package/ebuild/fetch.py | 38 ++++++++++++++++++++-------------- lib/portage/tests/ebuild/test_fetch.py | 22 ++++++++++++++++++++ 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()