[Yum-devel] [PATCH] createrepo: buggy getFileList() - please review

[Zdenek Pavlas]

Hi,

Tomas Mlcoch found that --skip-symlinks is broken,
and I've merged his simple and obviously correct patch.

But the directory test is broken too, and the overriden
getFileList() in SplitMetadataGenerator is buggy, too.
So I've fixed these as well:

commit 73400ac4a9717bcedb73902c7a510f3282e1a47b
Author: Zdeněk Pavlas <zpavlas at redhat.com>
Date:   Mon Nov 26 14:30:20 2012 +0100

    also fix os.path.isdir(), kill broken SplitMetadataGenerator.getFileList

diff --git a/createrepo/__init__.py b/createrepo/__init__.py
index ef51b14..b3a3c06 100644
--- a/createrepo/__init__.py
+++ b/createrepo/__init__.py
@@ -307,14 +307,13 @@ class MetaDataGenerator:
 
         def extension_visitor(filelist, dirname, names):
             for fn in names:
+                fn = os.path.join(dirname, fn)
                 if os.path.isdir(fn):
                     continue
-                if self.conf.skip_symlinks and os.path.islink(os.path.join(dirname, fn)):
+                if self.conf.skip_symlinks and os.path.islink(fn):
                     continue
                 elif fn[-extlen:].lower() == '%s' % (ext):
-                    relativepath = dirname.replace(startdir, "", 1)
-                    relativepath = relativepath.lstrip("/")
-                    filelist.append(os.path.join(relativepath, fn))
+                    filelist.append(fn[len(startdir):])
 
         filelist = []
         startdir = directory + '/'
@@ -1263,24 +1262,6 @@ class SplitMetaDataGenerator(MetaDataGenerator):
         (scheme, netloc, path, query, fragid) = urlparse.urlsplit(url)
         return urlparse.urlunsplit((scheme, netloc, path, query, str(fragment)))
 
-    def getFileList(self, directory, ext):
-
-        extlen = len(ext)
-
-        def extension_visitor(arg, dirname, names):
-            for fn in names:
-                if os.path.isdir(fn):
-                    continue
-                elif fn[-extlen:].lower() == '%s' % (ext):
-                    reldir = os.path.basename(dirname)
-                    if reldir == os.path.basename(directory):
-                        reldir = ""
-                    arg.append(os.path.join(reldir, fn))
-
-        rpmlist = []
-        os.path.walk(directory, extension_visitor, rpmlist)
-        return rpmlist
-
     def doPkgMetadata(self):
         """all the heavy lifting for the package metadata"""
         if len(self.conf.directories) == 1:

While the first hunk is fine, I'm not 100% sure if the getFileList()
in SplitMetadataGenerator wasn't modified on purpose.  It has been
added in this commit:

commit 4dfa59ce436bd59f9a84199ebb7d75dd31df05eb
Author: Paul Nasrat <pnasrat at redhat.com>
Date:   Thu Jul 20 22:27:51 2006 +0000

    Make splitmetadata handler do it' own getFileList to correctly manipulate paths.


But, when .RPM file is found 0 or 1 subdirs below the repo root, it works
exactly the same as the (current) base implementation.  When .RPM file is
2 or more levels deep, it returns only the latest directory element + basename,
and worker processes thus fail to open it.

So, always using the same getFileList() seems like a bugfix,
but maybe I've missed something not so obvious.