#530 add base of spec preprocessing functionality
Closed 3 years ago by onosek. Opened 3 years ago by clime.
clime/rpkg master  into  master

file modified
+2
@@ -54,6 +54,8 @@ 

  * ``rpmlint``: check SPEC.

  * ``copr-cli``: for building package in `Fedora Copr`_.

  * ``module-build-service``: for building modules.

+ * ``preproc``: for spec file preprocessing

+ * ``rpkg-macros``: for spec file preprocessing

  

  .. _`Fedora Copr`: https://copr.fedorainfracloud.org/

  

@@ -35,7 +35,7 @@ 

      local options="--help -v -q"

      local options_value="--dist --release --user --path"

      local commands="build chain-build ci clean clog clone co container-build container-build-config commit compile copr-build diff flatpak-build \

-     gimmespec giturl help gitbuildhash import install lint local mockbuild mock-config new new-sources patch prep pull push retire scratch-build sources \

+     gimmespec giturl help gitbuildhash import install lint local mockbuild mock-config new new-sources patch prep pull push retire scratch-build sources spec \

      srpm switch-branch tag unused-patches upload verify-files verrel"

  

      # parse main options and get command
@@ -206,6 +206,9 @@ 

          sources)

              options_dir="--outdir"

              ;;

+         spec)

+             options="--sources --print"

+             ;;

          srpm)

              options="--md5"

              ;;

file modified
+1
@@ -11,3 +11,4 @@ 

  build_client = koji

  clone_config_rpms =

    bz.default-component %(module)s

+ preprocess_spec = False

file modified
+369 -5
@@ -29,7 +29,10 @@ 

  import time

  from itertools import groupby

  from multiprocessing.dummy import Pool as ThreadPool

+ from tempfile import NamedTemporaryFile

  from operator import itemgetter

+ from munch import Munch

+ from pipes import quote

  

  import git

  import requests
@@ -48,7 +51,7 @@ 

  from pyrpkg.sources import SourcesFile

  from pyrpkg.utils import (cached_property, extract_srpm, find_me,

                            is_file_tracked, is_lookaside_eligible_file,

-                           log_result)

+                           log_result, run, macro_helper_cmd)

  

  from .gitignore import GitIgnore

  
@@ -92,7 +95,7 @@ 

                   build_client, user=None,

                   dist=None, target=None, quiet=False,

                   distgit_namespaced=False, realms=None, lookaside_namespaced=False,

-                  git_excludes=None):

+                  git_excludes=None, preprocess_spec=False, base_output_path=None):

          """Init the object and some configuration details."""

  

          # Path to operate on, most often pwd
@@ -214,6 +217,19 @@ 

          self.git_excludes = git_excludes or []

          # Layout setup

          self.layout = layout.build(self.path)

+         # preproc attributes

+         self._preproc_preprocess_spec = preprocess_spec

+         self._preproc_base_output_path = base_output_path

+         self._preproc_produce_sources = False

+         self._preproc_use_existing_outdir = False

+         self._preproc_no_regenerate = False

+         self._preproc_version_bump = False

+         self._preproc_release_bump = False

+         self._preproc_spec_content = None

+         self._preproc_spec_path = None

+         self._preproc_git_props = None

+         self._preproc_outdir = None

+         self._preproc_done = False

  

      # Define properties here

      # Properties allow us to "lazy load" various attributes, which also means
@@ -676,13 +692,20 @@ 

      def load_nameverrel(self):

          """Set the release of a package."""

  

+         tmp_spec_created = False

+         if self._preproc_preprocess_spec and not self._preproc_done:

+             spec = self._final_tmp_spec()

+             tmp_spec_created = True

+         else:

+             spec = self.spec

+ 

          cmd = ['rpm']

          cmd.extend(self.rpmdefines)

          # We make sure there is a space at the end of our query so that

          # we can split it later.  When there are subpackages, we get a

          # listing for each subpackage.  We only care about the first.

          cmd.extend(['-q', '--qf', '"??%{NAME} %{EPOCH} %{VERSION} %{RELEASE}??"',

-                     '--specfile', '"%s"' % os.path.join(self.path, self.spec)])

