rpkg

Clone

#361 Add flatpak-build subcommand
Merged 5 years ago by cqi. Opened 5 years ago by otaylor.
otaylor/rpkg flatpak-build  into  master

file modified
+8 -2 
@@ -34,8 +34,8 @@ 
 

  

      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 gimmespec giturl help \
 

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

+     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 scratch-build sources \
 

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

  

      # parse main options and get command
 
@@ -152,6 +152,12 @@ 
 

              after="file"
 

              after_more=true
 

              ;;
 

+         flatpak-build)
 

+             options="--scratch --nowait"
 

+             options_target="--target"
 

+             options_string="--repo-url"
 

+             options_arches="--arches"
 

+             ;;
 

          import)
 

              options="--create"
 

              options_branch="--branch"

file modified
+164 -24 
@@ -39,6 +39,7 @@ 
 

  from six.moves import urllib
 

  from six.moves.urllib.parse import urljoin
 

  import requests
 

+ import yaml
 

  

  from pyrpkg.errors import HashtypeMixingError, rpkgError, rpkgAuthError, \
 

      UnknownTargetError
 
@@ -47,6 +48,13 @@ 
 

  from pyrpkg.sources import SourcesFile
 

  from pyrpkg.utils import cached_property, log_result, find_me
 

  

+ PY26 = sys.version_info < (2, 7, 0)
 

+ 

+ if not PY26:
 

+     import gi
 

+     gi.require_version('Modulemd', '1.0')  # raises ValueError
 

+     from gi.repository import Modulemd  # noqa
 

+ 

  

  class NullHandler(logging.Handler):
 

      """Null logger to avoid spurious messages, add a handler in app code"""
 
@@ -156,6 +164,8 @@ 
 

          self._target = target
 

          # The build target for containers within the buildsystem
 

          self._container_build_target = target
 

+         # The build target for flatpaks within the buildsystem
 

+         self._flatpak_build_target = target
 

          # The top url to our build server
 

          self._topurl = None
 

          # The user to use or discover
 
@@ -191,6 +201,8 @@ 
 

          self.lookaside_namespaced = lookaside_namespaced
 

          # Deprecates self.module_name
 

          self._repo_name = None
 

+         # API URL for the module build server
 

+         self.module_api_url = None
 

  

      # Define properties here
 

      # Properties allow us to "lazy load" various attributes, which also means
 
@@ -888,6 +900,98 @@ 
 

          self._container_build_target = '%s-%s-candidate' % (self.branch_merge, self.ns)
 

  

      @property
 

+     def flatpak_build_target(self):
 

+         """This property ensures the target for flatpak builds."""
 

+         if not self._flatpak_build_target:
 

+             self.load_flatpak_build_target()
 

+         return self._flatpak_build_target
 

+ 

+     def _find_platform_stream(self, name, stream, version=None):
 

+         """Recursively search for the platform module in dependencies to find its stream.
 

+ 

+         The stream of the 'platform' pseudo-module determines what base package set
 

+         we need for the runtime - and thus what build target we need.
 

+         """
 

+ 

+         if version is not None:
 

+             nsvc = name + ':' + stream + ':' + version
 

+         else:
 

+             nsvc = name + ':' + stream
 

+ 

+         build = self.module_get_latest_build(nsvc)
 

+ 

+         if build is None:
 

+             raise rpkgError("Cannot find any builds for module %s" % nsvc)
 

+ 

+         mmd_str = build['modulemd']
 

+ 

+         objects = Modulemd.objects_from_string(mmd_str)
 

+         modules = [o for o in objects if isinstance(o,  Modulemd.Module)]
 

+         if len(modules) != 1:
 

+             raise rpkgError("Failed to load modulemd for %s" % nsvc)
 

+ 

+         # Streams should already be expanded in the modulemd's that we retrieve
 

+         # from MBS - modules were built against a particular dependency.
 

+         def get_stream(req, req_stream):
 

+             req_stream_list = req_stream.get()
 

+             if len(req_stream_list) != 1:
 

+                 raise rpkgError("%s: stream list for '%s' is not expanded (%s)" %
 

+                                 (nsvc, req, req_stream_list))
 

+             return req_stream_list[0]
 

+ 

+         # We first look for 'platform' as a direct dependency of this module,
 

+         # before recursing into the dependencies
 

+         for dep in modules[0].peek_dependencies():
 

+             for req, req_stream in dep.peek_requires().items():
 

+                 if req == 'platform':
 

+                     return get_stream(req, req_stream)
 

+ 

+         # Now recurse into the dependencies
 

+         for dep in modules[0].peek_dependencies():
 

+             for req, req_stream in dep.peek_requires().items():
 

+                 platform_stream = self._find_platform_stream(req,
 

+                                                              get_stream(req, req_stream))
 

+                 if platform_stream:
 

+                     return platform_stream
 

+ 

+         return None
 

+ 

+     def load_flatpak_build_target(self):
 

+         """This locates a target appropriate for the runtime that the Flatpak targets."""
 

+ 

+         # Find the module we are going to build from container.yaml
 

+         yaml_path = os.path.join(self.path, "container.yaml")
 

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

+             raise rpkgError("Cannot find 'container.yaml' to determine build target.")
 

+ 

+         with open(yaml_path) as f:
 

+             container_yaml = yaml.safe_load(f)
 

+ 

+         compose = container_yaml.get('compose', {})
 

+         modules = compose.get('modules', [])
 

+         if not modules:
 

+             raise rpkgError("No modules listed in 'container.yaml'")
 

+         if len(modules) > 1:
 

+             raise rpkgError("Multiple modules listed in 'container.yaml'")
 

+         module = modules[0]
 

+ 

+         parts = module.split(':')
 

+         if len(parts) == 2:
 

+             name, stream = parts
 

+             version = None
 

+         elif len(parts) == 3:
 

+             name, stream, version = parts
 

+         else:
 

+             raise rpkgError("Module in container.yaml should be NAME:STREAM[:VERSION]")
 

+ 

+         platform_stream = self._find_platform_stream(name, stream, version=version)
 

+         if platform_stream is None:
 

+             raise rpkgError("Unable to find 'platform' module in the dependencies of '%s'; "
 

+                             "can't determine target" % module)
 

+ 

+         self._flatpak_build_target = '%s-flatpak-candidate' % platform_stream
 

+ 

+     @property
 

      def topurl(self):
 

          """This property ensures the topurl attribute"""
 

  
@@ -2824,10 +2928,14 @@ 
 

                               kojiconfig=None, kojiprofile=None,
 

                               build_client=None,
 

                               koji_task_watcher=None,
 

-                              nowait=False):
 

+                              nowait=False,
 

+                              flatpak=False):
 

+ 

          # check if repo is dirty and all commits are pushed
 

          self.check_repo()
 

-         container_target = self.target if target_override else self.container_build_target
 

+         container_target = self.target if target_override \
 

+             else self.flatpak_build_target if flatpak \
 

+             else self.container_build_target
 

  

          # This is for backward-compatibility of deprecated kojiconfig.
 

          # Signature of container_build_koji is not changed in case someone
 
@@ -2871,6 +2979,9 @@ 
 

                      raise rpkgError('Cannot override arches for non-scratch builds')
 

                  task_opts['arch_override'] = ' '.join(arches)
 

  

+             if flatpak:
 

+                 task_opts['flatpak'] = True
 

