Skip to content

AppStreams promise type enhancements#136

Open
nickanderson wants to merge 1 commit intocfengine:masterfrom
nickanderson:appstreams-enhancements-v2
Open

AppStreams promise type enhancements#136
nickanderson wants to merge 1 commit intocfengine:masterfrom
nickanderson:appstreams-enhancements-v2

Conversation

@nickanderson
Copy link
Copy Markdown
Member

Generic DNF Options Support

Previously the appstreams promise type had no way to pass DNF configuration options. Users could not control behavior like weak dependency installation.

appstreams:
  "php"
    state => "installed",
    stream => "8.2",
    profile => "minimal";
    # No way to prevent weak dependencies from being installed
$ rpm -q httpd
httpd-2.4.57-11.el9_4.1.x86_64  # Weak dependency was installed
$ rpm -q php-cli
php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64

After adding support for options the module now accepts any DNF configuration option.

appstreams:
  "php"
    state => "installed",
    stream => "8.2",
    profile => "minimal",
    options => { "install_weak_deps=false" };  # NEW
$ rpm -q httpd
package httpd is not installed  # Weak dependency excluded!
$ rpm -q php-cli
php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64
appstreams:
  "nodejs"
    state => "installed",
    stream => "20",
    profile => "minimal",
    options => {
      "install_weak_deps=false",
      "best=true",
      "skip_broken=false"
    };

Changes now log to dnf history. sys.argv is faked so that dnf history will output useful information in the prominent "Command Line" field.

Command Line   : module install -y php:8.2/minimal --setopt=install_weak_deps=false
Comment        : CFEngine appstreams promise: php state=installed

It's now possibly to easily switch streams. Previously, if a module was already enabled at one stream and you requested a different stream, the promise would report KEPT without making changes or would fail.

$ dnf module list php
Name Stream  Profiles                Summary               
php  8.1 [e] common [d], devel, minimal [i] PHP scripting language
php  8.2     common [d], devel, minimal     PHP scripting language

$ rpm -q php-cli
php-cli-8.1.32-1.module+el9.7.0+40003+454ed3c4.x86_64
appstreams:
  "php"
    state => "installed",
    stream => "8.2",  # Want to upgrade to 8.2
    profile => "minimal";
$ rpm -q php-cli
php-cli-8.1.32-1.module+el9.7.0+40003+454ed3c4.x86_64  # Still on 8.1!

Promise would report KEPT because it detected the module was "installed", but didn't check if the stream matched. Now there is automatic detection and handling of stream changes (both upgrades and downgrades).

Same policy, same starting state:

appstreams:
  "php"
    state => "installed",
    stream => "8.2",
    profile => "minimal",
    options => { "install_weak_deps=false" };
info: Switching module php from stream 8.1 to 8.2
info: Module php:8.2/minimal switched successfully
$ rpm -q php-cli
php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64  # Upgraded!

$ dnf module list php
Name Stream  Profiles                Summary               
php  8.1     common [d], devel, minimal     PHP scripting language
php  8.2 [e] common [d], devel, minimal [i] PHP scripting language

DNF History shows:

Command Line   : module switch-to -y php:8.2/minimal --setopt=install_weak_deps=false
Comment        : CFEngine appstreams promise: php state=installed
Packages Altered:
    Upgrade  php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64
    Upgraded php-cli-8.1.32-1.module+el9.7.0+40003+454ed3c4.x86_64

And downgrades:

appstreams:
  "php"
    state => "installed",
    stream => "8.1",  # Downgrade from 8.2 to 8.1
    profile => "minimal";

Result:

$ dnf history info last
Command Line   : module switch-to -y php:8.1/minimal
Packages Altered:
    Downgrade  php-cli-8.1.32-1.module+el9.7.0+40003+454ed3c4.x86_64
    Downgraded php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64

Fixed Package Versions installed by stream.

Previously packages might be installed from the wrong stream or default stream instead of the requested module stream.

Policy:

appstreams:
  "php"
    state => "installed",
    stream => "8.2",  # Request 8.2 specifically
    profile => "minimal";

Result (with old mpc.enable/install API):

