From 516fc709fb5441c39d7dd52075877a1e618ab620 Mon Sep 17 00:00:00 2001 From: Michael Tremer Date: Sun, 4 Mar 2012 15:29:06 +0100 Subject: [PATCH] PBS: Add more verbosity to build logs. When builds have crashed earlier they were not always traces about the reason in the log. This commit tries to fix that. --- python/pakfire/actions.py | 4 +-- python/pakfire/base.py | 8 ++--- python/pakfire/builder.py | 2 ++ python/pakfire/errors.py | 3 +- python/pakfire/packages/solv.py | 4 +-- python/pakfire/repository/remote.py | 15 ++++---- python/pakfire/transaction.py | 55 ++++++++++++++++++----------- 7 files changed, 55 insertions(+), 36 deletions(-) diff --git a/python/pakfire/actions.py b/python/pakfire/actions.py index 37d446f39..e508f135a 100644 --- a/python/pakfire/actions.py +++ b/python/pakfire/actions.py @@ -63,11 +63,11 @@ class Action(object): return self.type in ("install", "reinstall", "upgrade", "downgrade", "change",) \ and not isinstance(self.pkg, packages.BinaryPackage) - def download(self, text): + def download(self, text, logger=None): if not self.needs_download: return - self.pkg = self.pkg.download(text) + self.pkg = self.pkg.download(text, logger=logger) def run(self): raise NotImplementedError diff --git a/python/pakfire/base.py b/python/pakfire/base.py index 2a7d2263c..5da012e78 100644 --- a/python/pakfire/base.py +++ b/python/pakfire/base.py @@ -254,7 +254,7 @@ class Pakfire(object): t.dump(logger=logger) # Run the transaction. - t.run() + t.run(logger=logger) def localinstall(self, files, yes=None, allow_uninstall=False): repo_name = repo_desc = "localinstall" @@ -296,12 +296,12 @@ class Pakfire(object): if not t.cli_yesno(): return elif yes: - t.dump() + t.dump(logger=logger) else: return # If okay, run the transcation. - t.run() + t.run(logger=logger) finally: # Remove the temporary copy of the repository we have created earlier. @@ -461,7 +461,7 @@ class Pakfire(object): return # Run the transaction. - t.run() + t.run(logger=logger) def downgrade(self, pkgs, allow_vendorchange=False, allow_archchange=False): assert pkgs diff --git a/python/pakfire/builder.py b/python/pakfire/builder.py index 94da4cf9e..2f42d01cf 100644 --- a/python/pakfire/builder.py +++ b/python/pakfire/builder.py @@ -677,6 +677,8 @@ class BuildEnviron(object): self.do(" ".join(build_command), logger=self.log) except Error: + self.log.error(_("Build failed."), exc_info=True) + raise BuildError, _("The build command failed. See logfile for details.") # Perform install test. diff --git a/python/pakfire/errors.py b/python/pakfire/errors.py index 2daa7283d..fa9df78c8 100644 --- a/python/pakfire/errors.py +++ b/python/pakfire/errors.py @@ -58,7 +58,8 @@ class DependencyError(Error): message = _("One or more dependencies could not been resolved.") class DownloadError(Error): - pass + message = _("An error occured when pakfire tried to download files.") + class FileError(Error): pass diff --git a/python/pakfire/packages/solv.py b/python/pakfire/packages/solv.py index c120ce69f..9bbbc2a3d 100644 --- a/python/pakfire/packages/solv.py +++ b/python/pakfire/packages/solv.py @@ -227,9 +227,9 @@ class SolvPackage(base.Package): if path and self.repo.cache.verify(path, self.hash1): return file.BinaryPackage(self.pakfire, self.repo, path) - def download(self, text=""): + def download(self, text="", logger=None): if not self.repo.local: - self.repo.download(self, text=text) + self.repo.download(self, text=text, logger=logger) return self.get_from_cache() diff --git a/python/pakfire/repository/remote.py b/python/pakfire/repository/remote.py index e707c59d1..370be5921 100644 --- a/python/pakfire/repository/remote.py +++ b/python/pakfire/repository/remote.py @@ -70,10 +70,13 @@ class RepositorySolv(base.RepositoryFactory): return priority - def download(self, pkg, text=""): + def download(self, pkg, text="", logger=None): """ Downloads 'filename' from repository and returns the local filename. """ + if logger is None: + logger = log + filename, hash1 = pkg.filename, pkg.hash1 # Marker, if we need to download the package. @@ -83,7 +86,7 @@ class RepositorySolv(base.RepositoryFactory): # Check if file already exists in cache. if self.cache.exists(cache_filename): - log.debug("File exists in cache: %s" % filename) + logger.debug("File exists in cache: %s" % filename) # If the file does already exist, we check if the hash1 matches. if hash1 and self.cache.verify(cache_filename, hash1): @@ -104,7 +107,7 @@ class RepositorySolv(base.RepositoryFactory): filename = str(filename) while download: - log.debug("Going to download %s" % filename) + logger.debug("Going to download %s" % filename) # If we are in offline mode, we cannot download any files. if self.pakfire.offline and not self.baseurl.startswith("file://"): @@ -125,11 +128,11 @@ class RepositorySolv(base.RepositoryFactory): o.close() if self.cache.verify(cache_filename, hash1): - log.debug("Successfully downloaded %s (%s)." % (filename, hash1)) + logger.debug("Successfully downloaded %s (%s)." % (filename, hash1)) break - log.warning(_("The checksum of the downloaded file did not match.")) - log.warning(_("Trying an other mirror.")) + logger.warning(_("The checksum of the downloaded file did not match.")) + logger.warning(_("Trying an other mirror.")) # Go to the next mirror. grabber.increment_mirror() diff --git a/python/pakfire/transaction.py b/python/pakfire/transaction.py index f18f9453c..5f5076d1d 100644 --- a/python/pakfire/transaction.py +++ b/python/pakfire/transaction.py @@ -80,20 +80,23 @@ class TransactionCheck(object): return True - def print_errors(self): + def print_errors(self, logger=None): + if logger is None: + logger = logging.getLogger("pakfire") + for name, files in sorted(self.error_files.items()): assert len(files) >= 2 pkgs = [f.pkg.friendly_name for f in files] if len(files) == 2: - log.critical( + logger.critical( _("file %s from %s conflicts with file from package %s") % \ (name, pkgs[0], pkgs[1]) ) elif len(files) >= 3: - log.critical( + logger.critical( _("file %s from %s conflicts with files from %s") % \ (name, pkgs[0], i18n.list(pkgs[1:])) ) @@ -102,8 +105,7 @@ class TransactionCheck(object): if mp.space_left >= 0: continue - print util.format_size(mp.free), util.format_size(mp.disk_usage) - log.critical(_("There is not enough space left on %(name)s. Need at least %(size)s to perform transaction.") \ + logger.critical(_("There is not enough space left on %(name)s. Need at least %(size)s to perform transaction.") \ % { "name" : mp.path, "size" : util.format_size(mp.space_needed) }) def load_filelist(self): @@ -264,7 +266,10 @@ class Transaction(object): def downloads(self): return sorted([a for a in self.actions if a.needs_download]) - def download(self): + def download(self, logger=None): + if logger is None: + logger = logging.getLogger("pakfire") + # Get all download actions as a list. downloads = [d for d in self.downloads] downloads.sort() @@ -288,19 +293,19 @@ class Transaction(object): raise DownloadError, _("Not enough space to download %s of packages.") \ % util.format_size(download_size) - log.info(_("Downloading packages:")) + logger.info(_("Downloading packages:")) time_start = time.time() i = 0 for action in downloads: i += 1 - action.download(text="(%d/%d): " % (i, len(downloads))) + action.download(text="(%d/%d): " % (i, len(downloads)), logger=logger) # Write an empty line to the console when there have been any downloads. width, height = util.terminal_size() # Print a nice line. - log.info("-" * width) + logger.info("-" * width) # Format and calculate download information. time_stop = time.time() @@ -313,8 +318,8 @@ class Transaction(object): line = "%s | %5sB %s " % \ (download_speed, download_size, download_time) line = " " * (width - len(line)) + line - log.info(line) - log.info("") + logger.info(line) + logger.info("") def dump_pkg(self, pkg): ret = [] @@ -341,7 +346,7 @@ class Transaction(object): def dump(self, logger=None): if logger is None: - logger = log + logger = logging.getLogger("pakfire") width = 80 line = "=" * width @@ -394,8 +399,11 @@ class Transaction(object): return util.ask_user(_("Is this okay?")) - def check(self): - log.info(_("Running Transaction Test")) + def check(self, logger=None): + if logger is None: + logger = logging.getLogger("pakfire") + + logger.info(_("Running Transaction Test")) # Initialize the check object. check = TransactionCheck(self.pakfire, self) @@ -407,33 +415,38 @@ class Transaction(object): raise if check.successful: - log.info(_("Transaction Test Succeeded")) + logger.info(_("Transaction Test Succeeded")) return # In case of an unsuccessful transaction test, we print the error # and raise TransactionCheckError. - check.print_errors() + check.print_errors(logger=logger) raise TransactionCheckError, _("Transaction test was not successful") - def run(self): + def run(self, logger=None): assert not self.__need_sort, "Did you forget to sort the transaction?" + if logger is None: + logger = logging.getLogger("pakfire") + # Download all packages. + # (don't add logger here because I do not want to see downloads + # in the build logs on the build service) self.download() # Run the transaction test - self.check() + self.check(logger=logger) - log.info(_("Running transaction")) + logger.info(_("Running transaction")) # Run all actions in order and catch all kinds of ActionError. for action in self.actions: try: action.run() except ActionError, e: - log.error("Action finished with an error: %s - %s" % (action, e)) + logger.error("Action finished with an error: %s - %s" % (action, e)) - log.info("") + logger.info("") # Commit repository metadata. self.local.commit() -- 2.39.5