+ 

              priority = opts.get("priority", None)
 

              task_id = self.kojisession.buildContainer(source,
 

                                                        container_target,
 
@@ -2933,13 +3044,12 @@ 
 

          cmd.extend([project, srpm_name])
 

          self._run_command(cmd)
 

  

-     def module_build_cancel(self, api_url, build_id, auth_method,
 

+     def module_build_cancel(self, build_id, auth_method,
 

                              oidc_id_provider=None, oidc_client_id=None,
 

                              oidc_client_secret=None, oidc_scopes=None):
 

          """
 

          Cancel an MBS build
 

  

-         :param str api_url: URL of the MBS API
 

          :param int build_id: build ID to cancel
 

          :param str auth_method: authentication method used by the MBS
 

          :kwarg str oidc_id_provider: the OIDC provider when MBS is using OIDC
 
@@ -2953,8 +3063,8 @@ 
 

              for authentication
 

          """
 

          # Make sure the build they are trying to cancel exists
 

-         self.module_get_build(api_url, build_id)
 

-         url = self.module_get_url(api_url, build_id, action='PATCH')
 

+         self.module_get_build(build_id)
 

+         url = self.module_get_url(build_id, action='PATCH')
 

          resp = self.module_send_authorized_request(
 

              'PATCH', url, {'state': 'failed'}, auth_method, oidc_id_provider,
 

              oidc_client_id, oidc_client_secret, oidc_scopes, timeout=60)
 
@@ -2967,18 +3077,17 @@ 
 

                  'The cancellation of module build #{0} failed with:\n{1}'
 

                  .format(build_id, error_msg))
 

  

-     def module_build_info(self, api_url, build_id):
 

+     def module_build_info(self, build_id):
 

          """
 

          Show information about an MBS build
 

  

-         :param str api_url: URL of the MBS API
 

          :param int build_id: build ID to query MBS about
 

          """
 

          # Load the Koji session anonymously so we get access to the Koji web
 

          # URL
 

          self.load_kojisession(anon=True)
 

          state_names = self.module_get_koji_state_dict()
 

-         data = self.module_get_build(api_url, build_id)
 

+         data = self.module_get_build(build_id)
 

          print('Name:           {0}'.format(data['name']))
 

          print('Stream:         {0}'.format(data['stream']))
 

          print('Version:        {0}'.format(data['version']))
 
@@ -3000,14 +3109,13 @@ 
 

                  state_names[task_data.get('state', None)]))
 

              print('    Koji Task:  {0}\n'.format(koji_task_url))
 

  

-     def module_get_build(self, api_url, build_id):
 

+     def module_get_build(self, build_id):
 

          """
 

          Get an MBS build
 

-         :param api_url: a string of the URL of the MBS API
 

          :param build_id: an integer of the build ID to query MBS about
 

          :return: None or a dictionary representing the module build
 

          """
 

-         url = self.module_get_url(api_url, build_id)
 

+         url = self.module_get_url(build_id)
 

          response = requests.get(url, timeout=60)
 

          if response.ok:
 

              return response.json()
 
@@ -3020,11 +3128,42 @@ 
 

                  'The following error occurred while getting information on '
 

                  'module build #{0}:\n{1}'.format(build_id, error_msg))
 

  

-     def module_get_url(self, api_url, build_id, action='GET'):
 

+     def module_get_latest_build(self, nsvc):
 

+         """
 

+         Get the latest MBS build for a particular module. If the module is
 

+         built with multiple contexts, a random one will be returned.
 

+ 

+         :param nsvc: a NAME:STREAM:VERSION:CONTEXT to filter the query
 

+                (may be partial - e.g. only NAME or only NAME:STREAM)
 

+         :return: the latest build
 

+         """
 

+         url = self.module_get_url(None)
 

+         params = {
 

+             'nsvc': nsvc,
 

+             'order_desc_by': 'version',
 

+             'per_page': 1
 

+         }
 

+ 

+         response = requests.get(url, timeout=60, params=params)
 

+         if response.ok:
 

+             j = response.json()
 

+             if len(j['items']) == 0:
 

+                 return None
 

+             else:
 

+                 return j['items'][0]
 

+         else:
 

+             try:
 

+                 error_msg = response.json()['message']
 

+             except (ValueError, KeyError):
 

+                 error_msg = response.text
 

+             raise rpkgError(
 

+                 'The following error occurred while getting information on '
 

+                 'module #{0}:\n{1}'.format(nsvc, error_msg))
 

+ 

+     def module_get_url(self, build_id, action='GET'):
 

          """
 

          Get the proper MBS API URL for the desired action
 

  

-         :param str api_url: a string of the URL of the MBS API
 

          :param int build_id: an integer of the module build desired. If this is
 

              set to None, then the base URL for all module builds is returned.
 

          :param str action: a string determining the HTTP action. If this is set
 
@@ -3033,7 +3172,7 @@ 
 

          :return: a string of the desired MBS API URL.
 

          :rtype: str
 

          """
 

-         url = urljoin(api_url, 'module-builds/')
 

+         url = urljoin(self.module_api_url, 'module-builds/')
 

          if build_id is not None:
 

              url = '{0}{1}'.format(url, build_id)
 

          else:
 
@@ -3172,11 +3311,10 @@ 
 

              else:
 

                  raise
 

  

-     def module_overview(self, api_url, limit=10, finished=True):
 

+     def module_overview(self, limit=10, finished=True):
 

          """
 

          Show the overview of the latest builds in MBS
 

  

-         :param str api_url: a string of the URL of the MBS API
 

          :param int limit: an integer of the number of most recent module builds
 

              to display. This defaults to 10.
 

          :param bool finished: a boolean that determines if only finished or
 
@@ -3193,7 +3331,7 @@ 
 

              'failed': 4,
 

              'ready': 5,
 

          }
 

-         baseurl = self.module_get_url(api_url, build_id=None)
 

+         baseurl = self.module_get_url(build_id=None)
 

          if finished:
 

              # These are the states when a build is finished
 

              states = [build_states['done'], build_states['ready'],
 
@@ -3339,14 +3477,13 @@ 
 

              raise rpkgError('An unsupported MBS "auth_method" was provided')
 

          return resp
 

  

-     def module_submit_build(self, api_url, scm_url, branch, auth_method,
 

+     def module_submit_build(self, scm_url, branch, auth_method,
 

                              optional=None, oidc_id_provider=None,
 

                              oidc_client_id=None, oidc_client_secret=None,
 

                              oidc_scopes=None):
 

          """
 

          Submit a module build to the MBS
 

  

-         :param api_url: a string of the URL of the MBS API
 

          :param scm_url: a string of the module's SCM URL
 

          :param branch: a string of the module's branch
 

          :param str auth_method: a string of the authentication method used by
 
@@ -3378,7 +3515,7 @@ 
 

                  'Optional arguments are not in the proper "key=value" format')
 

  

          body.update(optional_dict)
 

-         url = self.module_get_url(api_url, build_id=None, action='POST')
 

+         url = self.module_get_url(build_id=None, action='POST')
 

          resp = self.module_send_authorized_request(
 

              'POST', url, body, auth_method, oidc_id_provider, oidc_client_id,
 

              oidc_client_secret, oidc_scopes, timeout=120)
 
@@ -3399,13 +3536,12 @@ 
 

          builds = data if isinstance(data, list) else [data]
 

          return [build['id'] for build in builds]
 

  

-     def module_watch_build(self, api_url, build_ids):
 

+     def module_watch_build(self, build_ids):
 

          """
 

          Watches the first MBS build in the list in a loop that updates every 15
 

          seconds. The loop ends when the build state is 'failed', 'done', or
 

          'ready'.
 

  

-         :param str api_url: a string of the URL of the MBS API
 

          :param build_ids: a list of module build IDs
 

          :type build_ids: list[int]
 

          """
 
@@ -3423,7 +3559,7 @@ 
 

          done = False
 

          while not done:
 

              state_names = self.module_get_koji_state_dict()
 

-             build = self.module_get_build(api_url, build_id)
 

+             build = self.module_get_build(build_id)
 

              tasks = {}
 

              if 'rpms' in build['tasks']:
 

                  tasks = build['tasks']['rpms']
 
@@ -3494,6 +3630,10 @@ 
 

          :param api_url: a string of the URL of the MBS API
 

          :return: an int of the API version
 

          """
 

+ 

+         # We don't use self.module_api_url since this is used exclusively by the code
 

+         # that is loading and validating the API URL before setting it.
 

+ 

          url = '{0}/about/'.format(api_url.rstrip('/'))
 

          response = requests.get(url, timeout=60)
 

          if response.ok:

file modified
+86 -41 
@@ -31,7 +31,7 @@ 
 

  import pyrpkg.utils as utils
 

  import six
 

  

- from pyrpkg import rpkgError, log as rpkgLogger
 

+ from pyrpkg import PY26, rpkgError, log as rpkgLogger
 

  from six.moves import configparser
 

  

  
@@ -424,6 +424,7 @@ 
 

  

          # Add a common parsers
 

          self.register_build_common()
 

+         self.register_container_build_common()
 

          self.register_rpm_common()
 

  

          # Other targets
 
@@ -438,6 +439,8 @@ 
 

          self.register_container_build()
 

          self.register_container_build_setup()
 

          self.register_diff()
 

+         if not PY26:
 

+             self.register_flatpak_build()
 

          self.register_gimmespec()
 

          self.register_gitbuildhash()
 

          self.register_gitcred()
 
@@ -1379,11 +1382,54 @@ 
 

              'verrel', help='Print the name-version-release')
 

          verrel_parser.set_defaults(command=self.verrel)
 

  

+     def register_container_build_common(self):
 

+         parser = ArgumentParser(
 

+             'container_build_common', add_help=False, allow_abbrev=False)
 

+ 

+         self.container_build_parser_common = parser
 

+ 

+         parser.add_argument(
 

+             '--target',
 

+             help='Override the default target',
 

+             default=None)
 

+ 

+         parser.add_argument(
 

+             '--nowait',
 

+             action='store_true',
 

+             default=False,
 

+             help="Don't wait on build")
 

+ 

+         parser.add_argument(
 

+             '--scratch',
 

+             help='Scratch build',
 

+             action="store_true")
 

+ 

+         parser.add_argument(
 

+             '--arches',
 

+             action='store',
 

+             nargs='*',
 

+             help='Limit a scratch build to an arch. May have multiple arches.')
 

+ 

+         parser.add_argument(
 

+             '--skip-remote-rules-validation',
 

+             action='store_true',
 

+             default=False,
 

+             help="Don't check if there's a valid gating.yaml file in the repo")
 

+ 

      def register_container_build(self):
 

          self.container_build_parser = self.subparsers.add_parser(
 

              'container-build',
 

              help='Build a container',
 

-             description='Build a container')
 

+             description='Build a container',
 

+             parents=[self.container_build_parser_common])
 

+ 

+         # These arguments are specific to non-Flatpak containers
 

+         #
 

+         # --compose-id is implemented for Flatpaks as a side-effect of the internal
 

+         #      implementation, but it is unlikely to be useful to trigger through rpkg.
 

+         # --signing-intent is not implemented for Flatpaks, though it could be useful
 

+         # --repo-url makes no sense for flatpaks, since they must be built from a
 

+         #      compose of a single module.
 

  

          group = self.container_build_parser.add_mutually_exclusive_group()
 

          group.add_argument(
 
@@ -1405,35 +1451,16 @@ 
 

                                 'Cannot be used with --signing-intent or --compose-id',
 

                            nargs='+')
 

  

-         self.container_build_parser.add_argument(
 

-             '--target',
 

-             help='Override the default target',
 

-             default=None)
 

- 

-         self.container_build_parser.add_argument(
 

-             '--nowait',
 

-             action='store_true',
 

-             default=False,
 

-             help="Don't wait on build")
 

- 

-         self.container_build_parser.add_argument(
 

-             '--scratch',
 

-             help='Scratch build',
 

-             action="store_true")
 

- 

-         self.container_build_parser.add_argument(
 

-             '--arches',
 

-             action='store',
 

-             nargs='*',
 

-             help='Limit a scratch build to an arch. May have multiple arches.')
 

+         self.container_build_parser.set_defaults(command=self.container_build)
 

  

-         self.container_build_parser.add_argument(
 

-             '--skip-remote-rules-validation',
 

-             action='store_true',
 

-             default=False,
 

-             help="Don't check if there's a valid gating.yaml file in the repo")
 

+     def register_flatpak_build(self):
 

+         self.flatpak_build_parser = self.subparsers.add_parser(
 

+             'flatpak-build',
 

+             help='Build a Flatpak',
 

+             description='Build a Flatpak',
 

+             parents=[self.container_build_parser_common])
 

  

-         self.container_build_parser.set_defaults(command=self.container_build)
 

+         self.flatpak_build_parser.set_defaults(command=self.flatpak_build)
 

  

      def register_container_build_setup(self):
 

          self.container_build_setup_parser = \
 
@@ -1762,7 +1789,7 @@ 
 

          # Keep it around for backward compatibility
 

          self.container_build()
 

  

-     def container_build(self):
 

+     def container_build(self, flatpak=False):
 

          target_override = False
 

          # Override the target if we were supplied one
 

          if self.args.target:
 
@@ -1771,11 +1798,15 @@ 
 

  

          opts = {"scratch": self.args.scratch,
 

                  "quiet": self.args.q,
 

-                 "yum_repourls": self.args.repo_url,
 

                  "git_branch": self.cmd.branch_merge,
 

-                 "arches": self.args.arches,
 

+                 "arches": self.args.arches}
 

+ 

+         if not flatpak:
 

+             opts.update({
 

+                 "yum_repourls": self.args.repo_url,
 

                  "compose_ids": self.args.compose_ids,
 

-                 "signing_intent": self.args.signing_intent}
 

+                 "signing_intent": self.args.signing_intent,
 

+             })
 

  

          section_name = "%s.container-build" % self.name
 

          err_msg = "Missing %(option)s option in [%(plugin.section)s] section. " \
 
@@ -1808,6 +1839,10 @@ 
 

  

          self.check_remote_rules_gating()
 

  

+         # We use MBS to find information about the module to build into a Flatpak
 

+         if flatpak:
 

+             self.set_module_api_url()
 

+ 

          self.cmd.container_build_koji(
 

              target_override,
 

              opts=opts,
 
@@ -1815,7 +1850,11 @@ 
 

              kojiprofile=kojiprofile,
 

              build_client=build_client,
 

              koji_task_watcher=koji_cli.lib.watch_tasks,
 

-             nowait=self.args.nowait)
 

+             nowait=self.args.nowait,
 

+             flatpak=flatpak)
 

+ 

+     def flatpak_build(self):
 

+         self.container_build(flatpak=True)
 

  

      def container_build_setup(self):
 

          self.cmd.container_build_setup(get_autorebuild=self.args.get_autorebuild,
 
@@ -1923,7 +1962,7 @@ 
 

  

      def module_build(self):
 

          """Builds a module using MBS"""
 

-         api_url = self.module_api_url
 

+         self.set_module_api_url()
 

          self.module_validate_config()
 

          scm_url, branch = self.cmd.module_get_scm_info(
 

              self.args.scm_url, self.args.branch)
 
@@ -1933,7 +1972,7 @@ 
 

          if not self.args.q:
 

              print('Submitting the module build...')
 

          build_ids = self._cmd.module_submit_build(
 

-             api_url, scm_url, branch, auth_method, self.args.optional,
 

+             scm_url, branch, auth_method, self.args.optional,
 

              oidc_id_provider, oidc_client_id, oidc_client_secret, oidc_scopes)
 

          if self.args.watch:
 

              self.module_watch_build(build_ids)
 
@@ -1948,7 +1987,7 @@ 
 

  

      def module_build_cancel(self):
 

          """Cancel an MBS build"""
 

-         api_url = self.module_api_url
 

+         self.set_module_api_url()
 

          build_id = self.args.build_id
 

          auth_method, oidc_id_provider, oidc_client_id, oidc_client_secret, \
 

              oidc_scopes = self.module_get_auth_config()
 
@@ -1956,14 +1995,15 @@ 
 

          if not self.args.q:
 

              print('Cancelling module build #{0}...'.format(build_id))
 

          self.cmd.module_build_cancel(
 

-             api_url, build_id, auth_method, oidc_id_provider, oidc_client_id,
 

+             build_id, auth_method, oidc_id_provider, oidc_client_id,
 

              oidc_client_secret, oidc_scopes)
 

          if not self.args.q:
 

                  print('The module build #{0} was cancelled'.format(build_id))
 

  

      def module_build_info(self):
 

          """Show information about an MBS build"""
 

-         self.cmd.module_build_info(self.module_api_url, self.args.build_id)
 

+         self.set_module_api_url()
 

+         self.cmd.module_build_info(self.args.build_id)
 

  

      def module_build_local(self):
 

          """Build a module locally using mbs-manager"""
 
@@ -2072,14 +2112,18 @@ 
 

                  api_url.rstrip('/'), api_version)
 

          return self._module_api_url
 

  

+     def set_module_api_url(self):
 

+         self.cmd.module_api_url = self.module_api_url
 

+ 

      def module_build_watch(self):
 

          """Watch an MBS build from the command-line"""
 

          self.module_watch_build([self.args.build_id])
 

  

      def module_overview(self):
 

          """Show the overview of the latest builds in the MBS"""
 

+         self.set_module_api_url()
 

          self.cmd.module_overview(
 

-             self.module_api_url, self.args.limit,
 

+             self.args.limit,
 

              finished=(not self.args.unfinished))
 

  

      def module_validate_config(self):
 
@@ -2134,7 +2178,8 @@ 
 

          :param build_ids: a list of module build IDs
 

          :type build_ids: list[int]
 

          """
 

