koji

Clone

#1743 basic zchunk support for dist-repo
Merged 5 years ago by tkopecek. Opened 5 years ago by tkopecek.
tkopecek/koji issue1198  into  master

file modified
+19 -3 
@@ -5508,7 +5508,8 @@ 
 

          if oldrepo:
 

              oldrepodata = os.path.join(oldrepodir, arch, 'repodata')
 

          self.do_createrepo(self.repodir, '%s/pkglist' % self.repodir,
 

-                     groupdata, oldpkgs=oldpkgs, oldrepodata=oldrepodata)
 

+                     groupdata, oldpkgs=oldpkgs, oldrepodata=oldrepodata,
 

+                     zck=opts.get('zck'), zck_dict_dir=opts.get('zck_dict_dir'))
 

          for subrepo in self.subrepos:
 

              if oldrepo:
 

                  oldrepodata = os.path.join(oldrepodir, arch, subrepo, 'repodata')
 
@@ -5516,7 +5517,9 @@ 
 

                      '%s/%s' % (self.repodir, subrepo),
 

                      '%s/%s/pkglist' % (self.repodir, subrepo),
 

                      groupdata, oldpkgs=oldpkgs, oldrepodata=oldrepodata,
 

-                     logname='createrepo_%s' % subrepo)
 

+                     logname='createrepo_%s' % subrepo,
 

+                     zck=opts.get('zck'),
 

+                     zck_dict_dir=opts.get('zck_dict_dir'))
 

          if len(self.kojipkgs) == 0:
 

              fn = os.path.join(self.repodir, "repodata", "EMPTY_REPO")
 

              with open(fn, 'w') as fp:
 
@@ -5566,7 +5569,7 @@ 
 

          self.session.uploadWrapper(fn, self.uploadpath)
 

  

      def do_createrepo(self, repodir, pkglist, groupdata, oldpkgs=None,
 

-             logname=None, oldrepodata=None):
 

+             logname=None, oldrepodata=None, zck=False, zck_dict_dir=None):
 

          """Run createrepo
 

  

          This is derived from CreaterepoTask.create_local_repo, but adapted to
 
@@ -5577,6 +5580,10 @@ 
 

              cmd = ['/usr/bin/createrepo_c']
 

          else:
 

              cmd = ['/usr/bin/createrepo']
 

+             if zck:
 

+                 raise koji.GenericError("createrepo doesn't support zchunks")
 

+         if zck_dict_dir and not zck:
 

+             raise koji.GenericError("--zck-dict-dir makes no sense without --zck")
 

          cmd.extend(['-vd', '-i', pkglist])
 

          if groupdata and os.path.isfile(groupdata):
 

              cmd.extend(['-g', groupdata])
 
@@ -5599,6 +5606,15 @@ 
 

              cmd.append('--deltas')
 

              for op_dir in oldpkgs:
 

                  cmd.extend(['--oldpackagedirs', op_dir])
 

+         if zck:
 

+             cmd.append('--zck')
 

+         if zck_dict_dir:
 

+             zck_dict_dir = os.path.normpath(zck_dict_dir)
 

+             if os.path.isfile(zck_dict_dir):
 

+                 raise koji.GenericError("zchunk dir path is file: %s" % zck_dict_dir)

Would this throw an error if the directory is a symlink? We probably want this to work if it is a symlink.

 
+             if not os.path.isdir(zck_dict_dir):
 

+                 raise koji.GenericError("zchunk dir path doesn't exist: %s" % zck_dict_dir)
 

+             cmd.extend(['--zck-dict-dir', zck_dict_dir])
 

          cmd.append(repodir)
 

  

          if logname is None:

file modified
+7 -1 
@@ -6985,6 +6985,10 @@ 
 

          help=_('Do not wait for the task to complete'))
 

      parser.add_option('--skip-missing-signatures', action='store_true', default=False,
 

          help=_('Skip RPMs not signed with the desired key(s)'))
 

+     parser.add_option('--zck', action='store_true', default=False,
 

+         help=_('Generate zchunk files as well as the standard repodata'))
 

+     parser.add_option('--zck-dict-dir', action='store', default=None,
 

+         help=_('Directory containing compression dictionaries for use by zchunk (on builder)'))
 

      task_opts, args = parser.parse_args(args)
 

      if len(args) < 1:
 

          parser.error(_('You must provide a tag to generate the repo from'))
 
@@ -7064,7 +7068,9 @@ 
 

          'multilib': task_opts.multilib,
 

          'split_debuginfo': task_opts.split_debuginfo,
 

          'skip_missing_signatures': task_opts.skip_missing_signatures,
 

-         'allow_missing_signatures': task_opts.allow_missing_signatures
 

+         'allow_missing_signatures': task_opts.allow_missing_signatures,
 

+         'zck': task_opts.zck,
 

+         'zck_dict_dir': task_opts.zck_dict_dir,
 

      }
 

      task_id = session.distRepo(tag, keys, **opts)
 

      print("Creating dist repo for tag " + tag)

file modified
+4 
@@ -274,6 +274,10 @@ 
 

    --nowait              Do not wait for the task to complete
 

    --skip-missing-signatures
 

                          Skip RPMs not signed with the desired key(s)
 

+   --zck                 Generate zchunk files as well as the standard repodata
 

+   --zck-dict-dir=ZCK_DICT_DIR
 

+                         Directory containing compression dictionaries for use
 

+                         by zchunk (on builder)
 

  """ % self.progname)

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready
5 years ago

Should be default=None for consistency?

Should be default=None for consistency?

yep, updated

rebased onto 3a1a777ecd51fc4750a4bdee8a5396a44bd31334
5 years ago

@tkopecek How difficult would it be to make this a per dist-repo tag feature instead of a per-builder feature?

@ngompa It would be easy. What I'm struggling with, is how zdict data would appear on builder. They could be 'always there' installed via rpm (then we just need path or filename in tag's extra), or they need to be downloaded from somewhere (and url in extras), which seems to me costly.

or it is a file? and I'm nore sure if ../* could bring security issues here

@tkopecek They're significant enough that I think it's most likely they'll always be available via RPM. But it could be fetched, and maybe kojid could cache them somewhere in that case?

rebased onto e23f1adf76f621fafc2c5033e907ff21d2484740
5 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done
5 years ago

@julian8628 What exactly do you mean? There is a check on existence of directory few lines later.

@tkopecek I think supporting either local files (installed on builder) or fetching remotely should be supported. But I'm comfortable with this being per-tag and also being local-only if you feel that's the way to go.

I know Koji instances exist that build for multiple distributions (heck, Fedora's is one!) and it's totally conceivable to have different zdicts for each build target.

@julian8628 What exactly do you mean? There is a check on existence of directory few lines later.

Ah, I meant the GenericError is not accurate, if zck_dict_dir exists but is a file.

And I'm not sure it will modify or expose some thing under zck_dict_dir (I don't know very well about this mechanism. Sorry for that stupid question :-( )

rebased onto 98c765d
5 years ago

:thumbsup:

Would this throw an error if the directory is a symlink? We probably want this to work if it is a symlink.

Commit 6ecffb8 fixes this pull-request

Pull-Request has been merged by tkopecek
5 years ago

@ngompa It works correctly for symlinks.
Changes Summary 3
+19 -3
file changed
builder/kojid
+7 -1
file changed
cli/koji_cli/commands.py
+4 -0
file changed
tests/test_cli/test_dist_repo.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.