From d8acfc679e198aa7b2529569f169172b2db47ade Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 28 2022 10:36:51 +0000 Subject: Download only the package header, not complete RPMs. Currently, the rpmdpelint downloads the complete RPMs just to load their headers to check for file conflicts. This is waste of time and resouces. This commit changes this to download only RPM headers. There is no function provided by the Python `rpm` module which would return the size of RPM header. The code therefore tries to download first N bytes of the RPM file and checks if the header is complete or not using the `hdrFromFdno` RPM funtion. As the header size can be very different from package to package, it tries to download first 100KB and if header is not complete, it fallbacks to 1MB and 5MB. If that is not enough, the final fallback downloads whole RPM file. This strategy still wastes some bandwidth, because we are downloading first N bytes repeatedly, but because header of typical RPM fits into first 100KB usually and because the RPM data is much bigger than what we download repeatedly, it saves lot of time and bandwidth overall. Checksums cannot be checked by this method, because checksums work only when complete RPM file is downloaded. Signed-off-by: Jan Kaluza --- diff --git a/rpmdeplint/__init__.py b/rpmdeplint/__init__.py index 5796338..2e7172f 100644 --- a/rpmdeplint/__init__.py +++ b/rpmdeplint/__init__.py @@ -165,17 +165,14 @@ class DependencyAnalyzer(object): def __exit__(self, type, value, tb): return - def download_package(self, solvable): + def download_package_header(self, solvable): if solvable in self.solvables: # It's a package under test, nothing to download return solvable.lookup_location()[0] href = solvable.lookup_location()[0] baseurl = solvable.lookup_str(self.pool.str2id('solvable:mediabase')) repo = self.repos_by_name[solvable.repo.name] - checksum = solvable.lookup_checksum(self.pool.str2id('solvable:checksum')) - return repo.download_package(href, baseurl, - checksum_type=checksum.typestr(), - checksum=checksum.hex()) + return repo.download_package_header(href, baseurl) def try_to_install_all(self): """ @@ -320,7 +317,7 @@ class DependencyAnalyzer(object): ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) left_hdr = ts.hdrFromFdno(open(left.lookup_location()[0], 'rb')) - right_hdr = ts.hdrFromFdno(open(self.download_package(right), 'rb')) + right_hdr = ts.hdrFromFdno(open(self.download_package_header(right), 'rb')) left_files = rpm.files(left_hdr) right_files = rpm.files(right_hdr) if left_files[filename].matches(right_files[filename]): @@ -350,7 +347,7 @@ class DependencyAnalyzer(object): ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) left_hdr = ts.hdrFromFdno(open(left.lookup_location()[0], 'rb')) - right_hdr = ts.hdrFromFdno(open(self.download_package(right), 'rb')) + right_hdr = ts.hdrFromFdno(open(self.download_package_header(right), 'rb')) left_fi = rpm.fi(left_hdr) try: while left_fi.FN() != filename: diff --git a/rpmdeplint/repodata.py b/rpmdeplint/repodata.py index db5ff94..fdd8d50 100644 --- a/rpmdeplint/repodata.py +++ b/rpmdeplint/repodata.py @@ -20,6 +20,7 @@ import glob import time from six.moves import configparser import librepo +import rpm logger = logging.getLogger(__name__) requests_session = requests.Session() @@ -274,26 +275,86 @@ class Repo(object): os.unlink(temp_path) raise - def download_package(self, location, baseurl, checksum_type, checksum): + def _is_header_complete(self, local_path): + """ + Returns `True` if the RPM file `local_path` has complete RPM header. + """ + try: + with open(local_path, 'rb') as f: + try: + ts = rpm.TransactionSet() + ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) + + # Supress the RPM error message printed to stderr in case + # the header is not complete. Set the log verbosity to CRIT + # to achieve that. This way the critical errors are still + # logged, but the expected "bad header" error is not. + rpm.setVerbosity(rpm.RPMLOG_CRIT) + ts.hdrFromFdno(f) + return True + except rpm.error as e: + return False + finally: + # Revert back to RPMLOG_ERR. + rpm.setVerbosity(rpm.RPMLOG_ERR) + except FileNotFoundError: + return False + + def download_package_header(self, location, baseurl): + """ + Downloads the package header so it can be parsed by `hdrFromFdno`. + + There is no function provided by the Python `rpm` module which would + return the size of RPM header. This method therefore tries to download + first N bytes of the RPM file and checks if the header is complete or + not using the `hdrFromFdno` RPM funtion. + + As the header size can be very different from package to package, it + tries to download first 100KB and if header is not complete, it + fallbacks to 1MB and 5MB. If that is not enough, the final fallback + downloads whole RPM file. + + This strategy still wastes some bandwidth, because we are downloading + first N bytes repeatedly, but because header of typical RPM fits + into first 100KB usually and because the RPM data is much bigger than + what we download repeatedly, it saves lot of time and bandwidth overall. + + Checksums cannot be checked by this method, because checksums work + only when complete RPM file is downloaded. + """ if self.librepo_handle.local: - local_path = os.path.join(self._root_path, location) logger.debug('Using package %s from local filesystem directly', local_path) return local_path + + # Check if we already downloaded this file and return it if so. + local_path = os.path.join(self._root_path, os.path.basename(location)) + if self._is_header_complete(local_path): + logger.debug("Using already downloaded package from %s", local_path) + return local_path + logger.debug('Loading package %s from repo %s', location, self.name) - target = librepo.PackageTarget(location, - base_url=baseurl, - checksum_type=librepo.checksum_str_to_type(checksum_type), - checksum=checksum, - dest=self._root_path, - handle=self.librepo_handle) - librepo.download_packages([target]) - if target.err and target.err == 'Already downloaded': - logger.debug('Already downloaded %s', target.local_path) - elif target.err: - raise PackageDownloadError('Failed to download %s from repo %s: %s' - % (location, self.name, target.err)) - else: - logger.debug('Saved as %s', target.local_path) + for byterangeend in [100000, 1000000, 5000000, 0]: + target = librepo.PackageTarget(location, + base_url=baseurl, + dest=self._root_path, + handle=self.librepo_handle, + byterangestart=0, + byterangeend=byterangeend + ) + + if byterangeend: + logger.debug('Download first %s bytes of %s', byterangeend, location) + else: + logger.debug('Download %s', location) + librepo.download_packages([target]) + if target.err and target.err != 'Already downloaded': + raise PackageDownloadError('Failed to download %s from repo %s: %s' + % (location, self.name, target.err)) + else: + if self._is_header_complete(target.local_path): + break + + logger.debug('Saved as %s', target.local_path) return target.local_path @property diff --git a/rpmdeplint/tests/test_dependency_analyzer.py b/rpmdeplint/tests/test_dependency_analyzer.py index e5b5b5b..b8ea996 100644 --- a/rpmdeplint/tests/test_dependency_analyzer.py +++ b/rpmdeplint/tests/test_dependency_analyzer.py @@ -9,41 +9,46 @@ from unittest import TestCase from rpmdeplint import DependencyAnalyzer from rpmdeplint.repodata import Repo import os -import rpmfluff + +try: + from rpmfluff import SimpleRpmBuild + from rpmfluff import YumRepoBuild +except ImportError: + from rpmfluff.rpmbuild import SimpleRpmBuild + from rpmfluff.yumrepobuild import YumRepoBuild class TestDependencyAnalyzer(TestCase): def test_repos(self): - lemon = rpmfluff.SimpleRpmBuild('lemon', '1', '3', ['noarch']) + lemon = SimpleRpmBuild('lemon', '1', '3', ['noarch']) lemon.add_provides('lemon-juice') lemon.add_provides('lemon-zest') - self.addCleanup(shutil.rmtree, lemon.get_base_dir()) - peeler = rpmfluff.SimpleRpmBuild('peeler', '4', '0', ['x86_64']) - self.addCleanup(shutil.rmtree, peeler.get_base_dir()) - cinnamon = rpmfluff.SimpleRpmBuild('cinnamon', '3', '0', ['noarch']) - self.addCleanup(shutil.rmtree, cinnamon.get_base_dir()) - apple_pie = rpmfluff.SimpleRpmBuild('apple-pie', '1.9', '1', ['x86_64']) + self.addCleanup(lemon.clean) + peeler = SimpleRpmBuild('peeler', '4', '0', ['x86_64']) + self.addCleanup(peeler.clean) + cinnamon = SimpleRpmBuild('cinnamon', '3', '0', ['noarch']) + self.addCleanup(cinnamon.clean) + apple_pie = SimpleRpmBuild('apple-pie', '1.9', '1', ['x86_64']) apple_pie.add_requires('apple-lib') apple_pie.add_requires('lemon-juice') apple_pie.add_requires('cinnamon >= 2.0') - self.addCleanup(shutil.rmtree, apple_pie.get_base_dir()) - base_1_repo = rpmfluff.YumRepoBuild([lemon, peeler, cinnamon, apple_pie]) + self.addCleanup(apple_pie.clean) + base_1_repo = YumRepoBuild([lemon, peeler, cinnamon, apple_pie]) base_1_repo.make('x86_64', 'noarch') - self.addCleanup(shutil.rmtree, base_1_repo.repoDir) - apple = rpmfluff.SimpleRpmBuild('apple', '4.9', '3', ['x86_64']) + apple = SimpleRpmBuild('apple', '4.9', '3', ['x86_64']) apple.add_provides('apple-lib') apple.add_requires('peeler') apple.add_requires('lemon-juice') apple.make() - self.addCleanup(shutil.rmtree, apple.get_base_dir()) - lemon_meringue_pie = rpmfluff.SimpleRpmBuild('lemon-meringue-pie', '1', '0', ['x86_64']) + self.addCleanup(apple.clean) + lemon_meringue_pie = SimpleRpmBuild('lemon-meringue-pie', '1', '0', ['x86_64']) lemon_meringue_pie.add_requires('lemon-zest') lemon_meringue_pie.add_requires('lemon-juice') lemon_meringue_pie.add_requires('egg-whites') lemon_meringue_pie.add_requires('egg-yolks') lemon_meringue_pie.add_requires('sugar') lemon_meringue_pie.make() - self.addCleanup(shutil.rmtree, lemon_meringue_pie.get_base_dir()) + self.addCleanup(lemon_meringue_pie.clean) da = DependencyAnalyzer( repos=[Repo(repo_name='base_1', baseurl=base_1_repo.repoDir)], @@ -56,15 +61,14 @@ class TestDependencyAnalyzer(TestCase): self.assertEqual(['nothing provides egg-whites needed by lemon-meringue-pie-1-0.x86_64'], dependency_set.package_dependencies['lemon-meringue-pie-1-0.x86_64']['problems']) - eggs = rpmfluff.SimpleRpmBuild('eggs', '1', '3', ['noarch']) + eggs = SimpleRpmBuild('eggs', '1', '3', ['noarch']) eggs.add_provides('egg-whites') eggs.add_provides('egg-yolks') - self.addCleanup(shutil.rmtree, eggs.get_base_dir()) - sugar = rpmfluff.SimpleRpmBuild('sugar', '4', '0', ['x86_64']) - self.addCleanup(shutil.rmtree, sugar.get_base_dir()) - base_2_repo = rpmfluff.YumRepoBuild([eggs, sugar]) + self.addCleanup(eggs.clean) + sugar = SimpleRpmBuild('sugar', '4', '0', ['x86_64']) + self.addCleanup(sugar.clean) + base_2_repo = YumRepoBuild([eggs, sugar]) base_2_repo.make('x86_64', 'noarch') - self.addCleanup(shutil.rmtree, base_2_repo.repoDir) da = DependencyAnalyzer( repos=[Repo(repo_name='base_1', baseurl=base_1_repo.repoDir),