#1462 rebuildSRPM task
Merged 2 years ago by mikem. Opened 2 years ago by tkopecek.
tkopecek/koji issue1396  into  master

file modified
+136 -2
@@ -554,6 +554,39 @@ 

          msg = '; see %s for more information' % logfile

          return parseStatus(rv, 'mock') + msg

  

+     def rebuild_srpm(self, srpm):

+         self.session.host.setBuildRootState(self.id,'BUILDING')

+ 

+         # unpack SRPM to tempdir

+         srpm_dir = os.path.join(self.tmpdir(), 'srpm_unpacked')

+         koji.ensuredir(srpm_dir)

+         top_dir = self.path_without_to_within(srpm_dir)

+         args = ['--no-clean', '--target', 'noarch', '--chroot', '--',

+                 'rpm', '--define', '_topdir %s' % top_dir, '-iv', srpm]

+         rv = self.mock(args)

+ 

+         # find specfile

+         spec_files = glob.glob("%s/SPECS/*.spec" % srpm_dir)

+         if len(spec_files) == 0:

+             raise koji.BuildError("No spec file found")

+         elif len(spec_files) > 1:

+             raise koji.BuildError("Multiple spec files found: %s" % spec_files)

+         spec_file = os.path.join(top_dir, "SPECS", os.path.basename(spec_files[0]))

+ 

+         # rebuild SRPM from spec + sources

+         args = ['--no-clean', '--target', 'noarch', '--chroot', '--',

+                 'rpmbuild', '--define', '_topdir %s' % top_dir, '-bs', '--nodeps', spec_file]

+         rv = self.mock(args)

+ 

+         result_dir = os.path.join(srpm_dir, 'SRPMS')

+         for fn in glob.glob('%s/*.src.rpm' % result_dir):

+             shutil.move(os.path.join(result_dir, fn), self.resultdir())

+ 

+         if rv:

+             self.expire()

+             raise koji.BuildError("error building srpm, %s" % self._mockResult(rv))

+ 

+ 

      def build_srpm(self, specfile, sourcedir, source_cmd):

          self.session.host.setBuildRootState(self.id,'BUILDING')

          if source_cmd:
@@ -1025,12 +1058,31 @@ 

              if SCM.is_scm_url(src):

                  return self.getSRPMFromSCM(src, build_tag, repo_id)

              else:

-                 #assume this is a path under uploads

-                 return src

+                 buildconfig = self.session.getBuildConfig(build_tag, event=self.event_id)

+                 if buildconfig['extra'].get('rebuild_srpm', True):

+                     # default is to always rebuild

+                     return self.getSRPMFromSRPM(src, build_tag, repo_id)

+                 else:

+                     return src

          else:

              raise koji.BuildError('Invalid source specification: %s' % src)

              #XXX - other methods?

  

+     def getSRPMFromSRPM(self, src, build_tag, repo_id):

+         # rebuild srpm in mock, so it gets correct disttag, rpm version, etc.

+         task_id = self.session.host.subtask(method='rebuildSRPM',

+                                             arglist=[src, build_tag, {'repo_id': repo_id, 'scratch': self.opts.get('scratch')}],

+                                             label='srpm',

+                                             parent=self.id)

+         # wait for subtask to finish

+         result = self.wait(task_id)[task_id]

+         if 'source' in result:

+             self.source = result['source']

+         else:

+             self.logger.warning('subtask did not provide source data')

+         srpm = result['srpm']

+         return srpm

