replacing os.chdir calls with basedir config option

Dennis Gregorovic <[email protected]>
Newsgroups gmane.linux.rpm.metadata
Message-ID <[email protected]> 
I've started using createrepo as a library instead of a standalone app.
The only issue that I've run into so far is that the os.chdir() calls
can cause problems, especially if you have multiple threads using
createrepo in parallel.

Attached is a patch that removes the os.chdir() calls and introduces a
'basedir' config option, settable with --basedir and defaulting to
os.getcwd().

Thoughts?

-- Dennis

_______________________________________________
Rpm-metadata mailing list
[email protected]
https://lists.dulug.duke.edu/mailman/listinfo/rpm-metadata
createrepo-basedir.patch (text/x-patch, 10.7 KB) 
Index: genpkgmetadata.py
===================================================================
RCS file: /cvsroot/metadata/cvs-root/generate/genpkgmetadata.py,v
retrieving revision 1.41
diff -w -u -r1.41 genpkgmetadata.py
--- genpkgmetadata.py	2 Nov 2005 20:23:56 -0000	1.41
+++ genpkgmetadata.py	11 Nov 2005 13:38:23 -0000
@@ -59,24 +59,29 @@
     sys.exit(retval)
 
 
-def getFileList(path, ext, filelist):
+def getFileList(basepath, path, ext, filelist):
     """Return all files in path matching ext, store them in filelist, recurse dirs
        return list object"""
     
     extlen = len(ext)
+    totalpath = os.path.normpath(os.path.join(basepath, path))
     try:
-        dir_list = os.listdir(path)
+        dir_list = os.listdir(totalpath)
     except OSError, e:
-        errorprint(_('Error accessing directory %s, %s') % (path, e))
+        errorprint(_('Error accessing directory %s, %s') % (totalpath, e))
         sys.exit(1)
         
     for d in dir_list:
-        if os.path.isdir(path + '/' + d):
-            filelist = getFileList(path + '/' + d, ext, filelist)
+        if os.path.isdir(totalpath + '/' + d):
+            filelist = getFileList(basepath, os.path.join(path, d), ext, filelist)
         else:
             if string.lower(d[-extlen:]) == '%s' % (ext):
-               newpath = os.path.normpath(path + '/' + d)
-               filelist.append(newpath)
+                if totalpath.find(basepath) == 0:
+                    relativepath = totalpath.replace(basepath, "", 1)
+                    relativepath = relativepath.lstrip("/")
+                    filelist.append(os.path.join(relativepath, d))
+                else:
+                    raise "basepath '%s' not found in path '%s'" % (basepath, totalpath)
                     
     return filelist
 
@@ -136,13 +141,14 @@
     cmds['pretty'] = 0
 #    cmds['updategroupsonly'] = 0
     cmds['cachedir'] = None
+    cmds['basedir'] = os.getcwd()
     cmds['cache'] = False
     cmds['file-pattern-match'] = ['.*bin\/.*', '^\/etc\/.*', '^\/usr\/lib\/sendmail$']
     cmds['dir-pattern-match'] = ['.*bin\/.*', '^\/etc\/.*']
 
     try:
         gopts, argsleft = getopt.getopt(args, 'phqVvg:s:x:u:c:', ['help', 'exclude=', 
-                                            'quiet', 'verbose', 'cachedir=',
+                                                                  'quiet', 'verbose', 'cachedir=', 'basedir=',
                                             'baseurl=', 'groupfile=', 'checksum=',
                                             'version', 'pretty'])
     except getopt.error, e:
@@ -188,7 +194,7 @@
                     errorprint(_('Error: Only one groupfile allowed.'))
                     usage()
                 else:
-                    if os.path.exists(directory + '/' + a):
+                    if os.path.exists(a):
                         cmds['groupfile'] = a
                     else:
                         errorprint(_('Error: groupfile %s cannot be found.' % a))
@@ -209,6 +215,8 @@
                 if not checkAndMakeDir(a):
                     errorprint(_('Error: cannot open/write to cache dir %s' % a))
                     usage()
+            elif arg == '--basedir':
+                cmds['basedir'] = a
                     
     except ValueError, e:
         errorprint(_('Options Error: %s') % e)