$ rpm -q php-cli
php-cli-8.0.30-5.el9_7.x86_64  # Got 8.0 (default stream) instead of 8.2!

dnf history showed what we desired and what actually happened:

Command Line   : module install -y php:8.2/minimal
Packages Altered:
    Install php-cli-8.0.30-5.el9_7.x86_64    # Wrong version!

Now it uses the ModuleBase api and resolves versions from the module stream correctly:

Same policy:

appstreams:
  "php"
    state => "installed",
    stream => "8.2",
    profile => "minimal";

Result:

$ rpm -q php-cli
php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64  # Correct 8.2 version!

DNF history:

Command Line   : module install -y php:8.2/minimal
Packages Altered:
    Install php-cli-8.2.30-1.module+el9.7.0+40073+569087df.x86_64  # Correct!
    Install php-common-8.2.30-1.module+el9.7.0+40073+569087df.x86_64
Feature Before After
DNF Options Not supported Any option via options attribute
Stream Switching Manual intervention required Automatic detection and switching
Package Versions Sometimes wrong stream Always correct via ModuleBase API
DNF History Command Blank Full command with all options
DNF History Comment Not supported CFEngine context included

@nickanderson nickanderson force-pushed the appstreams-enhancements-v2 branch 4 times, most recently from f73f03d to 8d34c03 Compare April 21, 2026 22:09
@nickanderson nickanderson requested a review from larsewi April 21, 2026 22:10
@nickanderson nickanderson force-pushed the appstreams-enhancements-v2 branch from 8d34c03 to 03bec2e Compare April 21, 2026 22:13
This commit adds three major enhancements to the appstreams promise type:

Generic DNF options support via 'options' attribute
 - Accepts list of "key=value" strings (e.g., ["install_weak_deps=false"])
 - Uses DNF's set_or_append_opt_value() for generic option handling
 - Enables any DNF configuration option without hardcoding

Automatic stream switching detection and handling
 - Detects when installed stream differs from requested stream
 - Uses ModuleBase.switch_to() API for proper stream transitions
 - Supports both upgrades and downgrades (e.g., 8.2→8.1, 8.1→8.3)

ModuleBase API integration for proper module context
   - Replaces mpc.enable/install/save with ModuleBase.install()
   - Ensures module-specific package versions are installed
   - Maintains upstream's sack reset and explicit package download logic

Example usage:
  appstreams:
    "php"
      state => "installed",
      stream => "8.2",
      profile => "minimal",
      options => { "install_weak_deps=false" };
@nickanderson nickanderson force-pushed the appstreams-enhancements-v2 branch from 03bec2e to a6c0ca7 Compare April 21, 2026 22:44
Copy link
Copy Markdown
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 5 new features. They probably deserve 1 commit each. It's difficult to review a diff that is all over the place. Otherwise, LGTM 🚀 Added some suggestions. They are not super important, so feel free to dismiss them 😉

self.log_error(f" Package {pkg} failed: {error}")

def _install_module(self, mpc, base, module_name, stream, profile):
def _apply_dnf_options(self, base, options):
Copy link
Copy Markdown
Contributor

@larsewi larsewi Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function seems to apply DNF configuration options at best effort. I.e., if it fails, it just logs a warning and continues. Is this the intention? If so, maybe you should rename the function to _try_apply_dnf_options?

self.log_error(
f"Failed to verify module switch for {module_name}:{stream}/{profile}"
)
return Result.NOT_KEPT
Copy link
Copy Markdown
Contributor

@larsewi larsewi Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can flatten this code to avoid too many nested if's. If you turn the conditions on their head you could get something like:

enabled_stream = mpc.getEnabledStream(module_name)
if enabled_stream != stream:
    # Log error saying enabled stream was not equal to stream
    return Result.NOT_KEPT

installed_profiles = mpc.getInstalledProfiles(module_name)
if profile not in installed_profiles:
    # Log error saying profile not in installed_profiles
    return Result.NOT_KEPT

# etc... and then eventually return REPAIRED

It's a little bit more readable in my opinion. But it's only an opinion. There is nothing wrong with the way you did it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants