#1 Initial createrepo_c support
Opened 8 years ago by parasense. Modified 7 years ago
parasense/mash master  into  master

Trivial whitespace fix
Jon Disnard • 8 years ago  
file modified
+91 -3
@@ -235,13 +235,101 @@ 

              _make_ancient(path, self.conf.excludes, self.previous, self.logger)

  

  

+ class MetadataOld_c:

+     def __init__(self, logger):

+         self.args = [ 'createrepo_c', '--update', '-q' ]

+         self.previous = None

+         self.logger = logger

+         self.excludes = []

+         self.ancient = False

+ 

+     def set_ancient(self, ancient):

+         self.ancient = ancient

+ 

+     def set_cachedir(self, path):

+         self.args.append('-c')

+         self.args.append(path)

+ 

+     def set_comps(self, comps):

+         self.args.append('-g')

+         self.args.append(comps)

+ 

+     def set_excludes(self, excludes):

+         self.args.append('-x')

+         self.args.append(excludes)

+         self.excludes.append(excludes)

+ 

+     def set_database(self, db):

+         self.args.append('-d')

+ 

+     def set_compress_type(self, compress_type):

+         self.args.append('--compress-type')

+         self.args.append(compress_type)

+ 

+     def set_hash(self, hashtype):

+         # Sorry, can't do that here.

+         #pass

+ 

+         ## YES, we can do that here.

+         self.args.append('--checksum')

+         self.args.append(hashtype)

+ 

+     def set_skipstat(self, skip):

+         if skip:

+             self.args.append('--skip-stat')

+ 

+     def set_delta(self, deltapaths, max_delta_rpm_size, max_delta_rpm_age, delta_workers):

+         # Sorry, can't do that here.

+         ## We still can not do that here.

+         pass

+ 

+     def set_previous(self, previous):

+         self.previous = previous

+ 

+     def set_distro_tags(self, distro_tags):

+         # Sorry, can't do that here.

+         #pass

+         ## YES, we can do that here.

+         self.args.append('--distro')

+         self.args.append(distro_tags)

+ 

+     def set_content_tags(self, content_tags):

+         # Sorry, can't do that here.

+         #pass

+         ## YES, we can do that here.

+         self.args.append('--content')

+         self.args.append(content_tags)

+ 

+     def run(self, path):

+         self.args.append(path)

+         if self.previous:

+             try:

+                 shutil.copytree("%s/repodata" %(self.previous,), "%s/repodata" % (path,))

+             except OSError:

+                 self.logger.error("Couldn't copy repodata from %s" % (self.previous,))

+         pid = os.fork()

+         if not pid:

+             os.execv("/usr/bin/createrepo_c", self.args)

+         else:

+             (p, status) = os.waitpid(pid, 0)

+         if self.ancient:

+             _make_ancient(path, self.excludes, self.previous, self.logger)

+ 

+ 

  class Metadata:

  

      def __init__(self, logger):

-         if usenew:

-             self.obj = MetadataNew(logger)

+         if repo_c:

+             if usenew:

+                 # For now use the old createcrepo api

+                 self.obj = MetadataNew(logger)

+             else:

+                 self.obj = MetadataOld_c(logger)

          else:

-             self.obj = MetadataOld(logger)

+             if usenew:

+                 self.obj = MetadataNew(logger)

+             else:

+                 self.obj = MetadataOld(logger)

  

      def set_ancient(self, ancient):

          self.obj.set_ancient(ancient)

no initial comment

So, this has been sitting here for 10 months? Here's a first stab at a review:

@parasense, there are a number of odd comments in the functions that don't quite make sense to me. (For instance in set_delta). Are they supposed to be a kind of dialogue with the comments from the other MetadataOld class? If so, maybe those could be cleaned up to explain why we can't do such and such thing instead of just stating that we can't.

Is there a reason why we can support createrepo_c for MetadataOld_c, but not also implement a MetadataNew_c class as well? If not, that's probably okay. It would just be good to have a record of the thinking on why.

Lastly, this whole section might benefit from some class inheritance. A number of the methods from MetadataOld_c just got copied over. Perhaps you could extend it instead and only reimplement the methods that you need to... to reduce code duplication.

we'll be killing off the use of mash and moving it to signed repos in koji before F-24 beta

OK. Then in a few months let's circle back here and close all the issues and PRs and update the README to shut down shop.

Tracking signed repos in koji over here.

@ralph @pbrobinson mash never got killed. Please revisit this.

Digging into this, we found that "dist repos" support got merged over here: https://pagure.io/koji/pull-request/318

Digging into this, we found that "dist repos" support got merged over here: https://pagure.io/koji/pull-request/318

Indeed

Metadata