#93 rpm2flatpak: Add an option to initialize from the Flathub manifest
Closed 4 months ago by nphilipp. Opened 4 months ago by otaylor.
modularity/ otaylor/fedmod init-from-flathub  into  master

file modified
+4 -2

@@ -77,6 +77,8 @@ 

  

  # modulemd generation for Flatpaks

  @_cli_commands.command()

+ @click.option("--flathub", metavar="ID_OR_SEARCH_TERM",

+               help="Initialize from a Flathub Flatpak.")

  @click.option("--output-modulemd", metavar="FILE",

                help="Write modulemd to FILE instead of <pkg>.yaml.")

  @click.option("--output-containerspec", metavar="FILE",

@@ -85,10 +87,10 @@ 

  @click.option("--force", "-f", is_flag=True,

                help="Overwriting existing output files")

  @click.argument("pkg", metavar='PKG', required=True)

- def rpm2flatpak(pkg, output_modulemd, output_containerspec, force):

+ def rpm2flatpak(pkg, output_modulemd, output_containerspec, force, flathub):

      """Generate modulemd from an RPM"""

      fg = FlatpakGenerator(pkg)

-     fg.run(output_modulemd, output_containerspec, force=force)

+     fg.run(output_modulemd, output_containerspec, force=force, flathub=flathub)

  

  

  @_cli_commands.command('flatpak-report')

file modified
+124 -4

@@ -1,5 +1,6 @@ 

  import json

  import os

+ import re

  import sys

  from textwrap import dedent

  

@@ -9,6 +10,10 @@ 

  gi.require_version('Modulemd', '1.0')  # noqa: E402

  from gi.repository import Modulemd

  

+ import requests

+ 

+ import yaml

+ 

  from . import _depchase

  from ._repodata import dataset_release_name

  from .get_module_builds import get_module_builds

@@ -16,6 +21,38 @@ 

  from .util import rpm_name_only

  

  

+ # Some PyYAML magic to get the output we want for container.yaml

+ 

+ class LiteralScalar(str):

+     """String subclass that gets dumped into yaml as a scalar"""

+     pass

+ 

+ 

+ def _represent_literal_scalar(dumper, s):

+     return dumper.represent_scalar(tag=u'tag:yaml.org,2002:str',

+                                    value=s,

+                                    style='|')

+ 

+ 

+ yaml.add_representer(LiteralScalar, _represent_literal_scalar)

+ 

+ 

+ class NoSortMapping(dict):

+     """dict subclass that dumped into yaml as a scalar without sorting keys"""

+     pass

+ 

+ 

+ def _represent_no_sort_mapping(dumper, d):

+     return yaml.MappingNode(tag='tag:yaml.org,2002:map',

+                             value=[(dumper.represent_data(k),

+                                     dumper.represent_data(v))

+                                    for k, v in d.items()],

+                             flow_style=False)

+ 

+ 

+ yaml.add_representer(NoSortMapping, _represent_no_sort_mapping)

+ 

+ 

  def dataset_release_branch():

      release_name = dataset_release_name()

      if release_name == 'rawhide':

@@ -36,6 +73,50 @@ 

      return set(mmd.peek_profiles()['runtime'].peek_rpms().get())

  

  

+ def _load_flathub_manifest(search_term):

+     response = requests.get("https://flathub.org/api/v1/apps")

+     response.raise_for_status()

+     apps = response.json()

+ 

+     matches = []

+     search_lower = search_term.lower()

+     for app in apps:

+         if (search_lower in app['flatpakAppId'].lower() or

+                 search_lower in app['name'].lower()):

+             matches.append((app['flatpakAppId'], app['name']))

+ 

+     if len(matches) > 1:

+         max_id_len = max([len(app_id) for app_id, _ in matches])

+         for app_id, name in matches:

+             print(app_id + (' ' * (max_id_len - len(app_id)) + ' ' + name))

+         raise click.ClickException("Multiple matches found on flathub.org")

+     elif len(matches) == 0:

+         raise click.ClickException("No match found on flathub.org")

+ 

+     app_id = matches[0][0]

+ 

+     for fname, is_yaml in [

+             (f"{app_id}.json", False),

+             (f"{app_id}.yaml", True),

+             (f"{app_id}.yml", True)]:

+         url = f"https://raw.githubusercontent.com/flathub/{app_id}/master/{fname}"

+         response = requests.get(url)

+         if response.status_code == 404:

+             continue

+         else:

+             break

+ 

+     response.raise_for_status()

+ 

+     if is_yaml:

+         return yaml.safe_load(response.text)

+     else:

+         # flatpak-builder supports non-standard comments in the manifest, strip

+         # them out. (Ignore the possibility of C comments embedded in strings.)

+         no_comments = re.sub(r'/\*.*?\*/', '', response.text, flags=re.DOTALL)

+         return json.loads(no_comments)

+ 

+ 

  class FlatpakGenerator(ModuleGenerator):

      def __init__(self, pkg):

          super().__init__([pkg])

@@ -68,7 +149,35 @@ 

  

          self.mmd.add_profile(default_profile)

  

-     def _write_container_yaml(self, output_fname):

+     def _flathub_container_yaml(self, manifest):

+         yml = NoSortMapping({

+             'compose': {

+                 'modules': [self.pkgs[0] + ':master'],

+             },

+             'flatpak': NoSortMapping({

+                 'id': manifest['app-id'],

+                 'branch': 'stable',

+             })

+         })

+ 

+         for key in ['command',

+                     'appstream-license',

+                     'appstream-compose',

+                     'desktop-file-name-prefix',

+                     'desktop-file-name-suffix',

+                     'rename-appdata-file',

+                     'rename-desktop-file',

+                     'rename-icon',

+                     'copy-icon']:

+             if key in manifest:

+                 yml['flatpak'][key] = manifest[key]

+ 

+         if 'finish-args' in manifest:

+             yml['flatpak']['finish-args'] = LiteralScalar('\n'.join(manifest['finish-args']))

+ 

+         return yaml.dump(yml, default_flow_style=False, indent=4)

+ 

+     def _default_container_yaml(self):

          pkg = self.pkgs[0]

          command = pkg

  

@@ -84,7 +193,7 @@ 

                  command: {command}

                  tags: []

                  # Not sandboxed. See 'man flatpak-build-finish'

-                 finish-args: >

+                 finish-args: |

                      --filesystem=host

                      --share=ipc

                      --socket=x11

@@ -92,13 +201,24 @@ 

                      --socket=session-bus

              ''')

  

+         return container_yaml

+ 

+     def _write_container_yaml(self, output_fname, flathub_manifest):

+         if flathub_manifest:

+             container_yaml = self._flathub_container_yaml(flathub_manifest)

+         else:

+             container_yaml = self._default_container_yaml()

+ 

          with open(output_fname, 'w') as f:

              f.write(container_yaml)

  

          print(f"Generated container specification: {output_fname!r}."

                f" Please edit appropriately.")

  

-     def run(self, output_modulemd, output_containerspec, force=False):

+     def run(self, output_modulemd, output_containerspec,

+             force=False, flathub=None):

+         flathub_manifest = _load_flathub_manifest(flathub) if flathub else None

+ 

          if output_modulemd is None:

              pkg = self.pkgs[0]

              output_modulemd = pkg + '.yaml'

@@ -115,7 +235,7 @@ 

  

          super().run(output_modulemd)

  

-         self._write_container_yaml(output_containerspec)

+         self._write_container_yaml(output_containerspec, flathub_manifest)

  

  

  def do_flatpak_report(pkgs, quiet=False):

file modified
+1

@@ -41,6 +41,7 @@ 

      tests_require=[

          'pytest',

          'decorator',

+         'responses',

      ],

      packages=find_packages(exclude=['tests']),

  )

file modified
+277 -5

@@ -2,7 +2,10 @@ 

  

  import logging

  import os

+ import sys

  import tempfile

+ from contextlib import contextmanager

+ from unittest.mock import Mock, patch

  

  from click.testing import CliRunner

  

@@ -12,24 +15,239 @@ 

  

  import pytest

  

+ import responses

+ 

  import yaml

  

  from _fedmod.cli import _cli_commands  # noqa: I100

  

  log = logging.getLogger(__name__)

  

+ APPS_JSON = """\

+ [{"flatpakAppId":"org.gnome.eog",

+  "name":"Eye of GNOME",

+  "summary":"Browse and rotate images",

+  "iconDesktopUrl":"/repo/appstream/x86_64/icons/128x128/org.gnome.eog.png"},

+ {"flatpakAppId":"org.gnome.FeedReader",

+  "name":"FeedReader","summary":"RSS client for various webservices",

+  "iconDesktopUrl":"/repo/appstream/x86_64/icons/128x128/org.gnome.FeedReader.png"}]

+ """

+ 

+ EOG_YAML = """\

+ app-id: org.gnome.eog

+ runtime: org.gnome.Platform

+ runtime-version: '3.30'

+ sdk: org.gnome.Sdk

+ branch: stable

+ command: eog

+ rename-desktop-file: eog.desktop

+ rename-appdata-file: eog.appdata.xml

+ rename-icon: eog

+ copy-icon: true

+ finish-args:

+   # X11 + XShm access

+   - --share=ipc

+   - --socket=x11

+ """

+ 

+ EOG_JSON = """\

+ {

+   "app-id": "org.gnome.eog",

+   "runtime": "org.gnome.Platform",

+   "runtime-version": "3.30",

+   "sdk": "org.gnome.Sdk",

+   "branch": "stable",

+   "command": "eog",

+   "rename-desktop-file": "eog.desktop",

+   "rename-appdata-file": "eog.appdata.xml",

+   "rename-icon": "eog",

+   "copy-icon": true,

+   "finish-args":

+   /* X11 + XShm access */

+   ["--share=ipc",

+    "--socket=x11"]

+ }

+ """

+ 

+ FLATPAK_RUNTIME_MODULEMD = """

+ document: modulemd

+ version: 2

+ data:

+   summary: Flatpak Runtime

+   description: This module defines two runtimes for Flatpaks.

+   license:

+     module:

+     - MIT

+   dependencies:

+   - buildrequires:

+       platform: [f29]

+     requires:

+       platform: [f29]

+   profiles:

+     runtime:

+       rpms: [

+         'flatpak-runtime-config',

+         'abattis-cantarell-fonts', 'acl', 'libacl', 'adwaita-cursor-theme',

+         'adwaita-icon-theme', 'alsa-lib', 'libargon2', 'aspell',

+         'at-spi2-atk', 'at-spi2-core', 'atk', 'attr', 'libattr', 'audit-libs',

+         'avahi-glib', 'avahi-libs', 'basesystem', 'bash', 'bluez-libs',

+         'brotli', 'bzip2', 'bzip2-libs', 'ca-certificates', 'cairo',

+         'cairo-gobject', 'cdparanoia-libs', 'chkconfig', 'clutter',

+         'clutter-gst3', 'clutter-gtk', 'cogl', 'colord-libs',

+         'compat-openssl10', 'compat-readline6', 'coreutils',

+         'coreutils-common', 'cpio', 'cracklib', 'crypto-policies',

+         'cryptsetup-libs', 'cups-libs', 'curl', 'libcurl', 'cyrus-sasl-lib',

+         'dbus', 'dbus-common', 'dbus-daemon', 'dbus-libs', 'dbus-tools',

+         'dbus-x11', 'dbus-glib', 'dconf', 'dejavu-fonts-common',

+         'dejavu-sans-fonts', 'dejavu-sans-mono-fonts', 'dejavu-serif-fonts',

+         'desktop-file-utils', 'libcom_err', 'libss', 'elfutils',

+         'elfutils-default-yama-scope', 'elfutils-libelf', 'elfutils-libs',

+         'emacs-filesystem', 'enchant', 'eosrei-emojione-fonts', 'expat',

+         'fedora-release', 'fedora-gpg-keys', 'fedora-repos', 'file',

+         'file-libs', 'filesystem', 'findutils', 'flac-libs', 'fontconfig',

+         'fontpackages-filesystem', 'freetype', 'fribidi', 'gawk', 'gc',

+         'libgcab1', 'libatomic', 'libgcc', 'libgfortran', 'libgomp',

+         'libquadmath', 'libstdc++', 'gcr', 'gdbm', 'gdbm-libs', 'gdk-pixbuf2',

+         'gdk-pixbuf2-modules', 'geoclue2', 'geoclue2-libs', 'gjs',

+         'glib-networking', 'glib2', 'glibc', 'glibc-all-langpacks',

+         'glibc-common', 'glibc-minimal-langpack', 'libnsl', 'gmp',

+         'adwaita-gtk2-theme', 'gnome-themes-extra', 'gnu-free-fonts-common',

+         'gnu-free-mono-fonts', 'gnu-free-sans-fonts', 'gnu-free-serif-fonts',

+         'gnupg', 'gnupg2', 'gnupg2-smime', 'gnutls', 'gobject-introspection',

+         'google-crosextra-caladea-fonts', 'google-crosextra-carlito-fonts',

+         'google-noto-emoji-color-fonts', 'gpgme', 'graphite2', 'grep',

+         'gsettings-desktop-schemas', 'gsm', 'gssdp', 'gstreamer1',

+         'gstreamer1-plugins-bad-free', 'gstreamer1-plugins-base', 'gtk2',

+         'gtk2-engines', 'gtk-update-icon-cache', 'gtk3', 'guile', 'gupnp',

+         'gupnp-igd', 'gzip', 'harfbuzz', 'harfbuzz-icu', 'hicolor-icon-theme',

+         'hunspell', 'hunspell-en-US', 'hwdata', 'hyphen', 'ibus-gtk2',

+         'ibus-gtk3', 'ibus-libs', 'libicu', 'iptables-libs', 'iso-codes',

+         'jasper-libs', 'jbigkit-libs', 'json-c', 'json-glib', 'qt-settings',

+         'keyutils-libs', 'kmod-libs', 'krb5-libs', 'krb5-server',

+         'krb5-workstation', 'libkadm5', 'lcms2', 'lcms2-utils', 'less',

+         'libICE', 'libSM', 'libX11', 'libX11-common', 'libX11-xcb',

+         'libXScrnSaver', 'libXau', 'libXcomposite', 'libXcursor',

+         'libXdamage', 'libXdmcp', 'libXext', 'libXfixes', 'libXft', 'libXi',

+         'libXinerama', 'libXpm', 'libXrandr', 'libXrender', 'libXt',

+         'libXtst', 'libXv', 'libXxf86vm', 'libappstream-glib', 'libarchive',

+         'libassuan', 'libasyncns', 'libatomic_ops', 'libcanberra',

+         'libcanberra-gtk2', 'libcanberra-gtk3', 'libcap', 'libcap-ng',

+         'libcroco', 'libdatrie', 'libdb', 'libdrm', 'libdvdnav', 'libdvdread',

+         'libedit', 'libepoxy', 'liberation-fonts-common',

+         'liberation-mono-fonts', 'liberation-sans-fonts',

+         'liberation-serif-fonts', 'libev', 'libevdev', 'libexif', 'libffi',

+         'libgcrypt', 'libglvnd', 'libglvnd-egl', 'libglvnd-gles',

+         'libglvnd-glx', 'libglvnd-opengl', 'libgpg-error', 'libgudev',

+         'libgusb', 'libidn2', 'libinput', 'libjpeg-turbo', 'turbojpeg',

+         'libksba', 'libmetalink', 'libmng', 'libmodman', 'libmpc',

+         'libmpcdec', 'libnice', 'libnotify', 'libnsl2', 'libogg', 'libpcap',

+         'libpciaccess', 'libpng', 'libproxy', 'python2-libproxy', 'libpsl',

+         'libpwquality', 'librsvg2', 'librsvg2-tools', 'libsamplerate',

+         'libseccomp', 'libsecret', 'libselinux', 'libsemanage', 'libsepol',

+         'libsigsegv', 'libsndfile', 'libsoup', 'libsrtp', 'libssh',

+         'libstemmer', 'libtasn1', 'libtdb', 'libthai', 'libtheora', 'libtiff',

+         'libtirpc', 'libtool-ltdl', 'libunistring', 'libusb', 'libusbx',

+         'libutempter', 'libvdpau', 'libverto', 'libverto-libev', 'libvisual',

+         'libvorbis', 'libwacom', 'libwacom-data', 'libwebp', 'libxcb',

+         'libxcrypt', 'libxkbcommon', 'libxkbcommon-x11', 'libxml2',

+         'python2-libxml2', 'libxshmfence', 'libxslt', 'llvm-libs',

+         'logrotate', 'device-mapper', 'device-mapper-libs', 'lz4-libs',

+         'ModemManager-glib', 'make', 'mesa-dri-drivers', 'mesa-filesystem',

+         'mesa-libEGL', 'mesa-libGL', 'mesa-libgbm', 'mesa-libglapi',

+         'mesa-libxatracker', 'mesa-vulkan-drivers', 'glx-utils',

+         'mesa-libGLU', 'mlocate', 'mozjs60', 'mpfr', 'mpg123-libs', 'mtdev',

+         'mythes', 'ncompress', 'ncurses', 'ncurses-base',

+         'ncurses-compat-libs', 'ncurses-libs', 'nettle', 'libnghttp2', 'npth',

+         'nspr', 'nss', 'nss-sysinit', 'nss-tools', 'nss-softokn',

+         'nss-softokn-freebl', 'nss-util', 'ocl-icd', 'openal-soft',

+         'openldap', 'openssl', 'openssl-libs', 'opus', 'orc', 'p11-kit',

+         'p11-kit-trust', 'pam', 'pango', 'pcre', 'pcre-cpp', 'pcre2',

+         'pcre2-utf16', 'pcre2-utf32', 'pinentry', 'pixman', 'libpkgconf',

+         'pkgconf', 'pkgconf-m4', 'pkgconf-pkg-config', 'popt', 'procps-ng',

+         'publicsuffix-list-dafsa', 'pulseaudio-libs', 'pulseaudio-libs-glib2',

+         'pulseaudio-utils', 'python3-cairo', 'python3-gobject',

+         'python3-gobject-base', 'python-pip-wheel', 'python-setuptools-wheel',

+         'python2-setuptools', 'python-unversioned-command', 'python2',

+         'python2-libs', 'python3', 'python3-libs', 'qrencode-libs',

+         'readline', 'rest', 'rpcgen', 'SDL2', 'SDL2_image', 'SDL2_mixer',

+         'SDL2_net', 'SDL2_ttf', 'sed', 'setup', 'xml-common', 'shadow-utils',

+         'shared-mime-info', 'sound-theme-freedesktop', 'soundtouch', 'speex',

+         'speexdsp', 'spirv-tools-libs', 'sqlite-libs', 'systemd',

+         'systemd-libs', 'systemd-pam', 'tar', 'info', 'tzdata', 'unzip',

+         'libblkid', 'libfdisk', 'libmount', 'libsmartcols', 'libuuid',

+         'util-linux', 'vte-profile', 'vte291', 'vulkan-loader',

+         'vulkan-validation-layers', 'libwayland-client', 'libwayland-cursor',

+         'libwayland-egl', 'libwayland-server', 'webkit2gtk3',

+         'webkit2gtk3-jsc', 'webrtc-audio-processing', 'which', 'woff2',

+         'words', 'xcb-util', 'xcb-util-cursor', 'xcb-util-image',

+         'xcb-util-keysyms', 'xcb-util-renderutil', 'xcb-util-wm',

+         'xdg-user-dirs', 'xdg-utils', 'xkeyboard-config', 'xz', 'xz-libs',

+         'xz-lzma-compat', 'yelp', 'yelp-libs', 'yelp-xsl', 'zenity', 'zip',

+         'zlib', 'libzstd'

+       ]

+     buildroot:

+       rpms:

+       - flatpak-rpm-macros

+       - flatpak-runtime-config

+ """

+ 

+ 

+ @contextmanager

+ def mock_koji():

+     session = Mock()

+     session.getPackageID = Mock(return_value=42)

+     session.listBuilds = Mock(return_value=[{

+         'name': 'flatpak-runtime',

+         'version': 'f29',

+         'release': '20180831153244.1',

+         'build_id': '1167601',

+         'extra': {

+             'typeinfo': {

+                 'module': {

+                     'modulemd_str': FLATPAK_RUNTIME_MODULEMD

+                 }

+             }

+         }

+     }])

+     session.listTags = Mock(return_value=[

+         {'name': 'f29-modular-updates-candidate'}

+     ])

+ 

+     p1 = patch('koji.read_config',

+                return_value={'server':

+                              'https://koji.fedoraproject.org/kojihub'})

+     p2 = patch('koji.grab_session_options')

+     p3 = patch('koji.ClientSession', return_value=session)

+ 

+     p1.start()

+     p2.start()

+     p3.start()

+ 

+     yield

+ 

+     p1.stop()

+     p2.stop()

+     p3.stop()

  

- def _generate_flatpak(rpm):

+ 

+ def _generate_flatpak(rpm, flathub=None, expected_error_output=None):

      cmd = ['rpm2flatpak']

      cmd.append(rpm)

+     if flathub:

+         cmd += ['--flathub', flathub]

  

      prevdir = os.getcwd()

      with tempfile.TemporaryDirectory() as workdir:

          try:

              os.chdir(workdir)

              runner = CliRunner()

-             result = runner.invoke(_cli_commands, cmd)

-             assert result.exit_code == 0

+             result = runner.invoke(_cli_commands, cmd, catch_exceptions=False)

+             if expected_error_output is not None:

+                 assert result.exit_code != 0

+                 assert expected_error_output in result.output

+                 return

+             else:

+                 assert result.exit_code == 0

  

              modulemd_fname = rpm + '.yaml'

              with open(rpm + '.yaml') as f:

@@ -54,12 +272,12 @@ 

  

  

  class TestFlatpak(object):

- 

      @pytest.mark.filterwarnings('ignore::DeprecationWarning:koji')

      @pytest.mark.filterwarnings('ignore::PendingDeprecationWarning:koji')

      @pytest.mark.needs_metadata

      def test_generated_flatpak_files(self):

-         modmd, container_yaml = _generate_flatpak('eog')

+         with mock_koji():

+             modmd, container_yaml = _generate_flatpak('eog')

  

          # Expected description for 'grep'

          assert modmd.props.summary == "Eye of GNOME image viewer"

@@ -89,3 +307,57 @@ 

          requires = dependencies[0].props.requires

          assert set(requires) == {'flatpak-runtime'}

          assert requires['flatpak-runtime'].get() == ['f29']

+ 

+     @responses.activate

+     @pytest.mark.needs_metadata

+     @pytest.mark.parametrize(('search_term', 'extension', 'expected_error'),

+                              [

+                                  ('org.gnome.eog', 'yaml', None),

+                                  ('org.gnome.eog', 'yml', None),

+                                  ('org.gnome.eog', 'json', None),

+                                  ('eYe of gNome', 'yaml', None),

+                                  ('org.gnome', 'yaml',

+                                   'Multiple matches found on flathub.org'),

+                                  ('notexist', 'yaml',

+                                   'No match found on flathub.org'),

+                              ])

+     def test_flatpak_from_flathub(self, search_term, extension,

+                                   expected_error):

+         responses.add(responses.GET, 'https://flathub.org/api/v1/apps',

+                       body=APPS_JSON, content_type='application/json')

+ 

+         app_id = 'org.gnome.eog'

+         base = 'https://raw.githubusercontent.com/flathub'

+ 

+         for ext, content_type, body in [

+                 ('yml', 'application/x-yaml', EOG_YAML),

+                 ('yaml', 'application/x-yaml', EOG_YAML),

+                 ('json', 'application/json', EOG_JSON)]:

+             if extension == ext:

+                 responses.add(responses.GET,

+                               f"{base}/{app_id}/master/{app_id}.{ext}",

+                               body=body, content_type=content_type)

+             else:

+                 responses.add(responses.GET,

+                               f"{base}/{app_id}/master/{app_id}.{ext}",

+                               body='Not found', status=404)

+ 

+         with mock_koji():

+             if expected_error is None:

+                 modmd, container_yaml = \

+                     _generate_flatpak('eog',

+                                       flathub=search_term,

+                                       expected_error_output=expected_error)

+ 

+                 f = container_yaml['flatpak']

+ 

+                 assert f['id'] == 'org.gnome.eog'

+                 assert f['command'] == 'eog'

+                 assert f['rename-desktop-file'] == 'eog.desktop'

+                 assert f['rename-appdata-file'] == 'eog.appdata.xml'

+                 assert f['rename-icon'] == 'eog'

+                 assert f['copy-icon'] is True

+                 assert f['finish-args'] == '--share=ipc\n--socket=x11'

+             else:

+                 _generate_flatpak('eog', flathub=search_term,

+                                   expected_error_output=expected_error)

If an application is already packaged on Flathub, allow the user to
initialize the container.yaml from the Flathub manifest by passing

 --flathub=com.example.MyApp

or:

 --flathub=MyApp

to do a search (if the search term isn't unique, the operation will
error out showing a list of matches.)

The tests from flatpak generation are changed to mock out not just the new HTTP requests to flathub/github, but also the use of Koji, since the slowness of actually talking to koji became obvious when running a larger set of tests.

Shouldn't NoSortMapping be a subclass of collections.OrderedDict? AIUI that a dict is iterated in insertion order isn't guaranteed until Python 3.7, right?

flake8 flags these here (with the import-order plugin). I can fix them before applying (I'll rebase onto master anyway, so it's not a biggie).

Same thing about imports being flagged as above, I'll fix it when applying.

I'll probably move these to files beneath tests/files/.

Is there a reason for changing this one to print() but leaving the other log.info() a few lines above in place?

Metadata Update from @nphilipp:
- Request assigned

4 months ago

@otaylor, I'm waiting on an answer to my dict vs. collections.OrderedDict and print() vs. log.info() questions above. Other than these and the changes I mentioned I'd anyway, the PR looks good to me.

Shouldn't NoSortMapping be a subclass of collections.OrderedDict? AIUI that a dict is iterated in insertion order isn't guaranteed until Python 3.7, right?

it's guaranteed for CPython for 3.6, then made an official language feature for 3.7. Unless we want to support older Python 3 than any current Fedora or non-CPython in the near future, dict should be fine. What do you think?

Is there a reason for changing this one to print() but leaving the other log.info() a few lines above in place?

At this point, I can't reconstruct why I changed it to print - I think it was because it wasn't showing up in the pytest output. I'll change it back and push a new version with that and the imports reordered.

<hmm> what about changing the style - import ordering, with the defaults, test_flatpak_generator.py ends up as:

import logging
import os
import sys
import tempfile
from unittest.mock import Mock, patch
from contextlib import contextmanager

from click.testing import CliRunner

import gi
gi.require_version('Modulemd', '1.0')  # noqa: E402                                                                                                           
from gi.repository import Modulemd

import pytest

import responses

import yaml

from _fedmod.cli import _cli_commands  # noqa: I100     

Which is sort of ridiculous - every 3rd party import is forced into it's own group. While with import_order_style = google, application_import_names = _fedmod in setup.cfg:

from contextlib import contextmanager
import logging
import os
import sys
import tempfile
from unittest.mock import Mock, patch

from click.testing import CliRunner
import gi
gi.require_version('Modulemd', '1.0')  # noqa: E402                                                                                                           
from gi.repository import Modulemd
import pytest
import responses
import yaml

from _fedmod.cli import _cli_commands  # noqa: I100   

Which is much more like I'd arrange the imports.

2 new commits added

  • rpm2flatpak: Add an option to initialize from the Flathub manifest
  • flatpak_generator.py: Use a literal string for finish-args
4 months ago

Shouldn't NoSortMapping be a subclass of collections.OrderedDict? AIUI that a dict is iterated in insertion order isn't guaranteed until Python 3.7, right?

it's guaranteed for CPython for 3.6, then made an official language feature for 3.7. Unless we want to support older Python 3 than any current Fedora or non-CPython in the near future, dict should be fine. What do you think?

You're right.

<hmm> what about changing the style [...]
Which is sort of ridiculous - every 3rd party import is forced into it's own group. While with import_order_style = google, application_import_names = _fedmod in setup.cfg: [...]
Which is much more like I'd arrange the imports.

Good call. I've changed the configuration and fixed the places which get flagged now, but there are some conflicts now with the PR. Do you want to fix them or should I?

I've fixed the conflicts meanwhile and (as per my comment) also moved the test data to their own files.

Applied in commits abed17b..5f2f6e8

Pull-Request has been closed by nphilipp

4 months ago