From 91d396ff56020315443359b1f820f64da3f0f26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrer=20=C3=96zen?= Date: Sat, 13 Aug 2005 21:51:46 +0000 Subject: [PATCH] Evet, kocaman bir patch. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test/beta-light.sh, pisi build/install gzip ve run.py specfile denedim, biraz daha test/deneme lazım. Artık spec klaslarında .verify() yerine .has_errors() metotları var. Bunlar hata yoksa None, varsa bir hata listesi döndürüyorlar. Liste stringlerden oluşuyor. Amaç specfile'larda hata varsa, tüm bulunan hataları ve bunlara ait bilgileri ekrana basabilmek. Denemek için herhangi bi pspec'ten kritik bikaç tagi çıkarıp build etmeyi deneyin. Ufak bi sorun build.py de hataları döküp exception verdikten sonra bi şekilde tekrar sorun çıkması, sanırım exception handler içinde ui nesnesi kayboluyor? Buna bakabilirseniz iyi olur. --- pisi/build.py | 7 ++- pisi/context.py | 4 +- pisi/dependency.py | 8 ++- pisi/files.py | 21 ++++--- pisi/index.py | 6 +- pisi/metadata.py | 34 +++++++---- pisi/package.py | 5 +- pisi/sourcedb.py | 4 +- pisi/specfile.py | 131 +++++++++++++++++++++++------------------ pisi/util.py | 26 ++++++++ tests/specfiletests.py | 2 +- tools/repostats.py | 6 +- 12 files changed, 161 insertions(+), 93 deletions(-) diff --git a/pisi/build.py b/pisi/build.py index 2ba5bdd4..c28f992a 100644 --- a/pisi/build.py +++ b/pisi/build.py @@ -87,8 +87,11 @@ class PisiBuild: self.actionGlobals = None self.srcDir = None - if not self.spec.verify(): - ui.error("PSPEC file is not valid") + errs = self.spec.has_errors() + if errs: + ui.error("PSPEC file is not valid\n") + for e in errs: + ui.error(e + '\n') raise PisiBuildError, "invalid PSPEC file %s" % self.ctx.pspecfile def setState(self, state): diff --git a/pisi/context.py b/pisi/context.py index ff42fc07..25d40a35 100644 --- a/pisi/context.py +++ b/pisi/context.py @@ -30,7 +30,9 @@ class BuildContext(object): self.pspecfile = pspecfile spec = SpecFile() spec.read(pspecfile) - spec.verify() # check pspec integrity + # FIXME: following checks the integrity but does nothing when it is wrong + # -gurer + #spec.verify() # check pspec integrity self.spec = spec # directory accessor functions diff --git a/pisi/dependency.py b/pisi/dependency.py index 57e91e52..beaa22a3 100644 --- a/pisi/dependency.py +++ b/pisi/dependency.py @@ -18,6 +18,7 @@ from ui import ui from version import Version from xmlext import * from xmlfile import XmlFile +from util import Checks class DepInfo: def __init__(self, node = None): @@ -44,9 +45,10 @@ class DepInfo: node.setAttribute("releaseTo", self.versionTo) return node - def verify(self): - if not self.package: return False - return True + def has_errors(self): + if not self.package: + return [ "Dependency should have a package string" ] + return None def satisfies(self, pkg_name, version, release): """determine if a package ver. satisfies given dependency spec""" diff --git a/pisi/files.py b/pisi/files.py index 10c880f7..3eafab5b 100644 --- a/pisi/files.py +++ b/pisi/files.py @@ -15,6 +15,7 @@ from xmlext import * from xmlfile import XmlFile +from util import Checks class FileInfo: """FileInfo holds the information for a File node/tag in files.xml""" @@ -54,12 +55,13 @@ class FileInfo: elt.appendChild(hashElt) return elt - def verify(self): - if not self.path: return False - if not self.type: return False - if not self.size: return False - if not self.hash: return False - return True + def has_errors(self): + err = Checks() + err.has_tag(self.path, "File", "Path") + err.has_tag(self.type, "File", "Type") + err.has_tag(self.size, "File", "Size") + err.has_tag(self.hash, "File", "Hash") + return err.list class Files(XmlFile): @@ -83,7 +85,8 @@ class Files(XmlFile): document.appendChild(x.elt(self.dom)) self.writexml(filename) - def verify(self): + def has_errors(self): + err = Checks() for finfo in self.list: - if not finfo.verify(): return False - return True + err.join(finfo.has_errors()) + return err.list diff --git a/pisi/index.py b/pisi/index.py index dd46eb65..d2993a0c 100644 --- a/pisi/index.py +++ b/pisi/index.py @@ -94,7 +94,7 @@ class Index(XmlFile): #md.package.packageURI = util.removepathprefix(repo_uri, path) md.package.packageURI = os.path.realpath(path) # check package semantics - if md.verify(): - self.packages.append(md.package) - else: + if md.has_errors(): ui.error('Package ' + md.package.name + ': metadata corrupt\n') + else: + self.packages.append(md.package) diff --git a/pisi/metadata.py b/pisi/metadata.py index b6be1d8f..b5302c20 100644 --- a/pisi/metadata.py +++ b/pisi/metadata.py @@ -20,6 +20,7 @@ from ui import ui from xmlfile import * import specfile +from util import Checks class SourceInfo: @@ -39,9 +40,10 @@ class SourceInfo: node.appendChild(self.packager.elt(xml)) return node - def verify(self): - if not self.name: return False - return True + def has_errors(self): + if not self.name: + return [ "SourceInfo should have a Name" ] + return None class PackageInfo(specfile.PackageInfo): @@ -71,12 +73,16 @@ class PackageInfo(specfile.PackageInfo): xml.addTextNodeUnder(node, "PackageURI", str(self.packageURI)) return node - def verify(self): - ret = specfile.PackageInfo.verify(self) and self.build!=None + def has_errors(self): + # FIXME: there should be real error msgs + # and comment the logic here please, it isn't very clear -gurer + ret = (specfile.PackageInfo.has_errors(self) == None) and self.build!=None ret = ret and self.distribution!=None ret = ret and self.distributionRelease!=None ret = ret and self.architecture!=None and self.installedSize!=None - return ret + if ret: + return None + return [ "Some error in package metadata" ] def __str__(self): s = specfile.PackageInfo.__str__(self) @@ -126,10 +132,14 @@ class MetaData(XmlFile): self.addChild(self.package.elt(self)) self.writexml(filename) - def verify(self): - if not hasattr(self, 'source'): return False - if not self.source.verify(): return False + def has_errors(self): + err = Checks() + # FIXME: is this an internal error?? -gurer + if not hasattr(self, 'source'): + err.add("Metadata should have source") + err.join(self.source.has_errors()) - if not self.package: return False - if not self.package.verify(): return False - return True + if not self.package: + err.add("Metadata should have a package") + err.join(self.package.has_errors()) + return err.list diff --git a/pisi/package.py b/pisi/package.py index 3f5cd331..12110828 100644 --- a/pisi/package.py +++ b/pisi/package.py @@ -25,7 +25,6 @@ from pisi.purl import PUrl from pisi.metadata import MetaData from pisi.files import Files - class PackageError(pisi.Error): pass @@ -92,12 +91,12 @@ class Package: self.metadata = MetaData() self.metadata.read( join(outdir, const.metadata_xml) ) - if not self.metadata.verify(): + if self.metadata.has_errors(): raise PackageError, "MetaData format wrong" self.files = Files() self.files.read( join(outdir, const.files_xml) ) - if not self.files.verify(): + if self.files.has_errors(): raise PackageError, "invalid %s" % const.files_xml def pkg_dir(self): diff --git a/pisi/sourcedb.py b/pisi/sourcedb.py index 8c0d3856..00a9d55f 100644 --- a/pisi/sourcedb.py +++ b/pisi/sourcedb.py @@ -49,7 +49,9 @@ class SourceDB(object): return self.d[name] def add_source(self, source_info): - assert source_info.verify() + # FIXME: how can you make a negative assertion -gurer + # and yes i'm not very clever :) + # assert source_info.has_errors() name = str(source_info.name) self.d[name] = source_info diff --git a/pisi/specfile.py b/pisi/specfile.py index b89e0df6..8fd296c5 100644 --- a/pisi/specfile.py +++ b/pisi/specfile.py @@ -25,12 +25,16 @@ from xmlext import * from xmlfile import XmlFile from ui import ui from dependency import DepInfo +from util import Checks class PackagerInfo: def __init__(self, node = None): if node: self.name = getNodeText(getNode(node, "Name")) self.email = getNodeText(getNode(node, "Email")) + else: + self.name = None + self.email = None def elt(self, xml): node = xml.newNode("Packager") @@ -38,10 +42,11 @@ class PackagerInfo: xml.addTextNodeUnder(node, "Email", self.email) return node - def verify(self): - if not self.name: return False - if not self.email: return False - return True + def has_errors(self): + err = Checks() + err.has_tag(self.name, "Packager", "Name") + err.has_tag(self.email, "Packager", "Email") + return err.list def __str__(self): s = " ".join(self.name, self.email) @@ -61,10 +66,13 @@ class AdditionalFileInfo: if self.permission: node.setAttribute("permission", self.permission) - def verify(self): - if not self.filename: return False - if not self.target: return False - return True + def has_errors(self): + err = Checks() + if not self.filename: + err.add("AdditionalFile should have file name string") + if not self.target: + err.add("AdditionalFile should have a target attribute") + return err.list def __str__(self): s = "->".join(self.filename, self.target) @@ -98,9 +106,10 @@ class PatchInfo: node.setAttribute("target", self.target) return node - def verify(self): - if not self.filename: return False - return True + def has_errors(self): + if not self.filename: + return [ "Patch should have a filename string" ] + return None def __str__(self): s = self.filename @@ -128,11 +137,12 @@ class UpdateInfo: xml.addTextNodeUnder(node, "Type", self.type) return node - def verify(self): - if not self.date: return False - if not self.version: return False - if not self.release: return False - return True + def has_errors(self): + err = Checks() + err.has_tag(self.date, "Update", "Date") + err.has_tag(self.version, "Update", "Version") + err.has_tag(self.release, "Update", "Release") + return err.list def __str__(self): s = self.date @@ -155,9 +165,10 @@ class PathInfo: node.setAttribute("fileType", self.fileType) return node - def verify(self): - if not self.pathname: return False - return True + def has_errors(self): + if not self.pathname: + return [ "Path tag should have a name string" ] + return None def __str__(self): s = self.pathname @@ -176,10 +187,10 @@ class ComarProvide: node.setAttribute("script", self.script) return node - def verify(self): + def has_errors(self): if not self.om or not self.script: - return False - return True + return [ "COMAR provide should have something :)" ] + return None def __str__(self): s = self.script @@ -236,25 +247,29 @@ class SourceInfo: xml.addNodeUnder(node, "History", update.elt(xml)) return node - def verify(self): - if not self.name: return False - if not self.summary: return False - if not self.description: return False - if not self.packager: return False - if not self.license: return False - if (not self.archiveUri) or (not self.archiveType): return False - if not self.archiveSHA1: return False - if len(self.history) <= 0: return False + def has_errors(self): + err = Checks() + err.has_tag(self.name, "Source", "Name") + err.has_tag(self.description, "Source", "Description") + err.has_tag(self.summary, "Source", "Summary") + err.has_tag(self.packager, "Source", "Packager") + err.has_tag(self.license, "Source", "License") + if (not self.archiveUri) or (not self.archiveType): + err.add("Source archive URI and type should be given") + if not self.archiveSHA1: + errd.add("Source archive should have a SHA1 sum") + if len(self.history) <= 0: + err.add("Source needs some education about History :)") - if not self.packager.verify(): return False + err.join(self.packager.has_errors()) for update in self.history: - if not update.verify(): return False + err.join(update.has_errors()) for patch in self.patches: - if not patch.verify(): return False + err.join(patch.has_errors()) for dep in self.buildDeps: - if not dep.verify(): return False - - return True + err.join(dep.has_errors()) + + return err.list class PackageInfo(object): """A structure to hold package information. Package information is @@ -309,20 +324,23 @@ class PackageInfo(object): xml.addNodeUnder(node, "AdditionalFiles", afile.elt(xml)) return node - def verify(self): - if not self.name: return False - if not self.summary: return False - if not self.description: return False - if not self.license: return False - if len(self.paths) <= 0: return False - + def has_errors(self): + err = Checks() + err.has_tag(self.name, "Package", "Name") + err.has_tag(self.summary, "Package", "Summary") + err.has_tag(self.description, "Package", "Description") + err.has_tag(self.license, "Package", "License") + if len(self.paths) <= 0: + err.add("Package should have some files") + for path in self.paths: - if not path.verify(): return False + err.join(path.has_errors()) for dep in self.runtimeDeps: - if not dep.verify(): return False + err.join(dep.has_errors()) for afile in self.additionalFiles: - if not afile.verify(): return False - return True + err.join(afile.has_errors()) + + return err.list def __str__(self): s = 'Name: ' + self.name @@ -389,14 +407,15 @@ class SpecFile(XmlFile): elif not pkg.isa and self.source.isa: pkg.isa = self.source.isa - - def verify(self): - """Verify PSPEC structures, are they what we want of them?""" - if not self.source.verify(): return False - if len(self.packages) <= 0: return False - for x in self.packages: - if not x.verify(): return False - return True + def has_errors(self): + """Return errors of the PSPEC file if there are any.""" + err = Checks() + err.join(self.source.has_errors()) + if len(self.packages) <= 0: + errs.add("There should be at least one Package section") + for p in self.packages: + err.join(p.has_errors()) + return err.list def write(self, filename): """Write PSPEC file""" diff --git a/pisi/util.py b/pisi/util.py index 350fa1e3..d9120131 100644 --- a/pisi/util.py +++ b/pisi/util.py @@ -37,6 +37,32 @@ class UtilError(pisi.Error): pass +######################### +# spec validation utility # +######################### + +class Checks: + def __init__(self): + self.list = None + + def add(self, err): + if not self.list: + self.list = [] + self.list.append(err) + + def join(self, list): + if list != None: + if not self.list: + self.list = [] + self.list.extend(list) + + def has_tag(self, var, section, name): + if not var: + if not self.list: + self.list = [] + self.list.append("%s section should have a '%s' tag" % (section, name)) + + ######################### # string/list functions # ######################### diff --git a/tests/specfiletests.py b/tests/specfiletests.py index 429ba3cf..5b233df2 100644 --- a/tests/specfiletests.py +++ b/tests/specfiletests.py @@ -68,7 +68,7 @@ class SpecFileTestCase(unittest.TestCase): self.fail("Failed to match PartOf in Package") def testVerify(self): - if not self.spec.verify(): + if self.spec.has_errors(): self.fail("Failed to verify specfile") def testCopy(self): diff --git a/tools/repostats.py b/tools/repostats.py index ad7c6124..0521041c 100755 --- a/tools/repostats.py +++ b/tools/repostats.py @@ -72,8 +72,10 @@ for pak in paks: except Exception, inst: errors.append([pak, str(inst)]) continue - if spec.verify() is False: - errors.append([pak, "specfile verification failed"]) + errs = spec.has_errors() + if errs: + for e in errs: + errors.append([pak, e]) continue nr_binpaks += len(spec.packages) nr_patches += len(spec.source.patches)