+                     '--specfile', '"%s"' % os.path.join(self.path, spec)])

          joined_cmd = ' '.join(cmd)

          try:

              proc = subprocess.Popen(joined_cmd, shell=True,
@@ -697,13 +720,17 @@ 

                  self.log.error(err)

              raise rpkgError('Could not query n-v-r of %s: %s'

                              % (self.repo_name, e))

+         finally:

+             if tmp_spec_created:

+                 os.remove(spec)

+ 

          if err:

              self.log.debug('Errors occoured while running following command to get N-V-R-E:')

              self.log.debug(joined_cmd)

              self.log.error(err)

          if proc.returncode > 0:

              raise rpkgError('Could not get n-v-r-e from %s'

-                             % os.path.join(self.path, self.spec))

+                             % os.path.join(self.path, spec))

  

          # Get just the output, then split it by ??, grab the first and split

          # again to get ver and rel
@@ -785,6 +812,10 @@ 

              self.load_spec()

          return self._spec

  

+     @spec.setter

+     def spec(self, value):

+         self._spec = value

+ 

      def load_spec(self):

          """This sets the spec attribute"""

  
@@ -1361,6 +1392,315 @@ 

          }

          return giturl

  

+     #####################################################

+     ### PREPROCESSING HELPERS START

+     #####################################################

+ 

+     def _preprocess_spec(self, produce_sources=True,

+                          use_existing_outdir=False,

+                          no_regenerate=False):

+         """Does nothing if preprocessing is disabled.

+ 

+         Otherwise, runs preprocessing on the spec file

+         and updates the instance attributes accordingly.

+ 

+         The new spec file will be generated into a separate

+         newly created directory and self.path and self.layout

+         will be updated to point there.

+ 

+         :param bool produce_sources:     whether source files should be generated into

+                                          the outdir (needed for further building)

+         :param bool use_existing_outdir: whether we should operate on a previously

+                                          created outdir instead of creating new one

+         :param bool no_regenerate:       if the target outdir already contains a

+                                          previously generated spec file, do nothing.

+                                          In this mode, nothing will be produced even

+                                          if produce_sources is set to True.

+         """

+         # check if preprocessing is enabled first

+         if not self._preproc_preprocess_spec:

+             return

+ 

+         # preload repo information for the current self.path

+         # if not preloaded yet

+         if not self._repo:

+             self.load_repo()

+ 

+         # set the preprocessing flags

+         self._preproc_produce_sources = produce_sources

+         self._preproc_use_existing_outdir = use_existing_outdir

+         self._preproc_no_regenerate = no_regenerate

+ 

+         # do the job

+         self.spec = self._final_spec()

+ 

+         # update context

+         self.layout = layout.build(self._outdir)

+         self._path = self._outdir

+ 

+         # load rpmdefines now after path is updated

+         self.load_rpmdefines()

+ 

+         # make note that spec was already preprocessed

+         self._preproc_done = True

+ 

+     @property

+     def _outdir(self):

+         if self._preproc_outdir:

+             return self._preproc_outdir

+ 

+         dirname_base = os.path.splitext(

+             os.path.splitext(os.path.basename(self._spec_path))[0])[0]

+ 

+         cnt = len(glob.glob(os.path.join(self._preproc_base_output_path, dirname_base+'*')))

+ 

+         if not self._preproc_use_existing_outdir:

+             prefix = dirname_base + '-' + str(cnt+1) + '-'

+ 

+             if not os.path.isdir(self._preproc_base_output_path):

+                 prev_umask = os.umask(0)

+                 os.makedirs(self._preproc_base_output_path, mode=0o1777)

+                 os.umask(prev_umask)

+ 

+             self._preproc_outdir = tempfile.mkdtemp(dir=self._preproc_base_output_path, prefix=prefix)

+             return self._preproc_outdir

+ 

+         try:

+             prefix = dirname_base + '-' + str(cnt) + '-'

+             self._preproc_outdir = glob.glob(os.path.join(self._preproc_base_output_path, prefix+'*'))[0]

+             return self._preproc_outdir

+         except IndexError:

+             raise rpkgError('Could not load the latest outdir with prefix "%s" at %s'

+                             % (prefix, self._preproc_base_output_path))

+ 

+     @property

+     def _spec_content(self):

+         if not self._preproc_spec_content:

+             with open(self._spec_path, 'r') as f:

+                 self._preproc_spec_content = f.read()

