Evet, kocaman bir patch.

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

@@ -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):

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

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"""

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

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)

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

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):

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
 

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"""

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 #
 #########################

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):

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)