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)