+         return self._preproc_spec_content

+ 

+     @property

+     def _spec_path(self):

+         if not self._preproc_spec_path:

+             self._preproc_spec_path = self._locate_spec(self.path)

+         return self._preproc_spec_path

+ 

+     @property

+     def _git_props(self):

+         if self._preproc_git_props:

+             return self._preproc_git_props

+ 

+         git_props = Munch()

+ 

+         git_props.root = run(

+             macro_helper_cmd('git_root'),

+             capture_stderr=True, throw=False).out

+ 

+         git_props.branch = run(

+             macro_helper_cmd('git_branch'),

+             env={'GIT_ROOT': git_props.root},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.remote = run(

+             macro_helper_cmd('git_remote'),

+             env={'GIT_ROOT': git_props.root, 'GIT_BRANCH': git_props.branch},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.remote_url = run(

+             macro_helper_cmd('git_remote_url'),

+             env={'GIT_ROOT': git_props.root, 'GIT_REMOTE': git_props.remote},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.head = run(

+             macro_helper_cmd('git_head'),

+             env={'GIT_ROOT': git_props.root},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.head_short = run(

+             macro_helper_cmd('git_head_short'),

+             env={'GIT_ROOT': git_props.root, 'GIT_HEAD': git_props.head},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.status = run(

+             macro_helper_cmd('git_status'),

+             env={'GIT_ROOT': git_props.root},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.merged_tag_refs = run(

+             macro_helper_cmd('git_merged_tag_refs'),

+             env={'GIT_ROOT': git_props.root, 'GIT_HEAD': git_props.head},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.submodule_refs = run(

+             macro_helper_cmd('git_submodule_refs'),

+             env={'GIT_ROOT': git_props.root, 'GIT_HEAD': git_props.head},

+             capture_stderr=True, throw=False).out

+ 

+         git_props.remote_netloc = urllib.parse.urlparse(git_props.remote_url).netloc

+         git_props.dirty = 'yes' if (git_props.status or (

+             git_props.root and not git_props.head)) else ''

+ 

+         self._preproc_git_props = git_props

+         return self._preproc_git_props

+ 

+     def _preproc_source_params(self):

+         return '-s /usr/lib/rpkg.macros.d/git.bash'

+ 

+     def _preproc_env_params(self, spec_templ):

+         spec_templ_dir = os.path.dirname(spec_templ)

+ 

+         env_params = '-e INPUT_PATH={0} -e INPUT_DIR_PATH={1}'.format(

+             quote(spec_templ), quote(spec_templ_dir))

+ 

+         if self._preproc_produce_sources:

+             env_params += ' -e OUTDIR={0}'.format(quote(self._outdir))

+ 

+         if self._preproc_version_bump:

+             env_params += ' -e VERSION_BUMP=1'

+ 

+         if self._preproc_release_bump:

+             env_params += ' -e RELEASE_BUMP=1'

+ 

+         if self.verbose:

+             env_params += ' -e VERBOSE=1'

+ 

+         return env_params

+ 

+     def _preproc_git_props_params(self, status_file,

+                                  merged_tag_refs_file,

+                                  submodule_refs_file):

+         env_params = ''

+ 

+         merged_tag_refs_file.write(self._git_props.merged_tag_refs+'\n')

+         submodule_refs_file.write(self._git_props.submodule_refs+'\n')

+         status_file.write(self._git_props.status+'\n')

+ 

+         merged_tag_refs_file.close()

+         submodule_refs_file.close()

+         status_file.close()

+ 

+         for key, val in self._git_props.items():

+             if key == 'merged_tag_refs':

+                 env_params += ' -e GIT_MERGED_TAG_REFS_FILE={0}'.format(

+                     quote(merged_tag_refs_file.name))

+             elif key == 'submodule_refs':

+                 env_params += ' -e GIT_SUBMODULE_REFS_FILE={0}'.format(

+                     quote(submodule_refs_file.name))

+             elif key == 'status':

+                 env_params += ' -e GIT_STATUS_FILE={0}'.format(

+                     quote(status_file.name))

+             else:

+                 env_params += ' -e GIT_{0}={1}'.format(

+                     key.upper(), quote(val))

+ 

+         return env_params.strip()

+ 

+     def _locate_spec(cls, path):

+         spec_path = None

+ 

+         for f in os.listdir(path):

+             if (f.endswith('.spec') or f.endswith('.spec.rpkg')) and not f.startswith('.'):

+                 spec_path = os.path.abspath(os.path.join(path, f))

+                 break

+ 

+         if not spec_path:

+             raise rpkgError('No spec file found at {0}.'

+                             .format(path))

+         return spec_path

+ 

+     def _final_spec_text(self):

+         if not self._preproc_preprocess_spec:

+             return self._spec_content

+ 

+         source_params = self._preproc_source_params()

+         env_params = self._preproc_env_params(self._spec_path)

+ 

+         tmp0_file = NamedTemporaryFile(

+             'w', prefix='tmp_', dir='/tmp/', delete=False)

+         tmp1_file = NamedTemporaryFile(

+             'w', prefix='tmp_', dir='/tmp/', delete=False)

+         tmp2_file = NamedTemporaryFile(

+             'w', prefix='tmp_', dir='/tmp/', delete=False)

+         tmp3_file = NamedTemporaryFile(

+             'w', prefix='tmp_', dir='/tmp/', delete=False)

+ 

+         git_props_params = self._preproc_git_props_params(

+             tmp1_file, tmp2_file, tmp3_file)

+ 

+         tmp0_file.write(self._spec_content)

+         tmp0_file.close()

+ 

+         cmd = 'cat {0} | preproc -C {1} {2} {3} {4}'.format(

+             quote(tmp0_file.name), quote(self.path), source_params,

+             env_params, git_props_params)

+ 

+         try:

+             final_spec_text = run(cmd, shell=True, capture=True).out

+         finally:

+             os.unlink(tmp0_file.name)

+             os.unlink(tmp1_file.name)

+             os.unlink(tmp2_file.name)

+             os.unlink(tmp3_file.name)

+ 

+         return final_spec_text

+ 

+     def _final_spec_path(self):

+         final_spec_basename = os.path.basename(

+             self._spec_path[:-5] if self._spec_path.endswith('.rpkg') else self._spec_path)

+         return os.path.join(self._outdir, final_spec_basename)

+ 

+     def _final_spec(self):

+         if not self._preproc_preprocess_spec:

+             return self.spec

+ 

+         final_spec_path = self._final_spec_path()

+ 

+         if os.path.exists(final_spec_path) and self._preproc_no_regenerate:

+             return final_spec_path

+ 

+         with open(final_spec_path, 'w') as f:

+             f.write(self._final_spec_text()+'\n')

+ 

+         if self._preproc_produce_sources:

+             self._setup_source_symlinks(final_spec_path)

+ 

+         log.info('Wrote: {0}'.format(final_spec_path))

+         return final_spec_path

+ 

+     def _final_tmp_spec(self):

+         final_tmp_spec = NamedTemporaryFile(

+             'w', prefix='tmp_',

+             dir='/tmp',

+             delete=False)

+ 

+         self._preproc_produce_sources = False

+         final_tmp_spec.write(self._final_spec_text())

+         final_tmp_spec.close()

+ 

+         return final_tmp_spec.name

+ 

+     def _setup_source_symlinks(self, final_spec_path):

+         ts = rpm.ts()

+ 

+         try:

+             rpm_spec = ts.parseSpec(final_spec_path)

+         except ValueError as e:

+             raise rpkgError("Could not parse spec file with error: "+str(e))

+ 

+         for (filename, num, flags) in rpm_spec.sources:

+             filename = os.path.basename(filename)

+             symlink_source = os.path.join(self.path, filename)

+             symlink_dest = os.path.join(self._outdir, filename)

+ 

+             if os.path.isfile(symlink_source) and not \

+                     os.path.isfile(symlink_dest):

+                 os.symlink(symlink_source, symlink_dest)

+ 

+     #####################################################

+     ### PREPROCESSING HELPERS END

+     #####################################################

+ 

      def add_tag(self, tagname, force=False, message=None, file=None):

          """Add a git tag to the repository

  
@@ -2341,6 +2681,12 @@ 

      def clog(self, raw=False):

          """Write the latest spec changelog entry to a clog file"""

  

+         # remember orig path to write the "clog" file there

+         # becase preprocessing changes it if enabled

+         orig_path = self.path

+ 

+         self._preprocess_spec(produce_sources=False)

+ 

          spec_file = os.path.join(self.path, self.spec)

          # TODO: remove when fixed

          # Command contains workaround (undefines _changelog_trimtime) described at:
@@ -2374,7 +2720,7 @@ 

          buf.close()

  

          # Now open the clog file and write out the lines

-         with open(os.path.join(self.path, 'clog'), 'w') as clog:

+         with open(os.path.join(orig_path, 'clog'), 'w') as clog:

              clog.writelines(clog_lines)

  

      def compile(self, arch=None, short=False, builddir=None, nocheck=False,
@@ -2387,6 +2733,8 @@ 

          Logs the output and returns nothing

          """

  

+         self._preprocess_spec(produce_sources=True)

+ 

          # setup the rpm command

          cmd = ['rpmbuild']

          cmd.extend(self.rpmdefines)
@@ -2463,6 +2811,8 @@ 

             Parameter buildrootdir.

          """

  

+         self._preprocess_spec(produce_sources=True)

+ 

          # setup the rpm command

          cmd = ['rpmbuild']

          cmd.extend(self.rpmdefines)
@@ -2500,6 +2850,10 @@ 

          specified by the command line argument.

          """

  

+         self._preprocess_spec(produce_sources=False,

+                               use_existing_outdir=True,

+                               no_regenerate=True)

+ 

          # Check for srpm

          srpm = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel)

          if not os.path.exists(os.path.join(self.path, srpm)):
@@ -2563,6 +2917,8 @@ 

             Parameter buildrootdir.

          """

  

+         self._preprocess_spec(produce_sources=True)

+ 

          # This could really use a list of arches to build for and loop over

          # Get the sources

          # build up the rpm command
@@ -2872,6 +3228,8 @@ 

             Parameter buildrootdir.

          """

  

+         self._preprocess_spec(produce_sources=True)

+ 

          # setup the rpm command

          cmd = ['rpmbuild']

          cmd.extend(self.rpmdefines)
@@ -2914,6 +3272,8 @@ 

              the rpmbuild command.

          """

  

+         self._preprocess_spec(produce_sources=True)

+ 

          self.srpmname = os.path.join(self.path,

                                       "%s-%s-%s.src.rpm"

                                       % (self.repo_name, self.ver, self.rel))
@@ -3025,6 +3385,10 @@ 

             Parameter buildrootdir.

          """

  

+         self._preprocess_spec(produce_sources=False,

+                               use_existing_outdir=True,

+                               no_regenerate=True)

+ 

          # setup the rpm command

          cmd = ['rpmbuild']

          cmd.extend(self.rpmdefines)

file modified
+39 -4
@@ -259,6 +259,9 @@ 

                          for excl in items.get("git_excludes", '').split('\n')

                          if excl]

  

+         # find out if preprocessing is enabled

+         preprocess_spec = self._get_bool_opt('preprocess_spec')

+ 

          # Create the cmd object

          self._cmd = self.site.Commands(self.args.path,

                                         items['lookaside'],
@@ -276,7 +279,9 @@ 

                                         distgit_namespaced=dg_namespaced,

                                         realms=realms,

                                         lookaside_namespaced=la_namespaced,

-                                        git_excludes=git_excludes

+                                        git_excludes=git_excludes,

+                                        preprocess_spec=preprocess_spec,

+                                        base_output_path='/tmp/{0}'.format(self.name)

                                         )

  

          if self.args.module_name:
@@ -471,6 +476,7 @@ 

          self.register_retire()

          self.register_scratch_build()

          self.register_sources()

+         self.register_spec()

          self.register_srpm()

          self.register_switch_branch()

          self.register_tag()
@@ -1406,6 +1412,25 @@ 

              help='Directory to download files into (defaults to pwd)')

          sources_parser.set_defaults(command=self.sources)

  

+     def register_spec(self):

+         """Register the spec target"""

+ 

+         spec_parser = self.subparsers.add_parser(

+             'spec', help='Generate spec file from a spec template',

+             description='Preprocess a given spec template '

+             'and put the resulting spec file into outdir. The spec '

+             'template is simply copied into the outdir if '

+             'spec preprocessing is turned off or if the input spec '

+             'template does not contain any preprocessor macros.')

+         spec_parser.add_argument(

+             '--sources', action='store_true', default=False,

+             help='Enable source generation by the input '

+             'spec template. By default disabled.')

+         spec_parser.add_argument(

+             '--print', '-p', action='store_true', default=False,

+             help='Print rendered spec file to stdout.')

+         spec_parser.set_defaults(command=self.spec)

+ 

      def register_srpm(self):

          """Register the srpm target"""

  
@@ -1735,7 +1760,8 @@ 

              if self.args.srpm == 'CONSTRUCT':

                  self.log.debug('Generating an srpm')

                  self.srpm()

-                 self.args.srpm = '%s.src.rpm' % self.cmd.nvr

+                 srpm_path = os.path.join(self.cmd.path, '%s.src.rpm' % self.cmd.nvr)

+                 self.args.srpm = srpm_path

              return self._upload_file_for_build(self.args.srpm)

  

      def _watch_build_tasks(self, task_ids):
@@ -2099,9 +2125,9 @@ 

          self.log.debug('Generating an srpm')

          self.args.hash = None

          self.srpm()

-         srpm_name = '%s.src.rpm' % self.cmd.nvr

+         srpm_path = os.path.join(self.cmd.path, '%s.src.rpm' % self.cmd.nvr)

          self.cmd.copr_build(self.args.project[0],

-                             srpm_name,

+                             srpm_path,

                              self.args.nowait,

                              self.args.copr_config)

  
@@ -2598,6 +2624,15 @@ 

          outdir = getattr(self.args, 'outdir', None)

          self.cmd.sources(outdir)

  

+     def spec(self):

+         self.cmd._preproc_produce_sources = self.args.sources

+ 

+         if self.args.print:

+             final_spec_text = self.cmd._final_spec_text()

+             print(final_spec_text)

+         else:

+             self.cmd._final_spec()

+ 

      def srpm(self):

          self.sources()

  

file modified
+79
@@ -18,6 +18,12 @@ 

  import os

  import subprocess

  import sys

+ import munch

+ import tempfile

+ import textwrap

+ import logging

+ import pipes

+ import re

  

  import git

  import six
@@ -33,6 +39,9 @@ 

  

      getcwd = os.getcwdu

  

+ from pyrpkg.errors import rpkgError

+ 

+ log = logging.getLogger("pyrpkg")

  

  class cached_property(property):

      """A property caching its return value
@@ -294,3 +303,73 @@ 

      # output contains encoding ("binary", "us-ascii", ...)

      encoding = output.strip()  # strip newline at the end

      return encoding == "binary"

+ 

+ #####################################################

+ ### PREPROCESSING UTILS START

+ #####################################################

+ 

+ MACRO_HELPER_DIR = '/usr/lib/rpkg.macros.d/helpers/'

+ 

+ def run(cmd, shell=False, env=None, cwd=None, capture=True, capture_stderr=False, throw=True):

+     if capture:

+         stdout = subprocess.PIPE

+     else:

+         stdout = None

+ 

+     if capture_stderr:

+         stderr = subprocess.PIPE

+     else:

+         stderr = None

+ 

+     log.debug('Running: {0}'.format(

+         cmd_repr(cmd) if not shell else cmd))

+ 

+     process = subprocess.Popen(

+         cmd, stdout=stdout, stderr=stderr, shell=shell, env=env, cwd=cwd)

+ 

+     (out, err) = process.communicate()

+ 

+     err = err.decode('utf-8').strip('\n') if err else ''

+ 

+     if process.returncode != 0 and throw:

+         raise rpkgError(err)

+ 

+     out = out.decode('utf-8').strip('\n') if out else ''

+ 

+     return munch.Munch(out=out,

+                        err=err,

+                        rc=process.returncode)

+ 

+ 

+ def changelog_entry(commit_message):

+     entry = '- ' + commit_message.split('\n')[0] if commit_message else ''

+     return '\n'.join(textwrap.wrap(re.sub(r'(\s+)%([^%])', r'\1%%\2', entry), 80))

+ 

+ 

+ def edit(editor, text):

+     tmp_f = tempfile.NamedTemporaryFile('w', delete=False)

+     tmp_f.write(text)

+     tmp_f.close()

+     subprocess.check_call(editor.split()+[tmp_f.name])

+     tmp_f = open(tmp_f.name, 'r')

+     result = tmp_f.read()

+     tmp_f.close()

+     os.unlink(tmp_f.name)

+     return result

+ 

+ 

+ def cmd_repr(cmd):

+     quoted_items = []

+ 

+     for i in range(len(cmd)):

+         quoted_items.append(pipes.quote(cmd[i]))

+ 

+     return ' '.join(quoted_items)

+ 

+ 

+ def macro_helper_cmd(*args):

+     return [MACRO_HELPER_DIR+args[0]] + list(args[1:])

+ 

+ #####################################################

+ ### PREPROCESSING UTILS END

+ #####################################################

@@ -5,3 +5,5 @@ 

  rpm-build

  rpmlint

  module-build-service # needed for the module-buld-local command

+ rpkg-macros

+ preproc

@@ -10,6 +10,7 @@ 

  PyYAML

  # python2-openidc-client # used for MBS OIDC authentication

  # python2-requests-kerberos # used for MBS Kerberos authentication

+ python2-munch

  

  # For running tests

  python2-coverage

@@ -10,6 +10,7 @@ 

  python3-yaml

  # python3-openidc-client # used for MBS OIDC authentication

  # python3-requests-kerberos # used for MBS Kerberos authentication

+ python3-munch

  

  # For running tests

  python3-coverage

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

  six >= 1.9.0

  requests

  PyYAML

+ munch

  

  # rpm-py-installer

  #

file modified
+16 -8
@@ -1663,10 +1663,6 @@ 

          super(TestCoprBuild, self).tearDown()

  

      def assert_copr_build(self, cli_cmd, expected_copr_cli):

-         with patch('sys.argv', new=cli_cmd):

-             cli = self.new_cli()

-             cli.copr_build()

- 

          self.mock_srpm.assert_called_once()

          self.mock_run_command.assert_called_once_with(expected_copr_cli)

  
@@ -1674,18 +1670,26 @@ 

          cli_cmd = ['rpkg', '--path', self.cloned_repo_path,

                     'copr-build', 'user/project']

  

+         with patch('sys.argv', new=cli_cmd):

+             cli = self.new_cli()

+             cli.copr_build()

+ 

          self.assert_copr_build(cli_cmd, [

              'copr-cli', 'build', 'user/project',

-             '{0}.src.rpm'.format(self.mock_nvr.return_value)

+             os.path.join(cli.cmd.path, '{0}.src.rpm'.format(self.mock_nvr.return_value))

          ])

  

      def test_copr_build_no_wait(self):

          cli_cmd = ['rpkg', '--path', self.cloned_repo_path,

                     'copr-build', '--nowait', 'user/project']

  

+         with patch('sys.argv', new=cli_cmd):

+             cli = self.new_cli()

+             cli.copr_build()

+ 

          self.assert_copr_build(cli_cmd, [

              'copr-cli', 'build', '--nowait', 'user/project',

-             '{0}.src.rpm'.format(self.mock_nvr.return_value)

+             os.path.join(cli.cmd.path, '{0}.src.rpm'.format(self.mock_nvr.return_value))

          ])

  

      def test_copr_build_with_alternative_config_file(self):
@@ -1693,10 +1697,14 @@ 

                     'copr-build', '--config', '/path/to/alternative/config',

                     'user/project']

  

+         with patch('sys.argv', new=cli_cmd):

+             cli = self.new_cli()

+             cli.copr_build()

+ 

          self.assert_copr_build(cli_cmd, [

              'copr-cli', '--config', '/path/to/alternative/config',

              'build', 'user/project',

-             '{0}.src.rpm'.format(self.mock_nvr.return_value)

+             os.path.join(cli.cmd.path, '{0}.src.rpm'.format(self.mock_nvr.return_value))

          ])

  

  
@@ -3439,7 +3447,7 @@ 

          srpm_file, unique_path = args

  

          if expected_srpm_file is None:

-             self.assertEqual('{0}.src.rpm'.format(cli.cmd.nvr), srpm_file)

+             self.assertEqual(os.path.join(cli.cmd.path, '{0}.src.rpm'.format(cli.cmd.nvr)), srpm_file)

          else:

              self.assertEqual(expected_srpm_file, srpm_file)

          six.assertRegex(self, unique_path, r'^cli-build/\d+\.\d+\.[a-zA-Z]+$')

Hello,

this pull request enables spec preprocessing functionality that originated in rpkg-util. I tested this PR successfully with fedpkg tool. I haven't yet tested other tools like rfpkg but I assume they could work if they work in similar fashion to fedpkg. This complements work that was done in mock previously: https://github.com/rpm-software-management/mock/commit/bda814a743ef74e6dfbee261a4db6e2f219382d1

If this is accepted, I plan to continue the integration by extending the tag subcommand to be able to populate tag messages automatically from git commits from the latest reachable annotated tag and also add some more options (like --name, --version, or --release and possibly others to make the integration smooth).

Basically, what this commit does is that it enables usage of rpkg-macros in spec files (if preprocess_spec configuration option is set to True). This is a possible solution for automatic release and changelog generation problem but it offers more than that.

I am open to any required code changes and improvements in this PR or in future.

NOTES from the commit:

  • enabled by setting preprocess_spec to True in the config file
  • uses preproc and rpkg-macros to do the job
  • when preprocessing is enabled, the generated files will
    be put into an auto-generated directory under /tmp/<name>
    (e.g. /tmp/fedpkg for the fedpkg tool)
  • this enables preprocessing for all rpm-based commands:
    srpm, local, prep, install, compile, lint, verify-files, clog
    mockbuild, koji scratch build, copr build
  • also a new subcommand spec is introduced just to render
    the spec file without further building srpm etc.
  • some introduced functions are currently unused (e.g.
    utils.changelog_entry or utils.edit). The functions are
    intended to be used in a future commit that extends the
    tag subcommand to automatically pregenerate tag messages.
  • preproc and rpkg-macros packages are newly required,
    also python-munch

Signed-off-by: clime clime@fedoraproject.org

2 new commits added

  • fix macro_helper_cmd for python versions < 3.5
  • fix return value from _final_spec when no_regenerate and spec file exists
3 years ago

Pushed some small fixes. But not sure if it helps with the jenkins CI. There seems to be also some other import problem (from pyrpkg import layout).

rebased onto 2777480

3 years ago

pretty please pagure-ci rebuild

3 years ago

4 new commits added

  • add support for verrel command
  • fix macro_helper_cmd for python versions < 3.5
  • fix return value from _final_spec when no_regenerate and spec file exists
  • add base of spec preprocessing functionality
3 years ago

I am looking at the request. It is an interesting set of features. Currently, I am most concerned about the new dependencies.
Together with fedpkg am maintaining also the internal tool rhpkg and I am trying to keep both tools similar as much as possible. And providing comparable features. This approach would require me to maintain additional packages, that are already prepared in Fedora/koji (preproc, rpkg-macros, python-munch) for building rhpkg on the internal side. At least these direct dependencies.
Understand, that at least rpkg-macros and preproc are essential for the request.
Before I will go deeper into the review, I have to decide on this dilemma :).

init.py is getting larger and larger. I understand that it is probably correct to place functionality there, but would it be possible to think about separating it into a different module? Just exploring possibilities regarding this.

What improvements does this change enable?
Looking at rpkg-util repo, it seems to me this change is simplifying life for people who want to manage spec file outside of dist-git. However that is against Fedora packaging guidelines, which want dist-git to be the canonical location for packaging information IIRC.

@onosek yes, preproc, rpkg-macros, python-munch are needed. I can add you as a maintainer of the first two if needed.

We can place the code to a separate module, yes. I was thinking about it when I was writing this PR but waited for feedback first.

@lsedlar: This change enables automatic versioning, changelog and source generation based on git (meta)data. Something which is useful no matter where the spec file is placed. It tries to address problems that lead people to maintaining spec files outside of Fedora.

In the past days, I was thinking about it more. I am worried about the increased complexity of the code and package maintenance itself. But there is also an argument for not placing this into rpkg. I found that this feature was presented to Fedora committee and eventually rejected. It means I would have to prevent fedpkg (as a big rpkg's "customer") from using this functionality. And this, as per my opinion, considerably lowers benefits having it here.
I appreciate that you must have invested a lot of time and effort into this :-/. I am sorry, I decided to close the PR.

Pull-Request has been closed by onosek

3 years ago