@@ -222,7 +230,7 @@
 
     # rpms we're going to be dealing with
     files = []
-    files = getFileList('./', '.rpm', files)
+    files = getFileList(cmds['basedir'], ".", '.rpm', files)
     files = trimRpms(files, cmds['excludes'])
     pkgcount = len(files)
     
@@ -232,7 +240,7 @@
     basens = baseroot.newNs('http://linux.duke.edu/metadata/common', None)
     formatns = baseroot.newNs('http://linux.duke.edu/metadata/rpm', 'rpm')
     baseroot.setNs(basens)
-    basefilepath = os.path.join(cmds['tempdir'], cmds['primaryfile'])
+    basefilepath = os.path.join(cmds['basedir'], cmds['tempdir'], cmds['primaryfile'])
     basefile = _gzipOpen(basefilepath, 'w')
     basefile.write('<?xml version="1.0" encoding="UTF-8"?>\n')
     basefile.write('<metadata xmlns="http://linux.duke.edu/metadata/common" xmlns:rpm="http://linux.duke.edu/metadata/rpm" packages="%s">\n' % 
@@ -243,7 +251,7 @@
     filesroot = filesdoc.newChild(None, "filelists", None)
     filesns = filesroot.newNs('http://linux.duke.edu/metadata/filelists', None)
     filesroot.setNs(filesns)
-    filelistpath = os.path.join(cmds['tempdir'], cmds['filelistsfile'])
+    filelistpath = os.path.join(cmds['basedir'], cmds['tempdir'], cmds['filelistsfile'])
     flfile = _gzipOpen(filelistpath, 'w')    
     flfile.write('<?xml version="1.0" encoding="UTF-8"?>\n')
     flfile.write('<filelists xmlns="http://linux.duke.edu/metadata/filelists" packages="%s">\n' % 
@@ -255,7 +263,7 @@
     otherroot = otherdoc.newChild(None, "otherdata", None)
     otherns = otherroot.newNs('http://linux.duke.edu/metadata/other', None)
     otherroot.setNs(otherns)
-    otherfilepath = os.path.join(cmds['tempdir'], cmds['otherfile'])
+    otherfilepath = os.path.join(cmds['basedir'], cmds['tempdir'], cmds['otherfile'])
     otherfile = _gzipOpen(otherfilepath, 'w')
     otherfile.write('<?xml version="1.0" encoding="UTF-8"?>\n')
     otherfile.write('<otherdata xmlns="http://linux.duke.edu/metadata/other" packages="%s">\n' % 
@@ -266,7 +274,7 @@
     for file in files:
         current+=1
         try:
-            mdobj = dumpMetadata.RpmMetaData(ts, file, cmds)
+            mdobj = dumpMetadata.RpmMetaData(ts, cmds['basedir'], file, cmds)
             if not cmds['quiet']:
                 if cmds['verbose']:
                     print '%d/%d - %s' % (current, len(files), file)
@@ -346,7 +354,7 @@
     reporoot = repodoc.newChild(None, "repomd", None)
     repons = reporoot.newNs('http://linux.duke.edu/metadata/repo', None)
     reporoot.setNs(repons)
-    repofilepath = os.path.join(cmds['tempdir'], cmds['repomdfile'])
+    repofilepath = os.path.join(cmds['basedir'], cmds['tempdir'], cmds['repomdfile'])
     
     try:
         dumpMetadata.repoXML(reporoot, cmds)
@@ -376,74 +384,60 @@
     cmds['finaldir'] = 'repodata'
     cmds['olddir'] = '.olddata'
     
-    # save where we are right now
-    curdir = os.getcwd()
     # start the sanity/stupidity checks
-    if not os.path.exists(directory):
+    if not os.path.exists(os.path.join(cmds['basedir'], directory)):
         errorprint(_('Directory must exist'))
         sys.exit(1)
         
-    if not os.path.isdir(directory):
+    if not os.path.isdir(os.path.join(cmds['basedir'], directory)):
         errorprint(_('Directory of packages must be a directory.'))
         sys.exit(1)
         
-    if not os.access(directory, os.W_OK):
+    if not os.access(cmds['basedir'], os.W_OK):
         errorprint(_('Directory must be writable.'))
         sys.exit(1)
 
  
-    if not checkAndMakeDir(os.path.join(directory, cmds['tempdir'])):
+    if not checkAndMakeDir(os.path.join(cmds['basedir'], cmds['tempdir'])):
         sys.exit(1)
         
-    if not checkAndMakeDir(os.path.join(directory, cmds['finaldir'])):
+    if not checkAndMakeDir(os.path.join(cmds['basedir'], cmds['finaldir'])):
         sys.exit(1)
         
-    if os.path.exists(os.path.join(directory, cmds['olddir'])):
+    if os.path.exists(os.path.join(cmds['basedir'], cmds['olddir'])):
         errorprint(_('Old data directory exists, please remove: %s') % cmds['olddir'])
         sys.exit(1)
         
-    # change to the basedir to work from w/i the path - for relative url paths
-    os.chdir(directory)
-
     # make sure we can write to where we want to write to:
     for direc in ['tempdir', 'finaldir']:
         for file in ['primaryfile', 'filelistsfile', 'otherfile', 'repomdfile']:
-            filepath = os.path.join(cmds[direc], cmds[file])
+            filepath = os.path.join(cmds['basedir'], cmds[direc], cmds[file])
             if os.path.exists(filepath):
                 if not os.access(filepath, os.W_OK):
                     errorprint(_('error in must be able to write to metadata files:\n  -> %s') % filepath)
-                    os.chdir(curdir)
                     usage()
                     
     ts = rpm.TransactionSet()
-    try:
         doPkgMetadata(cmds, ts)
-    except:
-        # always clean up your messes
-        os.chdir(curdir)
-        raise
-    
-    try:
         doRepoMetadata(cmds)
-    except:
-        os.chdir(curdir)
-        raise
         
-    if os.path.exists(cmds['finaldir']):
+    if os.path.exists(os.path.join(cmds['basedir'], cmds['finaldir'])):
         try:
-            os.rename(cmds['finaldir'], cmds['olddir'])
+            os.rename(os.path.join(cmds['basedir'], cmds['finaldir']),
+                      os.path.join(cmds['basedir'], cmds['olddir']))
         except:
-            errorprint(_('Error moving final to old dir'))
-            os.chdir(curdir)
+            errorprint(_('Error moving final %s to old dir %s' % (os.path.join(cmds['basedir'], cmds['finaldir']),
+                                                                  os.path.join(cmds['basedir'], cmds['olddir']))))
             sys.exit(1)
         
     try:
-        os.rename(cmds['tempdir'], cmds['finaldir'])
+        os.rename(os.path.join(cmds['basedir'], cmds['tempdir']),
+                  os.path.join(cmds['basedir'], cmds['finaldir']))
     except:
         errorprint(_('Error moving final metadata into place'))
         # put the old stuff back
-        os.rename(cmds['olddir'], cmds['finaldir'])
-        os.chdir(curdir)
+        os.rename(os.path.join(cmds['basedir'], cmds['olddir']),
+                  os.path.join(cmds['basedir'], cmds['finaldir']))
         sys.exit(1)
         
     for file in ['primaryfile', 'filelistsfile', 'otherfile', 'repomdfile', 'groupfile']:
@@ -451,31 +445,21 @@
             fn = os.path.basename(cmds[file])
         else:
             continue
-        oldfile = os.path.join(cmds['olddir'], fn)
+        oldfile = os.path.join(cmds['basedir'], cmds['olddir'], fn)
         if os.path.exists(oldfile):
             try:
                 os.remove(oldfile)
             except OSError, e:
                 errorprint(_('Could not remove old metadata file: %s') % oldfile)
                 errorprint(_('Error was %s') % e)
-                os.chdir(curdir)
                 sys.exit(1)
             
     try:
-        os.rmdir(cmds['olddir'])
+        os.rmdir(os.path.join(cmds['basedir'], cmds['olddir']))
     except OSError, e:
         errorprint(_('Could not remove old metadata dir: %s') % cmds['olddir'])
         errorprint(_('Error was %s') % e)
         errorprint(_('Please clean up this directory manually.'))
-        os.chdir(curdir)
-        
-        
-        
-        
-    # take us home mr. data
-    os.chdir(curdir)
-        
-
         
 if __name__ == "__main__":
     if len(sys.argv) > 1:
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.