-         self.cmd.module_watch_build(self.module_api_url, build_ids)
 

+         self.set_module_api_url()
 

+         self.cmd.module_watch_build(build_ids)
 

  

      def new(self):
 

          new_diff = self.cmd.new()

file modified
+3 
@@ -1,10 +1,13 @@ 
 

+ libmodulemd
 

  python2-cccolutils
 

  python2-GitPython
 

+ python2-gobject-base
 

  python2-koji
 

  python2-pycurl
 

  python-six
 

  python2-rpm  # rpm-python originally
 

  python2-requests
 

+ PyYAML
 

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

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

file modified
+3 
@@ -1,10 +1,13 @@ 
 

+ libmodulemd
 

  python3-cccolutils
 

  python3-GitPython
 

+ python3-gobject-base
 

  python3-koji
 

  python3-pycurl
 

  python3-six
 

  python3-rpm  # rpm-python originally
 

  python3-requests
 

+ python3-yaml
 

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

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

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

  pycurl >= 7.19
 

  requests
 

  six >= 1.9.0
 

+ PyYAML
 

  

  # rpm-py-installer
 

  #

file modified
+44 -3 
@@ -23,6 +23,7 @@ 
 

  

  import git
 

  import pyrpkg.cli
 

+ from pyrpkg import PY26
 

  

  try:
 

      import openidc_client
 
@@ -223,7 +224,8 @@ 
 

              kojiprofile='koji',
 

              build_client=utils.build_client,
 

              koji_task_watcher=koji_cli.lib.watch_tasks,
 

-             nowait=False
 

+             nowait=False,
 

+             flatpak=False
 

          )
 

  

      def test_override_target(self):
 
@@ -250,7 +252,8 @@ 
 

              kojiprofile='koji',
 

              build_client=utils.build_client,
 

              koji_task_watcher=koji_cli.lib.watch_tasks,
 

-             nowait=False
 

+             nowait=False,
 

+             flatpak=False
 

          )
 

  

      def test_using_deprecated_kojiconfig(self):
 
@@ -286,7 +289,8 @@ 
 

              kojiprofile=None,
 

              build_client=utils.build_client,
 

              koji_task_watcher=koji_cli.lib.watch_tasks,
 

-             nowait=False
 

+             nowait=False,
 

+             flatpak=False
 

          )
 

  

      def test_use_container_build_own_config(self):
 
@@ -304,6 +308,43 @@ 
 

          self.assertEqual('koji-container', kwargs['kojiprofile'])
 

          self.assertEqual('koji', kwargs['build_client'])
 

  

+     @unittest.skipIf(
 

+         PY26,
 

+         'Skip on old Python versions where libmodulemd is not available.')
 

+     @patch('requests.get')
 

+     def test_flatpak(self, mock_get):
 

+         mock_rv = Mock()
 

+         mock_rv.ok = True
 

+         mock_rv.json.return_value = {
 

+             'auth_method': 'oidc',
 

+             'api_version': 2
 

+         }
 

+ 

+         mock_get.return_value = mock_rv
 

+ 

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

+                    'flatpak-build']
 

+ 

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

+             cli = self.new_cli()
 

+             cli.flatpak_build()
 

+ 

+         self.mock_container_build_koji.assert_called_once_with(
 

+             False,
 

+             opts={
 

+                 'scratch': False,
 

+                 'quiet': False,
 

+                 'git_branch': 'eng-rhel-7',
 

+                 'arches': None,
 

+             },
 

+             kojiconfig=None,
 

+             kojiprofile='koji',
 

+             build_client=utils.build_client,
 

+             koji_task_watcher=koji_cli.lib.watch_tasks,
 

+             nowait=False,
 

+             flatpak=True
 

+         )
 

+ 

  

  class TestClog(CliTestCase):

file added
+305 
@@ -0,0 +1,305 @@ 
 

+ import os
 

+ import subprocess
 

+ from textwrap import dedent
 

+ 

+ try:
 

+     import unittest2 as unittest
 

+ except ImportError:
 

+     import unittest
 

+ 

+ from mock import Mock, patch
 

+ import requests
 

+ 

+ from pyrpkg import PY26
 

+ from utils import CommandTestCase
 

+ 

+ EOG_MODULEMD = """
 

+ document: modulemd
 

+ version: 2
 

+ data:
 

+   name: eog
 

+   stream: f28
 

+   version: 20170629213428
 

+   summary: Eye of GNOME Application Module
 

+   description: The Eye of GNOME image viewer (eog) is the official image viewer for
 

+     the GNOME desktop. It can view single image files in a variety of formats, as
 

+     well as large image collections.
 

+   license:
 

+     module: [MIT]
 

+   dependencies:
 

+   - buildrequires:
 

+       flatpak-runtime: [f28]
 

+     requires:
 

+       flatpak-runtime: [f28]
 

+   profiles:
 

+     default:
 

+       rpms: [eog]
 

+   components:
 

+     rpms: {}
 

+   xmd:
 

+     mbs: OMITTED
 

+ """
 

+ 

+ FLATPAK_RUNTIME_MODULEMD = """

I know that similar testing scenario is used also in other tests, but what about placing so large input data to external files somehow? But I do not require it.

 
+ document: modulemd
 

+ version: 2
 

+ data:
 

+   name: flatpak-runtime
 

+   stream: f28
 

+   version: 20170701152209
 

+   summary: Flatpak Runtime
 

+   description: Libraries and data files shared between applications
 

+   api:
 

+     rpms: [librsvg2, gnome-themes-standard, abattis-cantarell-fonts, rest, xkeyboard-config,
 

+       adwaita-cursor-theme, python3-gobject-base, json-glib, zenity, gsettings-desktop-schemas,
 

+       glib-networking, gobject-introspection, gobject-introspection-devel, flatpak-rpm-macros,
 

+       python3-gobject, gvfs-client, colord-libs, flatpak-runtime-config, hunspell-en-GB,
 

+       libsoup, glib2-devel, hunspell-en-US, at-spi2-core, gtk3, libXtst, adwaita-gtk2-theme,
 

+       libnotify, adwaita-icon-theme, libgcab1, libxkbcommon, libappstream-glib, python3-cairo,
 

+       gnome-desktop3, libepoxy, hunspell, libgusb, glib2, enchant, at-spi2-atk]
 

+   dependencies:
 

+   - buildrequires:
 

+       platform: [f28]
 

+     requires:
 

+       platform: [f28]
 

+   license:
 

+     module: [MIT]
 

+   profiles:
 

+     buildroot:
 

+       rpms: [flatpak-rpm-macros, flatpak-runtime-config]
 

+     runtime:
 

+       rpms: [libwayland-server, librsvg2, libX11, libfdisk, adwaita-cursor-theme,
 

+         libsmartcols, popt, gdbm, libglvnd, openssl-libs, gobject-introspection, systemd,
 

+         ncurses-base, lcms2, libpcap, crypto-policies, fontconfig, libacl, libwayland-cursor,
 

+         libseccomp, gmp, jbigkit-libs, bzip2-libs, libunistring, freetype, nettle,
 

+         libidn, python3-six, gtk2, gtk3, ca-certificates, libdrm, rest, lzo, libcap,
 

+         gnutls, pango, util-linux, basesystem, p11-kit, libgcab1, iptables-libs, dbus,
 

+         python3-gobject-base, cryptsetup-libs, krb5-libs, sqlite-libs, kmod-libs,
 

+         libmodman, libarchive, enchant, libXfixes, systemd-libs, shared-mime-info,
 

+         coreutils-common, libglvnd-glx, abattis-cantarell-fonts, cairo, audit-libs,
 

+         libwayland-client, libpciaccess, sed, libgcc, libXrender, json-glib, libxshmfence,
 

+         glib-networking, libdb, fedora-modular-repos, keyutils-libs, hwdata, glibc,
 

+         libproxy, python3-pyparsing, device-mapper, libgpg-error, system-python, shadow-utils,
 

+         libXtst, libstemmer, dbus-libs, libpng, cairo-gobject, libXau, pcre, python3-packaging,
 

+         at-spi2-core, gawk, mesa-libglapi, libXinerama, adwaita-gtk2-theme, libX11-common,
 

+         device-mapper-libs, python3-appdirs, libXrandr, bash, glibc-common, libselinux,
 

+         elfutils-libs, libxkbcommon, libjpeg-turbo, libuuid, atk, acl, libmount, lz4-libs,
 

+         ncurses, libgusb, glib2, python3, libpwquality, at-spi2-atk, libattr, libcrypt,
 

+         gnome-themes-standard, libtiff, harfbuzz, libstdc++, libXcomposite, xkeyboard-config,
 

+         libxcb, libnotify, systemd-pam, readline, libXxf86vm, python3-cairo, gtk-update-icon-cache,
 

+         python3-pip, mesa-libEGL, zenity, python3-gobject, libXcursor, tzdata, gvfs-client,
 

+         libverto, libblkid, cracklib, libusbx, libcroco, libdatrie, gdk-pixbuf2, libXi,
 

+         qrencode-libs, python3-libs, graphite2, mesa-libwayland-egl, mesa-libGL, pixman,
 

+         libXext, glibc-all-langpacks, info, grep, fedora-modular-release, setup, zlib,
 

+         libtasn1, libepoxy, hunspell, libsemanage, python3-setuptools, fontpackages-filesystem,
 

+         libsigsegv, hicolor-icon-theme, libxml2, expat, libgcrypt, emacs-filesystem,
 

+         gsettings-desktop-schemas, chkconfig, xz-libs, mesa-libgbm, libthai, coreutils,
 

+         colord-libs, libcap-ng, flatpak-runtime-config, elfutils-libelf, hunspell-en-GB,
 

+         libsoup, pam, hunspell-en-US, jasper-libs, p11-kit-trust, avahi-libs, elfutils-default-yama-scope,
 

+         libutempter, adwaita-icon-theme, ncurses-libs, libidn2, system-python-libs,
 

+         libffi, libXdamage, libglvnd-egl, libXft, cups-libs, ustr, libcom_err, libappstream-glib,
 

+         gnome-desktop3, gdk-pixbuf2-modules, libsepol, filesystem, gzip, mpfr]
 

+     sdk:
 

+       rpms: [gcc]
 

+   components:
 

+     rpms: {}
 

+   xmd:
 

+     flatpak:
 

+       # This gives information about how to map this module into Flatpak terms
 

+       # this is used when building application modules against this module.
 

+       branch: f28
 

+       runtimes: # Keys are profile names
 

+         runtime:
 

+           id: org.fedoraproject.Platform
 

+           sdk: org.fedoraproject.Sdk
 

+         sdk:
 

+           id: org.fedoraproject.Sdk
 

+           runtime: org.fedoraproject.Platform
 

+     mbs: OMITTED
 

+ """  # noqa
 

+ 

+ UNEXPANDED_MODULEMD = """
 

+ document: modulemd
 

+ version: 2
 

+ data:
 

+   name: nodeps
 

+   stream: f28
 

+   version: 20181234567890
 

+   summary: No dependencies
 

+   description: This module has no deps
 

+   license:
 

+     module: [MIT]
 

+   dependencies:
 

+   - buildrequires:
 

+       platform: [f27, f28]
 

+     requires:
 

+       platform: [f27, f28]
 

+   components:
 

+     rpms: {}
 

+ """
 

+ 

+ NODEPS_MODULEMD = """
 

+ document: modulemd
 

+ version: 2
 

+ data:
 

+   name: nodeps
 

+   stream: f28
 

+   version: 20181234567890
 

+   summary: No dependencies
 

+   description: This module has no deps
 

+   license:
 

+     module: [MIT]
 

+   dependencies: []
 

+   components:
 

+     rpms: {}
 

+ """
 

+ 

+ BUILDS = {
 

+     'eog:f28': [
 

+         {'modulemd': EOG_MODULEMD}
 

+     ],
 

+     'eog:f28:20170629213428': [
 

+         {'modulemd': EOG_MODULEMD}
 

+     ],
 

+     'flatpak-runtime:f28': [
 

+         {'modulemd': FLATPAK_RUNTIME_MODULEMD}
 

+     ],
 

+     'bad-modulemd:f28': [
 

+         {'modulemd': "BLAH"}
 

+     ],
 

+     'unexpanded:f28': [
 

+         {'modulemd': UNEXPANDED_MODULEMD}
 

+     ],
 

+     'nodeps:f28': [
 

+         {'modulemd': NODEPS_MODULEMD}
 

+     ],
 

+ }
 

+ 

+ 

+ @unittest.skipIf(
 

+     PY26,
 

+     'Skip on old Python versions where libmodulemd is not available.')
 

+ class FlatpakBuildCase(CommandTestCase):
 

+     def set_container_modules(self, container_modules):
 

+         with open(os.path.join(self.repo_path, 'container.yaml'), 'w') as f:
 

+             f.write(dedent("""\
 

+                 compose:
 

+                     modules: {0}
 

+                 """.format(container_modules)))
 

+         git_cmds = [
 

+             ['git', 'add', 'container.yaml'],
 

+             ['git', 'commit', '-m', 'Update container.yaml'],
 

+         ]
 

+         for cmd in git_cmds:
 