+ 

      def getSRPMFromSCM(self, url, build_tag, repo_id):

          #TODO - allow different ways to get the srpm

          task_id = self.session.host.subtask(method='buildSRPMFromSCM',
@@ -4582,6 +4634,88 @@ 

          return report

  

  

+ class RebuildSRPM(BaseBuildTask):

+ 

+     Methods = ['rebuildSRPM']

+     _taskWeight = 1.0

+ 

+     def checkHost(self, hostdata):

+         tag = self.params[1]

+         return self.checkHostArch(tag, hostdata)

+ 

+     def handler(self, srpm, build_tag, opts=None):

+         if opts is None:

+             opts = {}

+         repo_id = opts.get('repo_id')

+         if not repo_id:

+             raise koji.BuildError("A repo id must be provided")

+ 

+         repo_info = self.session.repoInfo(repo_id, strict=True)

+         event_id = repo_info['create_event']

+         build_tag = self.session.getTag(build_tag, strict=True, event=event_id)

+ 

+         rootopts = {'install_group': 'srpm-build', 'repo_id': repo_id}

+         br_arch = self.find_arch('noarch', self.session.host.getHost(), self.session.getBuildConfig(build_tag['id'], event=event_id))

+         broot = BuildRoot(self.session, self.options, build_tag['id'], br_arch, self.id, **rootopts)

+         broot.workdir = self.workdir

+ 

+         self.logger.debug("Initializing buildroot")

+         broot.init()

+ 

+         # Setup files and directories for SRPM rebuild

+         # We can't put this under the mock homedir because that directory

+         # is completely blown away and recreated on every mock invocation

+         srpmdir = broot.tmpdir() + '/srpm'

+         koji.ensuredir(srpmdir)

+         uploadpath = self.getUploadDir()

+ 

+         fn = self.localPath("work/%s" % srpm)

+         if not os.path.exists(fn):

+             raise koji.BuildError("Input SRPM file missing: %s" % fn)

+         shutil.copy(fn, srpmdir)

+ 

+         # rebuild srpm

+         self.logger.debug("Running srpm rebuild")

+         br_srpm_path = os.path.join(broot.path_without_to_within(srpmdir), os.path.basename(srpm))

+         broot.rebuild_srpm(br_srpm_path)

+ 

+         srpms = glob.glob('%s/*.src.rpm' % broot.resultdir())

+         if len(srpms) == 0:

+             raise koji.BuildError("No srpms found in %s" % srpmdir)

+         elif len(srpms) > 1:

+             raise koji.BuildError("Multiple srpms found in %s: %s" % (srpmdir, ", ".join(srpms)))

+         else:

+             srpm = srpms[0]

+ 

+         # check srpm name

+         h = koji.get_rpm_header(srpm)

+         name = koji.get_header_field(h, 'name')

+         version = koji.get_header_field(h, 'version')

+         release = koji.get_header_field(h, 'release')

+         srpm_name = "%(name)s-%(version)s-%(release)s.src.rpm" % locals()

+         if srpm_name != os.path.basename(srpm):

+             raise koji.BuildError('srpm name mismatch: %s != %s' % (srpm_name, os.path.basename(srpm)))

+ 

+         # upload srpm and return

+         self.uploadFile(srpm)

+ 

+         brootid = broot.id

+         log_files = glob.glob('%s/*.log' % broot.resultdir())

+ 

+         broot.expire()

+ 

+         return {

+             'srpm': "%s/%s" % (uploadpath, srpm_name),

+             'logs': ["%s/%s" % (uploadpath, os.path.basename(f))

+                      for f in log_files],

+             'brootid': brootid,

+             'source': {

+                 'source': os.path.basename(srpm),

+                 'url': os.path.basename(srpm),

+             }

+         }

+ 

+ 

  class BuildSRPMFromSCMTask(BaseBuildTask):

  

      Methods = ['buildSRPMFromSCM']

file modified
+3
@@ -172,6 +172,9 @@ 

      'buildSRPMFromSCM' : [

          [['url', 'build_tag', 'opts'], None, None, (None,)],

      ],

+     'rebuildSRPM' : [

+         [['srpm', 'build_tag', 'opts'], None, None, (None,)],

+     ],

      'createrepo' : [

          [['repo_id', 'arch', 'oldrepo'], None, None, None],

      ],

file modified
+1
@@ -430,6 +430,7 @@ 

  # All Tasks

  _TASKS = ['build',

            'buildSRPMFromSCM',

+           'rebuildSRPM',

            'buildArch',

            'chainbuild',

            'maven',

New task rebuildSRPM for srpms uploaded to koji. If policy
'rebuild_srpm' returns True, build task will spawn rebuildSRPM task
first to recreate SRPM with updated buildroot macros, typically 'dist'.
Policy has access to same data as build_from_srpm and build_from_repo_id
policies. (user_id, source, task_id, build_tag, skip_tag, target, tag)

Fixes: https://pagure.io/koji/issue/1396

rebased onto ffaae125ca60af40324a84a6f1ab40ac4b41af14

2 years ago

Note that this might break assumptions with dynamic BuildRequires which are coming with RPM 4.15.. I did not test anything yet, but just saying.

@ignatenkobrain All this does is make builds from upload srpms more consistent with builds from source control. In the latter case, we build the srpm in a buildroot. That's all this should be doing. If that breaks the assumptions of dynamic BuildRequires, then those assumptions are likely wrong.

Can you expand on why you are worried?

Given my limited knowledge of koji internals, this looks good and makes sense to me.

However, please add a line about rebuild_srpm to the list in the Available policies section of docs/source/defining_hub_policies.rst. Thanks!

rebased onto d2bc427143963dfda79990a4abf38eca43d0923b

2 years ago

docs added

Excellent. Thank you.

Note that rpmbuild -rs is relatively new usage. It does not work in RHEL6 or RHEL7. Previously -r was short for --root.

# rpmbuild -rs
rpmbuild: arguments to --root (-r) must begin with a /

I believe the safest, backwards-compatible way is to install the srpm to a specific topdir and then rebuild it with -bs.

$ rpm --define "_topdir $PWD" -iv /tmp/koji-1.17.0-1.el8.src.rpm 
koji-1.17.0-1.el8.noarch
$ rpmbuild --define "_topdir $PWD" -bs SPECS/*.spec
Wrote: /tmp/temp-work.yRaT3g/SRPMS/koji-1.17.0-1.el8.src.rpm

I'm really not sure about adding a new policy for this case. I had imagined something more like a per-tag setting

Need to add the new task handler to:

  • LEGACY_SIGNATURES in koji/tasks.py
  • _TASKS in kojiweb/index.py

Probably also need --nodeps after -bs. Rebuilding the srpm doesn't actually require the build deps, and we're likely not to have them in the chroot,

3 new commits added

  • use backward-compatible rpmbuild
  • fix logger usage
  • add task signature
2 years ago

Added fixes + backward-compatible calls.

Policy vs. tag - I think, that policy could be more usable in some cases (e.g. for some hotfix release users will never have correct disttag, so all 'hotfix-*' can rebuild srpms). But yes, it can be done manually on admin side via setting some flag in extra. Would it make sense to support both? E.g. policy working by default, but it can be overridden by extra flag? And is extra field the right one? {'rebuild_srpm': True}?

rebased onto e97e4b247276a6b9d85b87246e8831cf02d64218

2 years ago

Dropped policy, default to always rebuild. Rebuild can be skipped on per-tag basis if tag.extra['rebuild_srpm'] == False.

rebased onto fb5092d2a2b3231aea6179a24c950f8a7aa1cd92

2 years ago

We should use getBuildConfig with the event id to get the tag extra data
https://github.com/mikem23/koji-playground/commits/pagure/pr/1462

Minor: the parameter name source_srpm seems redundant. Perhaps just source_rpm, or even srpm?

Stray pasted comment: 'check out spec file, etc. from SCM'

Is the comment 'TODO: download srpm' still relevant?

Is the "chown mock" bit strictly necessary? Also, couldn't the gid of mock outside and inside the buildroot be different? Is this just to avoid a warning, or does rpmbuild actually error without this?

Overall, looks good and works on my end.

I've cleaned it a bit. About chown - I've copied this part from BuildSRPMFromSCM. On the other hand, I'm not able to trigger any error if it is not there, so I've removed it for now.

rebased onto 279e653

2 years ago

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

2 years ago

About chown - I've copied this part from BuildSRPMFromSCM.

hah, I should have checked for that :laughing:

Commit 4ef89a4 fixes this pull-request

Pull-Request has been merged by mikem

2 years ago