#284 [RFC] comps-sync.py: improve support for multiple groups and architecture-specific packages
Closed a year ago by alebastr. Opened 2 years ago by alebastr.
alebastr/workstation-ostree-config comps-sync-arch  into  main

@@ -0,0 +1,169 @@ 

+ ---

+ # Desktop environment specific configurations

+ #

+ # Keys:

+ #   groups            list of groups to be synced

+ #   exclude-packages  filter packages collected from the groups before

+ #                     writing {name}-desktop-pkgs.yaml

+ 

+ gnome:

+   groups:

+     - gnome-desktop

+   exclude-packages:

+     # Non-critical apps -> Flatpak

+     - baobab

+     - cheese

+     - eog

+     - evince

+     - evince-djvu

+     - evince-nautilus

+     - file-roller

+     - file-roller-nautilus

+     - gnome-boxes

+     - gnome-calculator

+     - gnome-calendar

+     - gnome-characters

+     - gnome-clocks

+     - gnome-connections

+     - gnome-contacts

+     - gnome-documents

+     - gnome-font-viewer

+     - gnome-logs

+     - gnome-maps

+     - gnome-photos

+     - gnome-screenshot

+     - gnome-text-editor

+     - gnome-weather

+     - jwhois

+     - rdist

+     - sane-backends-drivers-scanners

+     - symlinks

+     - simple-scan

+     - sushi

+     - tcpdump

+     - telnet

+     - totem

+     - totem-nautilus

+     - traceroute

+ 

+ kde:

+   groups:

+     - kde-desktop

+   exclude-packages:

+     # Incompatible with ostree for various reasons

+     - abrt-desktop

+     - cups-pk-helper

+     - dnfdragora

+     - plasma-pk-updates

+     # Non-critical apps -> Flatpak

+     - akregator

+     - cagibi

+     - elisa-player

+     - gwenview

+     - kaddressbook

+     - kamera

+     - kcalc

+     - kcharselect

+     - kcolorchooser

+     - kdnssd

+     - kf5-kipi-plugins

+     - kfind

+     - kget

+     - kgpg

+     - kmail

+     - kmouth

+     - knode

+     - konqueror

+     - kontact

+     - korganizer

+     - kruler

+     - ksshaskpass

+     - kwrite

+     - libreoffice-kde

+     - okular

+     # Misc

+     - adwaita-gtk2-theme

+     - colord-kde

+     - keditbookmarks

+     - kwebkitpart

+     - plasma-nm-l2tp

+     - plasma-nm-openswan

+     - plasma-nm-pptp

+ 

+ xfce:

+   groups:

+     - xfce-desktop

+   exclude-packages:

+     # Incompatible with ostree for various reasons

+     - abrt-desktop

+     - dnfdragora-updater

+     # Non-critical apps -> Flatpak

+     - fros-recordmydesktop

+     - tumbler

+     # Misc

+     - alsa-utils

+     - firewall-config

+     - openssh-askpass

+     - vim-enhanced

+     # Remove uncommon NetworkManager plugins

+     - NetworkManager-fortisslvpn-gnome

+     - NetworkManager-iodine-gnome

+     - NetworkManager-l2tp-gnome

+     - NetworkManager-libreswan-gnome

+     - NetworkManager-sstp-gnome

+     - NetworkManager-strongswan-gnome

+ 

+ lxqt:

+   groups:

+     - lxqt-desktop

+   exclude-packages:

+     # Incompatible with ostree for various reasons

+     - dnfdragora-updater

+ 

+ deepin:

+   groups:

+     - deepin-desktop

+   exclude-packages:

+     # Incompatible with ostree for various reasons

+     - dnfdragora-updater

+ 

+ pantheon:

+   groups:

+     - pantheon-desktop

+ 

+ mate:

+   groups:

+     - mate-desktop

+   exclude-packages:

+     # Incompatible with ostree for various reasons

+     - abrt-desktop

+     - abrt-java-connector