+             self.run_cmd(cmd, cwd=self.repo_path,
 

+                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 

+         git_cmds = [
 

+             ['git', 'fetch', 'origin'],
 

+             ['git', 'reset', '--hard', 'origin/master'],
 

+         ]
 

+         for cmd in git_cmds:
 

+             self.run_cmd(cmd, cwd=self.cloned_repo_path,
 

+                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 

+ 

+     def setUp(self):
 

+         super(FlatpakBuildCase, self).setUp()
 

+ 

+         self.cmd = self.make_commands()
 

+         self.cmd.module_api_url = "https://mbs.example.com/module-build-service/1/"
 

+ 

+         self.requests_get_p = patch('requests.get')
 

+         self.mock_requests_get = self.requests_get_p.start()
 

+ 

+         def mock_get(url, params=None, timeout=None):
 

+             nsvc = params['nsvc']
 

+             del params['nsvc']
 

+             self.assertEquals(params, {
 

+                 'order_desc_by': 'version',
 

+                 'per_page': 1
 

+             })
 

+ 

+             response = Mock(requests.Response)
 

+             response.ok = True
 

+             response.json.return_value = {'items': BUILDS.get(nsvc, [])}
 

+ 

+             return response
 

+ 

+         self.mock_requests_get.side_effect = mock_get
 

+ 

+         self.load_krb_user_p = patch('pyrpkg.Commands._load_krb_user')
 

+         self.mock_load_krb_user = self.load_krb_user_p.start()
 

+ 

+         session = Mock()
 

+         self.kojisession = session
 

+         session.system.listMethods.return_value = ['buildContainer']
 

+ 

+         def load_kojisession(self):
 

+             self._kojisession = session
 

+ 

+         self.load_kojisession_p = patch('pyrpkg.Commands.load_kojisession',
 

+                                         new=load_kojisession)
 

+         self.mock_load_kojisession = self.load_kojisession_p.start()
 

+ 

+         session.getBuildTarget.return_value = {'dest_tag': 'f28-flatpak'}
 

+         session.getTag.return_value = {'locked': False}
 

+ 

+     def tearDown(self):
 

+         self.requests_get_p.stop()
 

+         self.load_krb_user_p.stop()
 

+         self.load_kojisession_p.stop()
 

+ 

+         super(FlatpakBuildCase, self).tearDown()
 

+ 

+     def test_find_target(self):
 

+         self.set_container_modules(['eog:f28'])
 

+         assert self.cmd.flatpak_build_target == 'f28-flatpak-candidate'
 

+ 

+     def test_find_target_version(self):
 

+         self.set_container_modules(['eog:f28:20170629213428'])
 

+         assert self.cmd.flatpak_build_target == 'f28-flatpak-candidate'
 

+ 

+     def module_failure(self, container_modules, exception_str):
 

+         if container_modules is not None:
 

+             self.set_container_modules(container_modules)
 

+         with self.assertRaises(Exception) as e:
 

+             self.cmd.load_flatpak_build_target()
 

+         self.assertIn(exception_str, str(e.exception))
 

+ 

+     def test_find_target_no_container_yaml(self):
 

+         self.module_failure(None, "Cannot find 'container.yaml'")
 

+ 

+     def test_find_target_no_modules(self):
 

+         self.module_failure([], "No modules listed in 'container.yaml'")
 

+ 

+     def test_find_target_multiple_modules(self):
 

+         self.module_failure(['eog:f28', 'foo:f28'],
 

+                             "Multiple modules listed in 'container.yaml'")
 

+ 

+     def test_find_target_bad_nsv(self):
 

+         self.module_failure(['NOT_A_MODULE'], "should be NAME:STREAM[:VERSION]")
 

+ 

+     def test_find_target_no_builds(self):
 

+         self.module_failure(['eog:f1'], "Cannot find any builds for module")
 

+ 

+     def test_find_target_bad_modulemd(self):
 

+         self.module_failure(['bad-modulemd:f28'], "Failed to load modulemd")
 

+ 

+     def test_find_target_unexpected(self):
 

+         self.module_failure(['unexpanded:f28'], "stream list for 'platform' is not expanded")
 

+ 

+     def test_find_target_no_platform(self):
 

+         self.module_failure(['nodeps:f28'], "Unable to find 'platform' module in the dependencies")
 

+ 

+     def test_flatpak_build(self):
 

+         self.set_container_modules(['eog:f28'])
 

+         self.cmd.container_build_koji(nowait=True, flatpak=True)
 

+ 

+         session = self.kojisession
 

+         session.getBuildTarget.assert_called_with('f28-flatpak-candidate')
 

+         session.getTag.assert_called_with('f28-flatpak')
 

+ 

+         session.buildContainer.assert_called()
 

+         args, kwargs = session.buildContainer.call_args
 

+         source, container_target, taskinfo = args
 

+ 

+         assert container_target == 'f28-flatpak-candidate'

file modified
+5 -2 
@@ -3,9 +3,12 @@ 
 

  

  [testenv]
 

  skip_install = True
 

- deps =
 

+ base_deps =
 

      rpm-py-installer
 

      -r{toxinidir}/requirements/test-pypi.txt
 

+ deps =
 

+     {[testenv]base_deps}
 

+     PyGObject
 

  commands =
 

      nosetests {posargs}
 

  setenv=
 
@@ -17,7 +20,7 @@ 
 

      # Since this version, Python 2.6 support has been dropped.
 

      pyOpenSSL<18.0.0
 

      unittest2
 

-     {[testenv]deps}
 

+     {[testenv]base_deps}
 

  

  [testenv:flake8]
 

  basepython = python3

The flatpak-build subcommand starts a build of the current git repository as a Flatpak. It's pretty much identical to container-build, but the build target is determined by looking up the module that will be built into the flatpak in container.yaml and then determining what platform version that uses by traversing module dependencies.

Missing pygobject in pypi.txt

It would be good to list libmodulemd in fedora-py[23].txt as well.

What's the reason to use assert here? If the req_stream_list is expected to have only one stream, should it raise a rpkgError from here?

In the first call to _find_platform_stream, there could be version passed. But, here version is not pass anymore. Is this expected? What cases make it unnecessary to pass version argument in recursive calls?

I'm thinking if these two for-loops could be merged into one, e.g.

for dep in modules[0].props.dependencies:
    if 'platformat' in dep.props.requires:
        # Do what in the first loop
    else:
        # Do what in the second loop

This merge would be much straightforward to understand the logic than reading through all lines of two nested for-loops.

So, the logic is if a module depends on platform, just return the platform stream, otherwise let's go through each dependency recursively to find out the possible dependent platform stream.

Is my understand correct?

I'm a little worried about the performance of this method due to the recursive calls.

What would be the data size of module's dependencies and the depth of module dependencies generally?

At least in my understanding, the ModuleMD for a module that has been built in MBS should already have streams expanded - that means that there should be exactly one stream in the stream list for the build. If this is not true, the assumptions this code is making is wrong, hence the assert. But there would be no problem making it a more verbose error message - I'll update the patch.

It's possible, though not particularly useful, to put a particular version (within a stream) in the container.yaml to build the Flatpak out of a particular build of a module, rather than just using the latest from a stream. Since this is supported by OSBS when building the container, it makes sense to support it here. But when we are traversing the dependencies of the module, dependencies are only specified by stream, and not by version, so we'll never have a version to pass in.

The reasoning for the double loops is to avoid recursing and making another network round trip when we already have a 'platform' requirement directly for this module. I think this is worthwhile from a performance point of view, and will add an explanatory comment.

The typical situation is:
application module => flatpak-runtime module => platform

So two modulemd's are queried from the network and then platform is found. This surprisingly takes a couple of seconds (MBS may need some optimization) but it's certainly not a big issue when the build that is being started will take ~5 minutes.

Can you follow the subparser register style of build, chain-build and scratch-build?

That is to define common arguments in a separate method, and register container-build and flatpak-build separately with the common arguments.

it's certainly not a big issue when the build that is being started will take ~5 minutes.

Sorry, what does it mean?

I mean that flatpak-build is a long running operation (like other builds),
so if looking up the target via the module build server takes a couple of
seconds, that's not going to annoy the user very much. In other contexts a
couple of seconds could be considered a long time.

On Tue, Aug 7, 2018, 11:32 AM Chenxiong Qi pagure@pagure.io wrote:

cqi commented on the pull-request: Add flatpak-build subcommand that you
are following:
``

it's certainly not a big issue when the build that is being started will
take ~5 minutes.

Sorry, what does it mean?
``

To reply, visit the link below or just reply to this email
https://pagure.io/rpkg/pull-request/361

When I run it locally several times, this test takes 3 seconds in average.
Edited 5 years ago by cqi

The tests should be really fast, since they don't do any network traffic.
What I'm talking about taking time is the actual build in OSBS that is
started via Koji. I hope that makes sense. Sorry for the confusion! Do you
still have concerns about the performance of the target lookup?

On Tue, Aug 7, 2018, 11:46 AM Chenxiong Qi pagure@pagure.io wrote:

cqi commented on the pull-request: Add flatpak-build subcommand that you
are following:
When I ran it locally several times, this test takes 3 seconds in average.

To reply, visit the link below or just reply to this email
https://pagure.io/rpkg/pull-request/361

On 08/07/2018 05:51 PM, Owen Taylor wrote:

otaylor commented on the pull-request: Add flatpak-build subcommand that you are following:
``
The tests should be really fast, since they don't do any network traffic.
What I'm talking about taking time is the actual build in OSBS that is
started via Koji. I hope that makes sense. Sorry for the confusion! Do you
still have concerns about the performance of the target lookup?

The target lookup traverses through built modules. I'm actually confused
why you mentioned the flatpak build could take long time to build. I'm
not clear the connection between your explanation and target lookup
connection. :)

It's ok at this moment. It's not a blocker for us to move forward.

On Tue, Aug 7, 2018, 11:46 AM Chenxiong Qi pagure@pagure.io wrote:

cqi commented on the pull-request: Add flatpak-build subcommand that you
are following:
When I ran it locally several times, this test takes 3 seconds in average.

To reply, visit the link below or just reply to this email
https://pagure.io/rpkg/pull-request/361

``

To reply, visit the link below or just reply to this email
https://pagure.io/rpkg/pull-request/361

--
Regards,
Chenxiong Qi

One comment: can the dependency on libmodulemd be made optional and raise an error if a flatpak build is attempted without it? Otherwise all users of rpkg will be forced to have it available, which could be difficult. For example rhpkg on RHEL 6 would break, since getting libmodulemd is not really possible there. People wanting to build flatpaks are probably using something slightly newer anyway, so it would not limit them.

While it might be possible to install pygobject from pypy, there would be lots of -devel package requirements, and I think it's better to install it from rpms. So I've added it to fedora-py[23].txt instead.

rebased onto 9005b541460de69eac483ec4c71a439d6f240f72
5 years ago

1 new commit added

  • Make the libmodulemd import optional
5 years ago

OK, I repushed my changes with (I think) all of @cqi's comments addressed, then added a commit on top to make the libmodulemd import optional as suggested by @lsedlar

While it might be possible to install pygobject from pypy, there would be lots of -devel package requirements, and I think it's better to install it from rpms. So I've added it to fedora-py[23].txt instead.

fedora-py[23].txt does not aim to list additional dependent packages for pypi.txt. It lists all packages required only for users who want to install from Fedora packages rather than PyPI.

All dependent Python packages should be added to pypi.txt, which is for many things for rpkg to be a Python package. -devel packages have to be installed from distro package repositories, which is a common case and acceptable.

Why write container.yaml here? Tests under tests/commands/ do not test flatpak and container build.

Hi @otaylor thanks for updating this PR. Generally it looks good to me. But still have issues, can you please fix them as well?

  • As my comment mentioned, pygobject should be listed in pypi.txt
  • You have commit which is not signed off

fedora-py[23].txt does not aim to list additional dependent packages for pypi.txt. It lists all packages required only for users who want to install from Fedora packages rather than PyPI.
All dependent Python packages should be added to pypi.txt, which is for many things for rpkg to be a Python package. -devel packages have to be installed from distro package repositories, which is a common case and acceptable.

OK, will add.

Why write container.yaml here? Tests under tests/commands/ do not test flatpak and container build.

Oops, forgot to add a file! - the tests of the new logic in pyrpkg.__init__.py mostly was meant to be in tests/commands/test_flatpak_build.py, which is why I added additional set up in tests/commands/__init__.py.

Will push a new build with the pypi.txt addition, the missing file, and the missing sign-off.
Edited 5 years ago by otaylor

3 new commits added

  • Make the libmodulemd import optional
  • Add flatpak-build subcommand
  • Don't pass the MBS API URL around as a parameter
5 years ago

Oops, forgot to add a file! - the tests of the new logic in pyrpkg.init.py mostly was meant to be in tests/commands/test_flatpak_build.py, which is why I added additional set up in tests/commands/init.py.

I see. test_cli.py is the right place to write tests for commands, like flatpak-build. tests/commands/ includes legacy tests that haven't been rewritten.

Pretty please pagure-ci rebuild

As far as I could see,tests_cli.py is testing the logic in cli.py - and the tests there basically just check that the methods on Commands get called with the right arguments. What I put into tests/commands/test_flatpak_build.py is more like the tests in tests/tests_commands.py but I thought it was better to keep it in a separate file. I can either merge it into test_commands.pyor I can make a separate file directly under tests/ - tests/test_flatpak.py. What do you think is better?

I see. test_cli.py is the right place to write tests for commands, like flatpak-build. tests/commands/ includes legacy tests that haven't been rewritten.

I just see file tests/commands/test_flatpak_build.py, which tests relative methods in Commands. So, please move those test to test_commands.py, or at least inherit from new CommandTestCase in tests/utils.py. It has some convenient methods to help you create git repos and make instance of Commands.

So, please move those test to test_commands.py, or at least inherit from new CommandTestCase in tests/utils.py. It has some convenient methods to help you create git repos and make instance of Commands.

OK, I'll look at that tomorrow.

Jenkins job is updated and tests succeed to start. Some new tests fail.

rebased onto 819e78a473568a77825d8172ef3bff32e19cc26d
5 years ago

4 new commits added

  • Make the libmodulemd import optional
  • Add flatpak-build subcommand
  • TestContainerBuildWithKoji: tear down the mock appropriately
  • Don't pass the MBS API URL around as a parameter
5 years ago

I moved tests/commands/test_flatpak_build.py to tests/test_flatpak_build.py and changed it to depend on utils.CommandTestCase - if you would like it merged into test_commands.py for consistency let me know.

Hopefully the last push will also fix the test failures - it changes the usage of libmodulemd to avoid some constructs that have been problematical in past versions of libmodulemd. I'm not sure which version of libmodulemd the tests ran against - they work fine with the latest version in f27 and f28.

pretty please pagure-ci rebuild.

4 new commits added

  • Make the libmodulemd import optional
  • Add flatpak-build subcommand
  • TestContainerBuildWithKoji: tear down the mock appropriately
  • Don't pass the MBS API URL around as a parameter
5 years ago

One last round of flake8 fixes and the tests pass!

I moved tests/commands/test_flatpak_build.py to tests/test_flatpak_build.py and changed it to depend on utils.CommandTestCase - if you would like it merged into test_commands.py for consistency let me know.

It's fine to it here at this moment. I've been considering to split test_commands.py and test_cli.py actually, which already have many tests.

rpkg has to run with Python 2.6. So, we have to use compatible format syntax. Here, {} should be replaced with {0}, and others.

Tests fail without libmodulemd is installed, which is not installed as BuildRequires for building EL6 package. These tests could probably be skipped if libmodulemd is not installed.

======================================================================
ERROR: test_find_target (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 246, in test_find_target
    assert self.cmd.flatpak_build_target == 'f28-flatpak-candidate'
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 908, in flatpak_build_target
    self.load_flatpak_build_target()
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 990, in load_flatpak_build_target
    platform_stream = self._find_platform_stream(name, stream, version=version)
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 931, in _find_platform_stream
    objects = Modulemd.objects_from_string(mmd_str)
NameError: global name 'Modulemd' is not defined

======================================================================
ERROR: test_find_target_version (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 250, in test_find_target_version
    assert self.cmd.flatpak_build_target == 'f28-flatpak-candidate'
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 908, in flatpak_build_target
    self.load_flatpak_build_target()
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 990, in load_flatpak_build_target
    platform_stream = self._find_platform_stream(name, stream, version=version)
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 931, in _find_platform_stream
    objects = Modulemd.objects_from_string(mmd_str)
NameError: global name 'Modulemd' is not defined

======================================================================
ERROR: test_flatpak_build (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 286, in test_flatpak_build
    self.cmd.container_build_koji(nowait=True, flatpak=True)
  File "/builddir/build/BUILD/rpkg-1.55/pyrpkg/__init__.py", line 2887, in container_build_koji
    raise rpkgError("libmodulemd is required to build Flatpaks")
rpkgError: libmodulemd is required to build Flatpaks

======================================================================
FAIL: test_find_target_bad_modulemd (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 276, in test_find_target_bad_modulemd
    self.module_failure(['bad-modulemd:f28'], "Failed to load modulemd")
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 257, in module_failure
    self.assertIn(exception_str, str(e.exception))
AssertionError: 'Failed to load modulemd' not found in "global name 'Modulemd' is not defined"
    """Fail immediately, with the given message."""
>>  raise self.failureException('\'Failed to load modulemd\' not found in "global name \'Modulemd\' is not defined"')


======================================================================
FAIL: test_find_target_no_platform (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 282, in test_find_target_no_platform
    self.module_failure(['nodeps:f28'], "Unable to find 'platform' module in the dependencies")
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 257, in module_failure
    self.assertIn(exception_str, str(e.exception))
AssertionError: "Unable to find 'platform' module in the dependencies" not found in "global name 'Modulemd' is not defined"
    """Fail immediately, with the given message."""
>>  raise self.failureException('"Unable to find \'platform\' module in the dependencies" not found in "global name \'Modulemd\' is not defined"')


======================================================================
FAIL: test_find_target_unexpected (test_flatpak_build.FlatpakBuildCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 279, in test_find_target_unexpected
    self.module_failure(['unexpanded:f28'], "stream list for 'platform' is not expanded")
  File "/builddir/build/BUILD/rpkg-1.55/tests/test_flatpak_build.py", line 257, in module_failure
    self.assertIn(exception_str, str(e.exception))
AssertionError: "stream list for 'platform' is not expanded" not found in "global name 'Modulemd' is not defined"
    """Fail immediately, with the given message."""
>>  raise self.failureException('"stream list for \'platform\' is not expanded" not found in "global name \'Modulemd\' is not defined"')
Edited 5 years ago by cqi

The tests should definitely just be skipped. Do you have a vagrantfile or Dockerfile that you use to run tests with EL6?

Not yet. I just run tests locally inside a Python virtualenv with tox. python26 has to be installed. Do you have any problem in this way?

I think those failure tests should be skipped if libmodulemd is not available as in EL6 libmodulemd will not be a BuildRequires in the SPEC.

I think those failure tests should be skipped if libmodulemd is not available as in EL6 libmodulemd will not be a BuildRequires in the SPEC.

Just realize that this is not correct. Instead, the skip criteria should be "if python version 2.6". Because, generally, libmodulemd is installed in developer's system, those tests should always run except with python26 interpreter.

Hi @otaylor, there is actually an issue to run tests with Python 2.6 with tox. I just created a PR https://pagure.io/rpkg/pull-request/378, FYI.

I think those failure tests should be skipped if libmodulemd is not available as in EL6 libmodulemd will not be a BuildRequires in the SPEC.

Just realize that this is not correct. Instead, the skip criteria should be "if python version 2.6". Because, generally, libmodulemd is installed in developer's system, those tests should always run except with python26 interpreter.

You could do it as skip if "import libmodulemd" fails. But I went with your suggestion of sys.version_info >= (2, 7, 0) as more robust - I don't want to start skipping the flatpak tests if libmodulemd imports happen to start failing.

I had ended up with removing PyGObject from test-pypi.txt and in tox.ini

[testenv]
deps =
     rpm-py-installer
     PyGObject
     -r{toxinidir}/requirements/test-pypi.txt

[testenv:py26]
deps =
    rpm-py-installer
    -r{toxinidir}/requirements/test-pypi.txt

Since PyGObject doesn't compile with the Python 2.6 headers. Is there a better way to handle this?

rebased onto b0827ff493c4717586ff040b903261fe0ed52b8f
5 years ago

Pretty please pagure-ci rebuild

Hi @otaylor, there is actually an issue to run tests with Python 2.6 with tox. I just created a PR https://pagure.io/rpkg/pull-request/378, FYI.

As an alternative, is it possible to change the Jenkins environment to use the system python2-virtualenv package? It looks like the F27/F28 Fedora python-virtualenv-1.16 was patched to add back python2.6 support:

https://bodhi.fedoraproject.org/updates/FEDORA-2018-2c7e519668

(Was wondering since it seemed to work fine here.)

As an alternative, is it possible to change the Jenkins environment to use the system python2-virtualenv package? It looks like the F27/F28 Fedora python-virtualenv-1.16 was patched to add back python2.6 support:
https://bodhi.fedoraproject.org/updates/FEDORA-2018-2c7e519668
(Was wondering since it seemed to work fine here.)

I filed a ticket[1] to upgrade python2-virtualenv in Jenkins F27 slave.

[1] https://pagure.io/fedora-infrastructure/issue/7200

It is still painful to handle Python 2.6 compatibility. PyGObject had dropped Python 2.6 support since 3.9.1, that was 5 years ago. Meanwhile, libmodulemd is not available in EL6 and we have decided in above comments to skip something when that happens. Now, we have to deal with PyGObject again.

I even tried to install PyGObject from VCS link, unfortunately, looks the code base is too old:

pip install -e "git+https://gitlab.gnome.org/GNOME/pygobject.git@3.8.1#egg=PyGObject"
DEPRECATION: Python 2.6 is no longer supported by the Python core team, please upgrade your Python. A future version of pip will drop support for Python 2.6
Obtaining PyGObject from git+https://gitlab.gnome.org/GNOME/pygobject.git@3.8.1#egg=PyGObject
  Cloning https://gitlab.gnome.org/GNOME/pygobject.git (to 3.8.1) to ./.py26env/src/pygobject
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    IOError: [Errno 2] No such file or directory: '/home/cqi/code/rpkg/.py26env/src/pygobject/setup.py'

    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /home/cqi/code/rpkg/.py26env/src/pygobject/

Current situation is command flatpak-build requires libmodulemd and PyGObject is required to load that module and access APIs. The former one is not available in EL6 (where Python 2.6 is the default interpreter), which causes flatpak-build command is not usable in EL6, so I think it makes no sense to import PyGObject as well.

So, some comments based on that:

  • define a module level variable PY26
  • if not PY26, do not register flatpak-build command
  • if not PY26, skip corresponding tests (this is already done in test_flatpak_build.py)
  • in __init__.py, if not PY26, do not import yaml and gi, then do not load Modulemd
  • HAVE_LIBMODULEMD can be saved, since command is not registered and argument flatpak of container_build_koji is always False

These updates should make the code much clear and easier for package maintainers to know which dependent packages should be dropped for EL6.

Meanwhile, the variable PY26 could be reused in other places.

Sorry for taking your time again to update this PR. The idea in this comment I had in mind emerged when I ran tests locally with Python 2.6. Thank you very much.

4 new commits added

  • Don't registry flatpak-build command on Python-2.6
  • Add flatpak-build subcommand
  • TestContainerBuildWithKoji: tear down the mock appropriately
  • Don't pass the MBS API URL around as a parameter
5 years ago

I even tried to install PyGObject from VCS link, unfortunately, looks the code base is too old:

Yeah, getting that working is not realistic.

define a module level variable PY26 ... if not PY26, do not register flatpak-build command ...

That sounds a little cleaner than my approach. I've updated the patch that way. Does it look like what you imagined?

Are there any remaining concerns with the PR?

This has to be added to setup.py, otherwise it will fail while provisioning py26 testenv.

EDIT:

Sorry, please ignore this comment.
Edited 5 years ago by cqi

Cool :thumbsup: I succeeded to schedule a scratch build https://koji.stg.fedoraproject.org/koji/taskinfo?taskID=90001748

No other comments except rebase and fixing conflicts. Thank you very much.

rebased onto 5faef22
5 years ago

Rebased - tox.ini required splitting the [testenv]deps variable into deps and base_deps. Please check you are happy with how I did that.

Looks good to me :thumbsup:

@otaylor changes to tox.ini is ok for me. I have another PR opened, that refactors tox.ini.

Thank you very much again.

The comment should probably either mention flatpak builds or be removed.

Typo here: flaptaks -> flatpaks
but it's good to see I'm not the only person making this one :smile:

Nothing serious that would need to be fixed! Thanks for the patch.
Looks good!

why not raise rpkgError("Cannot find any builds for module %s", nsvc) ?

Thanks for the reviews @lsedlar and @onosek! - I've pushed a new version with those fixes.

I know that similar testing scenario is used also in other tests, but what about placing so large input data to external files somehow? But I do not require it.

4 new commits added

  • Don't registry flatpak-build command on Python-2.6
  • Add flatpak-build subcommand
  • TestContainerBuildWithKoji: tear down the mock appropriately
  • Don't pass the MBS API URL around as a parameter
5 years ago

After code-review I went further and tried to build rpkg package. I am not sure whether it is related to the code or not ...
rpkg.spec:

...
%if 0%{?rhel}
BuildRequires:  PyYAML
BuildRequires:  python-gobject-base
BuildRequires:  libmodulemd
...
%if 0%{?rhel}
Requires:       PyYAML
Requires:       python-gobject-base
Requires:       libmodulemd
...

It failed. @cqi, you can check in brew
Edited 5 years ago by onosek

The failure there: 

 test_find_target (test_flatpak_build.FlatpakBuildCase) ... /usr/bin/python: symbol lookup error: /lib64/libmodulemd.so.1: undefined symbol: g_log_structured_standard

Means that libmodulemd was built against glib2 2.56 or 2.58, while the glib2 version installed in the buildroot is 2.54. This might because of an build override that timed out or something. I don't think it has anything to do with this patch, other than it's using libmodulemd. Feel free to follow up by email.

Thank you, @otaylor for your explanation. Let's say it was a warning for anybody who will do the release and who have to deal with these dependencies.

@onosek I think internal build of libmodulemd or its dependency like glib2 should be dealt with.

Lets't merge. I have no other comments to add.

Pull-Request has been merged by cqi
5 years ago
Metadata
None
No Tags
Flags
jenkins #141
success (100%)
Build successful (commit: 82f763cc)
5 years ago
jenkins #140
success (100%)
Build successful (commit: 58598f28)
5 years ago
jenkins #137
failure
Build failed (commit: fe071c06)
5 years ago
jenkins #133
failure
Build failed (commit: 359a3d08)
5 years ago
jenkins #132
failure
Build failed (commit: 359a3d08)
5 years ago
jenkins #116
success (100%)
Build successful (commit: 0f20b7db)
5 years ago
jenkins #115
failure
Build failed (commit: 60100469)
5 years ago
jenkins #114
failure
Build failed (commit: 0c3a9489)
5 years ago
jenkins #111
failure
Build failed (commit: f9a6ced0)
5 years ago
jenkins #110
failure
Build failed (commit: f9a6ced0)
5 years ago
jenkins #109
failure
Build failed (commit: 04ee9d3d)
5 years ago
jenkins #108
failure
Build failed (commit: 2ecfeb8b)
5 years ago
jenkins #92
failure
Build failed (commit: f5a64892)
5 years ago
Changes Summary 9
+8 -2
file changed
etc/bash_completion.d/rpkg.bash
+164 -24
file changed
pyrpkg/__init__.py
+86 -41
file changed
pyrpkg/cli.py
+3 -0
file changed
requirements/fedora-py2.txt
+3 -0
file changed
requirements/fedora-py3.txt
+1 -0
file changed
requirements/pypi.txt
+44 -3
file changed
tests/test_cli.py
+305
file added
tests/test_flatpak_build.py
+5 -2
file changed
tox.ini
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.