+     - dnfdragora-updater

+     # Non-critical apps -> Flatpak

+     - blivet-gui

+     - filezilla

+     - gnome-disk-utility

+     - gnome-logs

+     - gnote

+     - gparted

+     - hexchat

+     - p7zip

+     - p7zip-plugins

+     - simple-scan

+     - thunderbird

+     - transmission-gtk

+     - vim-enhanced

+     - xfburn

+     - yelp

+     # Non critical NetworkManager plugins

+     - NetworkManager-bluetooth

+     - NetworkManager-iodine-gnome

+     - NetworkManager-l2tp-gnome

+     - NetworkManager-libreswan-gnome

+     - NetworkManager-ovs

+     - NetworkManager-sstp-gnome

+     - NetworkManager-strongswan-gnome

+     - NetworkManager-team

+     - NetworkManager-wifi

+     # Already in the common set

+     - wireplumber

file modified
+21 -153
@@ -21,6 +21,26 @@ 

    - "mozilla-openh264"

    - "openh264"

  

+ # Architecture-specific package list.

+ # The packages listed here will be added to `packages-{arch}` instead of the

+ # default `packages` list.

+ # Architecture information is not exposed in libcomps python bindings and is

+ # not even always specified. We have to maintain our own list instead.

+ arch_specific_list:

+   armhfp:

+     - xorg-x11-drv-armada

+   x86_64:

+     - hyperv-daemons

+     - microcode_ctl

+     - mcelog

+     - open-vm-tools-desktop

+     - thermald

+     - virtualbox-guest-additions

+     - xorg-x11-drv-intel

+     - xorg-x11-drv-openchrome

+     - xorg-x11-drv-vesa

+     - xorg-x11-drv-vmware

+ 

  # Common exclude list for all ostree desktop versions

  exclude_list:

    core:
@@ -43,20 +63,9 @@ 

      - lsvpd

      - s390utils-base

    base-x:

-     # x86 specific packages. Added back by fedora-common-ostree.yaml

-     - xorg-x11-drv-intel

-     - xorg-x11-drv-openchrome

-     - xorg-x11-drv-vesa

-     - xorg-x11-drv-vmware

-     # ARM specific packages

-     - xorg-x11-drv-armada

+     # Unused ARM specific packages

      - xorg-x11-drv-armsoc

      - xorg-x11-drv-omap

-   guest-desktop-agents:

-     # x86 specific packages. Added back by fedora-common-ostree.yaml

-     - hyperv-daemons

-     - open-vm-tools-desktop

-     - virtualbox-guest-additions

    workstation-product:

      # We use rpm-ostree for the host

      - dnf
@@ -116,10 +125,6 @@ 

      - gnome-shell-extension-background-logo

      - pinentry-gnome3

      - qgnomeplatform

-     # x86 specific packages. Added back by fedora-common-ostree.yaml

-     - mcelog

-     - microcode_ctl

-     - thermald

    networkmanager-submodules:

      # Let's use the builtin one by default

      - dhcp-client
@@ -128,140 +133,3 @@ 

      - cups-pk-helper

      # For now...

      - ghostscript

- 

- # Desktop environment specific exclude lists

- desktop_exclude_list:

-   gnome-desktop:

-     # Non-critical apps -> Flatpak

-     - baobab

-     - cheese

-     - eog

-     - evince

-     - evince-djvu

-     - evince-nautilus

-     - file-roller

-     - file-roller-nautilus

-     - gnome-boxes

-     - gnome-calculator

-     - gnome-calendar

-     - gnome-characters

-     - gnome-clocks

-     - gnome-connections

-     - gnome-contacts

-     - gnome-documents

-     - gnome-font-viewer

-     - gnome-logs

-     - gnome-maps

-     - gnome-photos

-     - gnome-screenshot

-     - gnome-text-editor

-     - gnome-weather

-     - jwhois

-     - rdist

-     - sane-backends-drivers-scanners

-     - symlinks

-     - simple-scan

-     - sushi

-     - tcpdump

-     - telnet

-     - totem

-     - totem-nautilus

-     - traceroute

-   kde-desktop:

-     # Incompatible with ostree for various reasons

-     - abrt-desktop

-     - cups-pk-helper

-     - dnfdragora

-     - plasma-pk-updates

-     # Non-critical apps -> Flatpak

-     - akregator

-     - cagibi

-     - elisa-player

-     - gwenview

-     - kaddressbook

-     - kamera

-     - kcalc

-     - kcharselect

-     - kcolorchooser

-     - kdnssd

-     - kf5-kipi-plugins

-     - kfind

-     - kget

-     - kgpg

-     - kmail

-     - kmouth

-     - knode

-     - konqueror

-     - kontact

-     - korganizer

-     - kruler

-     - ksshaskpass

-     - kwrite

-     - libreoffice-kde

-     - okular

-     # Misc

-     - adwaita-gtk2-theme

-     - colord-kde

-     - keditbookmarks

-     - kwebkitpart

-     - plasma-nm-l2tp

-     - plasma-nm-openswan

-     - plasma-nm-pptp

-   xfce-desktop:

-     # Incompatible with ostree for various reasons

-     - abrt-desktop

-     - dnfdragora-updater

-     # Non-critical apps -> Flatpak

-     - fros-recordmydesktop

-     - tumbler

-     # Misc

-     - alsa-utils

-     - firewall-config

-     - openssh-askpass

-     - vim-enhanced

-     # Remove uncommon NetworkManager plugins

-     - NetworkManager-fortisslvpn-gnome

-     - NetworkManager-iodine-gnome

-     - NetworkManager-l2tp-gnome

-     - NetworkManager-libreswan-gnome

-     - NetworkManager-sstp-gnome

-     - NetworkManager-strongswan-gnome

-   lxqt-desktop:

-     # Incompatible with ostree for various reasons

-     - dnfdragora-updater

-   deepin-desktop:

-     # Incompatible with ostree for various reasons

-     - dnfdragora-updater

-   mate-desktop:

-     # Incompatible with ostree for various reasons

-     - abrt-desktop

-     - abrt-java-connector

-     - dnfdragora-updater

-     # Non-critical apps -> Flatpak

-     - blivet-gui

-     - filezilla

-     - gnome-disk-utility

-     - gnome-logs

-     - gnote

-     - gparted

-     - hexchat

-     - p7zip

-     - p7zip-plugins

-     - simple-scan

-     - thunderbird

-     - transmission-gtk

-     - vim-enhanced

-     - xfburn

-     - yelp

-     # Non critical NetworkManager plugins

-     - NetworkManager-bluetooth

-     - NetworkManager-iodine-gnome

-     - NetworkManager-l2tp-gnome

-     - NetworkManager-libreswan-gnome

-     - NetworkManager-ovs

-     - NetworkManager-sstp-gnome

-     - NetworkManager-strongswan-gnome

-     - NetworkManager-team

-     - NetworkManager-wifi

-     # Already in the common set

-     - wireplumber

file modified
+191 -69
@@ -1,33 +1,164 @@ 

  #!/usr/bin/python3

  # Usage: ./comps-sync.py /path/to/comps-f37.xml.in

  #

- # Can both remove packages from the manifest

- # which are not mentioned in comps, and add packages from

- # comps.

+ # Can both remove packages from the manifest which are not mentioned in comps,

+ # and add packages from comps.

  

- import os, sys, subprocess, argparse, shlex, json, yaml, re

+ import argparse

+ import re

+ import sys

+ import yaml

  import libcomps

  

+ from dataclasses import dataclass, field

+ 

+ 

+ @dataclass

+ class DesktopEnvironment:

+     """

+     Represents a desktop environment configuration which could include

+     contents of multiple groups and filter out specified packages.

+     """

+ 

+     groups: set[str] = field(default_factory=set)

+     exclude_packages: set[str] = field(default_factory=set)

+ 

+     @classmethod

+     def from_dict(cls, obj: dict):

+         """Create DesktopEnvironment object from deserialized yaml dict"""

+         groups = set(obj.get("groups", []))

+         exclude_packages = set(obj.get("exclude-packages", []))

+         return cls(groups=groups, exclude_packages=exclude_packages)

+ 

+ 

+ @dataclass(order=True)

+ class Package():

+     name: str

+     type: int = libcomps.PACKAGE_TYPE_DEFAULT

+     # restricts the package to the listed architectures if specified

+     arch: set[str] = field(default_factory=set)

+     # groups that require the package

+     groups: set[str] = field(default_factory=set)

+ 

+     def __str__(self):

+         arches = ''

+         if len(self.arch) > 0:

+             arches = ', arch: ' + ', '.join(self.arch)

+         groups = ''

+         if len(self.groups) > 0:

+             groups = ', groups: ' + ', '.join(self.groups)

+         return f"{self.name} ({format_pkgtype(self.type)}{groups}{arches})"

+ 

+ 

+ class Dumper(yaml.SafeDumper):

+     """Workaround for yaml/pyyaml#234 - preserve current indentation style"""

+ 

+     def increase_indent(self, flow=False, *args, **kwargs):

+         return super().increase_indent(flow=flow, indentless=False)

+ 

+ 

+ class Manifest():

+ 

+     def __init__(self):

+         self.includes = []

+         self.packages = {}

+ 

+     def add(self, pkgname, type, group=None):

+         # try to update existing list entry

+         if pkg := self.packages.get(pkgname, None):

+             if group is not None:

+                 pkg.groups.add(group)

+             if (pkg.type == libcomps.PACKAGE_TYPE_DEFAULT

+                     and type == libcomps.PACKAGE_TYPE_MANDATORY):

+                 pkg.type = type

+             return

+ 

+         groups = set()

+         if group is not None:

+             groups.add(group)

+         self.packages[pkgname] = Package(pkgname,

+                                          type=type,

+                                          arch=get_pkg_arches(pkgname),

+                                          groups=groups)

+ 

+     def compare(self, other):

+         """Compare two manifests and return lists of added/removed entries"""

+         added = []

+         removed = []

+         all_pkgnames = set(self.packages.keys()) | set(other.packages.keys())

+         for name in all_pkgnames:

+             pkg1 = self.packages.get(name, None)

+             pkg2 = other.packages.get(name, None)

+             if pkg1 is None:

+                 removed.append(pkg2)

+             elif pkg2 is None:

+                 added.append(pkg1)

+             elif pkg1.arch != pkg2.arch:

+                 # package architectures has changed,

+                 # treat this as a package list change

+                 added.append(pkg1)

+                 removed.append(pkg2)

+         return added, removed

+ 

+     def load(self, stream):

+         """Load from yaml stream or string"""

+         temp = yaml.safe_load(stream) or {}

+         includes = temp.get('include', [])

+         if isinstance(includes, str):

+             includes = set([includes])

+ 

+         packages = {pkg: set() for pkg in temp.get('packages', [])}

+         for key, values in temp.items():

+             if m := re.match(r'^packages-(\w+)$', key):

+                 for pkg in values:

+                     packages.setdefault(pkg, set()).add(m[1])

+ 

+         self.includes = includes

+         self.packages = {

+             name: Package(name, arch=arch, groups={'manifest'})

+             for name, arch in packages.items()

+         }

+ 

+     def write(self, stream):

+         # assemble temporary dict matching the treefile structure

+         temp = {'packages': []}

+         for pkg in sorted(self.packages.values()):

+             if len(pkg.arch) > 0:

+                 for arch in pkg.arch:

+                     temp.setdefault(f'packages-{arch}', []).append(pkg.name)

+             else:

+                 temp['packages'].append(pkg.name)

+         yaml.dump(temp, stream, Dumper=Dumper)

+ 

+ 

  def fatal(msg):

-     print >>sys.stderr, msg

+     print(msg, file = sys.stderr)

      sys.exit(1)

  

  def format_pkgtype(n):

      if n == libcomps.PACKAGE_TYPE_DEFAULT:

          return 'default'

-     elif n == libcomps.PACKAGE_TYPE_MANDATORY:

+     if n == libcomps.PACKAGE_TYPE_MANDATORY:

          return 'mandatory'

-     else:

-         assert False

+     if n == libcomps.PACKAGE_TYPE_OPTIONAL:

+         return 'optional'

+     if n == libcomps.PACKAGE_TYPE_CONDITIONAL:

+         return 'conditional'

+     assert False

+ 

+ 

+ def get_pkg_arches(pkgname):

+     """lookup package architecture restrictions from arch_specific_list"""

+     return set(

+         (arch for arch, pkgs in comps_arch_specific_list.items() if pkgname in pkgs))

+ 

  

  def write_manifest(fpath, pkgs, include=None):

-     with open(fpath, 'w') as f:

+     with open(fpath, 'w', encoding='UTF-8') as f:

          f.write("# DO NOT EDIT! This content is generated from comps-sync.py\n")

          if include is not None:

              f.write("include: {}\n".format(include))

-         f.write("packages:\n")

-         for pkg in sorted(pkgs):

-             f.write("  - {}\n".format(pkg))

+         pkgs.write(f)

          print("Wrote {}".format(fpath))

  

  parser = argparse.ArgumentParser()
@@ -39,16 +170,16 @@ 

  print("Syncing packages common to all desktops:")

  

  base_pkgs_path = 'fedora-common-ostree-pkgs.yaml'

- with open(base_pkgs_path) as f:

-     manifest = yaml.safe_load(f)

- manifest_packages = set(manifest['packages'])

+ manifest = Manifest()

+ with open(base_pkgs_path, encoding='UTF-8') as f:

+     manifest.load(f)

  

- with open('comps-sync-exclude-list.yml') as f:

+ with open('comps-sync-exclude-list.yml', encoding='UTF-8') as f:

      doc = yaml.safe_load(f)

      comps_exclude_list = doc['exclude_list']

      comps_include_list = doc['include_list']

+     comps_arch_specific_list = doc.get('arch_specific_list', {})

      comps_exclude_list_groups = doc['exclude_list_groups']

-     comps_desktop_exclude_list = doc['desktop_exclude_list']

      comps_exclude_list_all = [re.compile(x) for x in doc['exclude_list_all_regexp']]

  

  def is_exclude_listed(pkgname):
@@ -68,7 +199,7 @@ 

  ws_env_name = 'workstation-product-environment'

  ws_ostree_name = 'workstation-ostree-support'

  ws_environ = comps.environments[ws_env_name]

- ws_pkgs = {}

+ new_manifest = Manifest()

  for gid in ws_environ.group_ids:

      group = comps.groups_match(id=gid.name)[0]

      if gid.name in comps_exclude_list_groups:
@@ -81,26 +212,20 @@ 

              continue

          if pkgname in exclude_list or is_exclude_listed(pkgname):

              continue

-         pkgdata = ws_pkgs.get(pkgname)

-         if pkgdata is None:

-             ws_pkgs[pkgname] = pkgdata = (pkg.type, set([gid.name]))

-         if (pkgdata[0] == libcomps.PACKAGE_TYPE_DEFAULT and

-             pkg.type == libcomps.PACKAGE_TYPE_MANDATORY):

-             ws_pkgs[pkgname] = pkgdata = (pkg.type, pkgdata[1])

-         pkgdata[1].add(gid.name)

+         new_manifest.add(pkgname, type=pkg.type, group=gid.name)

  

  ws_ostree_pkgs = set()

  for pkg in comps.groups_match(id=ws_ostree_name)[0].packages:

      if not is_exclude_listed(pkg.name):

          ws_ostree_pkgs.add(pkg.name)

  

- comps_unknown = set()

- for pkg in manifest_packages:

-     if (pkg not in comps_include_list and

-         pkg not in ws_pkgs and

-         pkg not in ws_ostree_pkgs):

-         comps_unknown.add(pkg)

- 

+ ws_added, comps_unknown = new_manifest.compare(manifest)

+ # deal with the packages that should magically migrate from the old manifest

+ # while having no other reason to exist in the freshly generated one

+ comps_unknown = [

+     x for x in comps_unknown

+     if x.name not in comps_include_list and x.name not in ws_ostree_pkgs

+ ]

  # Look for packages in the manifest but not in comps at all

  n_manifest_new = len(comps_unknown)

  if n_manifest_new == 0:
@@ -109,53 +234,57 @@ 

      print("  - {} packages not in {}:".format(n_manifest_new, ws_env_name))

      for pkg in sorted(comps_unknown):

          print('    {}'.format(pkg))

-         manifest_packages.remove(pkg)

+         del manifest.packages[pkg.name]

  

  # Look for packages in workstation but not in the manifest

- ws_added = {}

- for (pkg,data) in ws_pkgs.items():

-     if pkg not in manifest_packages:

-         ws_added[pkg] = data

-         manifest_packages.add(pkg)

- 

  n_comps_new = len(ws_added)

  if n_comps_new == 0:

      print("  - All comps packages are already listed in manifest.")

  else:

      print("  - {} packages not in manifest:".format(n_comps_new))

      for pkg in sorted(ws_added):

-         (req, groups) = ws_added[pkg]

-         print('    {} ({}, groups: {})'.format(pkg, format_pkgtype(req), ', '.join(groups)))

+         print('    {}'.format(pkg))

+         manifest.packages[pkg.name] = pkg

  

  if (n_manifest_new > 0 or n_comps_new > 0) and args.save:

-     write_manifest(base_pkgs_path, manifest_packages)

+     write_manifest(base_pkgs_path, manifest)

+ 

+ # List of comps groups used for each desktop

+ with open('comps-sync-desktop-list.yml', encoding='UTF-8') as f:

+     doc = yaml.safe_load(f)

+     desktops_comps_groups = {

+         name: DesktopEnvironment.from_dict(obj)

+         for name, obj in doc.items() if obj is not None

+     }

  

  # Generate treefiles for all desktops

- for desktop in [ 'gnome-desktop', 'kde-desktop', 'xfce-desktop',

-         'lxqt-desktop', 'deepin-desktop', 'pantheon-desktop', 'mate-desktop']:

+ for desktop, conf in desktops_comps_groups.items():

      print()

      print("Syncing packages for {}:".format(desktop))

  

-     manifest_path = '{}-pkgs.yaml'.format(desktop)

-     with open(manifest_path) as f:

-         manifest = yaml.safe_load(f)

-     manifest_packages = set(manifest['packages'])

+     manifest_path = '{}-desktop-pkgs.yaml'.format(desktop)

+     manifest = Manifest()

+     with open(manifest_path, encoding='UTF-8') as f:

+         manifest.load(f)

  

-     # Filter packages in the comps desktop group using the exclude_list

-     comps_group_pkgs = set()

-     for pkg in comps.groups_match(id=desktop)[0].packages:

-         pkgname = pkg.name

-         exclude_list = comps_desktop_exclude_list.get(desktop, set())

-         if pkgname in exclude_list or is_exclude_listed(pkgname):

-             continue

-         comps_group_pkgs.add(pkg.name)

+     desktop_exclude_list = conf.exclude_packages

  

-     # Look for packages in the manifest but not in comps group

-     comps_unknown = set()

-     for pkg in manifest_packages:

-         if pkg not in comps_group_pkgs:

-             comps_unknown.add(pkg)

+     # Filter packages in the comps groups associated with a given desktop using

+     # the per group exclude_list comps_group_pkgs = set()

+     new_manifest = Manifest()

+     for group in conf.groups:

+         group_exclude_list = comps_exclude_list.get(group, set())

+         if group_exclude_list is None:

+             group_exclude_list = set()

+         for pkg in comps.groups_match(id=group)[0].packages:

+             pkgname = pkg.name

+             if (pkgname in desktop_exclude_list or pkgname in group_exclude_list

+                     or is_exclude_listed(pkgname)):

+                 continue

+             new_manifest.add(pkg.name, type=pkg.type, group=group)

  

+     # Look for packages in the manifest but not in comps group

+     desktop_pkgs_added, comps_unknown = new_manifest.compare(manifest)

      n_manifest_new = len(comps_unknown)

      if n_manifest_new == 0:

          print("  - All manifest packages are already listed in comps.")
@@ -163,14 +292,8 @@ 

          print("  - {} packages not in {} comps group:".format(n_manifest_new, desktop))

          for pkg in sorted(comps_unknown):

              print('    {}'.format(pkg))

-             manifest_packages.remove(pkg)

  

      # Look for packages in comps but not in the manifest

-     desktop_pkgs_added = set()

-     for pkg in comps_group_pkgs:

-         if pkg not in manifest_packages:

-             desktop_pkgs_added.add(pkg)

- 

      n_comps_new = len(desktop_pkgs_added)

      if n_comps_new == 0:

          print("  - All comps packages are already listed in manifest.")
@@ -178,8 +301,7 @@ 

          print("  - {} packages not in {} manifest:".format(n_comps_new, desktop))

          for pkg in sorted(desktop_pkgs_added):

              print('    {}'.format(pkg))

-             manifest_packages.add(pkg)

  

      # Update manifest

      if (n_manifest_new > 0 or n_comps_new > 0) and args.save:

-         write_manifest(manifest_path, manifest_packages, include="fedora-common-ostree.yaml")

+         write_manifest(manifest_path, new_manifest, include="fedora-common-ostree.yaml")

@@ -234,3 +234,16 @@ 

    - zd1211-firmware

    - zip

    - zram-generator-defaults

+ packages-armhfp:

+   - xorg-x11-drv-armada

+ packages-x86_64:

+   - hyperv-daemons

+   - mcelog

+   - microcode_ctl

+   - open-vm-tools-desktop

+   - thermald

+   - virtualbox-guest-additions

+   - xorg-x11-drv-intel

+   - xorg-x11-drv-openchrome

+   - xorg-x11-drv-vesa

+   - xorg-x11-drv-vmware

file modified
-11
@@ -57,7 +57,6 @@ 

    - shim

  packages-armhfp:

    - extlinux-bootloader

-   - xorg-x11-drv-armada

  packages-ppc64:

    - grub2

    - ostree-grub2
@@ -72,16 +71,6 @@ 

    - efibootmgr

    - shim-ia32

    - shim-x64

-   - microcode_ctl

-   - mcelog

-   - thermald

-   - hyperv-daemons

-   - open-vm-tools-desktop

-   - virtualbox-guest-additions

-   - xorg-x11-drv-intel

-   - xorg-x11-drv-openchrome

-   - xorg-x11-drv-vesa

-   - xorg-x11-drv-vmware

  

  # Make sure the following are not pulled in when Recommended by other packages

  exclude-packages:

Based on #280, extended for a better support of architecture-specific packages and refactored to satisfy my OCD.

The PR is motivated by our ongoing work on a Sway ostree spin for f38. It is going to be wayland-only for obvious reasons, and I wanted a clean way to get rid of base-x in fedora-common-ostree and move it to a X desktop package lists. For reference, here is the final form of the changes.

Unfortunately, architecture metadata (arch attribute of packagereq tag) is not exposed via libcomps and even if it was, it's pretty much useless — see how many packages in base-x have architecture specified and how many really should. I can pursue the necessary changes in fedora-comps and libcomps independently, but that's not going to happen immediately :disappointed:. Maybe once that's done we could remove arch_specific_list.

One of the things that still confuse me is the existence of include_list in comps-sync-exclude-list.yml. I'm pretty sure it's safe to move the packages to fedora-common-ostree.yaml and simplify the code just a bit more.

Metadata Update from @siosm:
- Request assigned

2 years ago

Thanks for working on this! I'll take a look.

This PR changes the package list in fedora-common-ostree.yaml for arch specific packages.
Can you make sure that running a sync after your change does not change the current list?
If the packages should be removed then we need to make that explicit or make another PR.

It's easier to review changes with the sync result in the same commit as the change.
Can you push your changes with a sync here?
Another sync that would make this review easier is a split with a first PR that adds the code but results in no package list change and then the changes that moves packages around to per-variant lists.

It's easier to review changes with the sync result in the same commit as the change.
Can you push your changes with a sync here?
Another sync that would make this review easier is a split with a first PR that adds the code but results in no package list change and then the changes that moves packages around to per-variant lists.

Makes sense, I'll try to get that done this weekend.
I'll leave base-x changes in our gitlab branch though, as we're not ready to send a PR for Sway yet.

rebased onto a59518e

2 years ago

Couple of random thoughts as I'm struggling to dedicate review time for this one as we both don't have CI and a way to validate that the changes are a NOP for now:

If you could get the changes required in fedora-comps (I can merge them there) and libcomps (I can not but can review) then that would be great for the future

How about we make this script generate yaml manifests that include themselves like the comps groups instead of generating very verbose and duplicated lists? This might make things easier to manage and follow. With this layout, we would be reproducing the comps groups as a tree of manifests.

libcomps PR: https://github.com/rpm-software-management/libcomps/pull/92

For the fedora-comps update, what are the architectures we care about? I want to keep it minimal, so if the package is, for example, ExcludeArch: s390x, we may not even need to bother.

How about we make this script generate yaml manifests that include themselves like the comps groups instead of generating very verbose and duplicated lists? This might make things easier to manage and follow. With this layout, we would be reproducing the comps groups as a tree of manifests.

I think that is fine for core groups, but what if the KDE or Sway or anything else wants a different set of packages from, say, @input-methods? Current scheme allows to exclude packages for the specific desktop from any group that is not in the fedora-ostree-common part, do we want to preserve that?

By the way, would getting #280 in first help the review?
I only had one nit on that PR, where exclude_list = comps_desktop_exclude_list.get(group, set()) should probably be exclude_list = comps_desktop_exclude_list.get(f"{desktop}-desktop", set()) to ensure that desktop_exclude_list still works as expected.

libcomps PR: https://github.com/rpm-software-management/libcomps/pull/92

Thanks for the libcomps PR.

what are the architectures we care about?

We care about x86, aarch64 & ppc64le for Silverblue & Kinoite.

I think that is fine for core groups, but what if the KDE or Sway or anything else wants a different set of packages from, say, @input-methods? Current scheme allows to exclude packages for the specific desktop from any group that is not in the fedora-ostree-common part, do we want to preserve that?

Good question. I'm not sure this happens that often.

I'm not sure #280 is the right approach.

I didn't see this before I wrote mine, but my #312 handles the arch problem here in, I think, a better way.

BTW, as of https://pagure.io/fedora-comps/pull-request/767 , fedora-comps should be 100% 'correct' regarding non-existent packages and arches. I'm re-running the check script to see if any new problems appeared in the last two months. edit: looks like there's a handful of new retirements, I'll fix those.

Metadata Update from @siosm:
- Request assignee reset

a year ago

I'm not sure this is any use now, since my #312 was merged. I'm pretty sure that addresses the problems.

We still need the functionality from #280, but at this point I'll just wait for @siosm to complete it instead of trying to salvage this PR. Or work around it in a very ugly way :)

Pull-Request has been closed by alebastr

a year ago

that shouldn't be too hard, I don't think, I didn't worry about it as it didn't seem necessary for anything ATM. do you need it for some new desktop or something?