Better support for dnf5 in Fedora 41/42

[gregoster]

'grouplist' and 'groupinfo' are each now two words. 'group' and 'group-id' have been replaced with 'name' and 'id', so check for those too.

What does this PR do?

Improve support for installing via groups with dnf5 on Fedora 41/42. Issue found when attempting to run salt 3007 on Fedora 42. Problem also exists in master/main, so starting there.

What issues does this PR fix or reference?

Fixes salt failing on a Fedora 41/42 install when (e.g.) installing KDE Fonts via a group:

Fonts:
  pkg.group_installed

The name of the group doesn't seem to matter -- all groups fail, as 'groupinfo' and 'grouplist' have been deprecated since dnf5 as shipped with Fedora 41.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Salt’s Contributor Guide
• Join our Community Discord
• Salt Project YouTube channel
• Community Wiki

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject.pdl@broadcom.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[twangboy]

Please create a changelog and write a test for this.

[gregoster]

Changelog entry added.

I looked to add a test to test/pytest/unit/modules/test_yumpkg.py, but on running (without any changes):

python -m nox -e "test-3(coverage=False)" -- tests/pytests/unit/modules/test_yumpkg.py
it blew up in various ways ("Building wheel for immutables (setup.py): finished with status 'error'", among others) and so I've punted. :(

I don't have a test environment setup for salt development, and don't intend to be doing further salt development, so my incentive to debug why this didn't Just Work is quite low :( Sorry.

[twangboy]

Could you write a simple test here, or ensure that existing tests cover these changes

[gregoster]

Please see my previous comment. I've tried to build a test, but I'm failing right now to even get a basic test environment setup.
Command python -m pip install --progress-bar=off -r requirements/static/ci/py3.13/linux.txt failed with exit code 1: ... <many lines of failure> ...
So while I could probably dig enough to figure out what to write for a test case, I have no way to test my test :(

[dwoz]

@gregoster Sorry, there are more conflicts that need to be addressed.

[ggiesen]

Heads-up that this also affects the 3006 LTS line: dnf5 ships on Fedora 41+ and AlmaLinux/RHEL 10, and on 3006.26 pkg.group_list / pkg.group_info (and therefore pkg.group_installed) already fail there -- confirmed on Fedora 44:

Unknown argument "grouplist" for command "dnf5"

Since 3006.x is supported into 2027 and the contributing guide has bug fixes start on the oldest supported branch and merge forward, would it make sense to target this at 3006.x rather than master? The group code has diverged a little between the two (3006.x carries the #60276 env-group member-expansion change), so it'd be a small rebase rather than a straight base swap.

If it helps, I'm happy to put up a 3006.x backport that preserves your authorship and adds the changelog + tests -- just say the word, or feel free to retarget this one. Thanks for chasing this down.

[gregoster]

If it helps, I'm happy to put up a 3006.x backport that preserves your authorship and adds the changelog + tests -- just say the word, or feel free to retarget this one. Thanks for chasing this down.

I'm happy to have you (or anyone else) take over the lead on this :)

[twangboy]

The Problem
The current regex r"^(\S+?)\s+(.+?)\s*(yes|no)$" relies on a lazy match (.+?) for the group name. If a group name naturally contains the word "yes" or "no" (e.g., "Just (testing) yes"), the lazy match can cut off early or misalign the columns.

The Fix
Since terminal tables from tools like dnf5 use predictable, whitespace-aligned columns, it is safer to tokenize the line from the right side first to isolate the status, or use a regex that explicitly binds the trailing status to the end of the line while allowing spaces in the name.

# Instead of: match = re.match(r"^(\S+?)\s+(.+?)\s*(yes|no)$", line_lc)

# A more robust regex approach:
# This ensures the group ID is a single word, the status is strictly the last word,
# and everything in between is captured cleanly as the group name.
match = re.match(r"^(\S+)\s+(.+)\s+(yes|no)$", line_lc)

Alternatively, you can split by whitespace and pop the trailing status directly, which completely avoids regex backtracking traps:

parts = line.strip().split()
if len(parts) >= 3:
    pkg_installed = parts[-1].lower()
    if pkg_installed in ("yes", "no"):
        pkg_id = parts[0]
        pkg_name = " ".join(parts[1:-1])
        # Proceed with logic...

[twangboy]

The Problem
Mutating the line variable mid-stream using a broad re.match(r"^.*:\s*(.*?)$", line) can result in an empty string if a header layout has trailing spaces or localized structural content. If line becomes empty, it can bypass downstream assertions or drop packages.

The Fix
Instead of overwriting the loop context variable line directly and executing an immediate continue, explicitly extract the package payload into a dedicated variable name and process it safely inside a structured block:

# Instead of mutating the loop variable:
# if rematch_dnf5:
#     line = rematch_dnf5.group(1)

# Safer approach:
match_dnf5 = re.match(
    pkgtypes_capturegroup + r" (?:groups|packages)\s*:\s*(.*?)$",
    line.lower(),
)
if match_dnf5:
    if target_found:
        break
    
    if match_dnf5.group(1) == pkgtype:
        target_found = True
        
        # Safely extract the inline package without losing the loop context
        header_package_match = re.match(r"^[^:]+:\s*(.+)$", line)
        if header_package_match:
            inline_pkg = header_package_match.group(1).strip()
            if inline_pkg and inline_pkg not in completed_groups:
                # Append the inline package immediately here
                if expand and ret["type"] == "environment group":
                    # handle environment group nesting...
                    pass
                else:
                    ret[pkgtype].append(inline_pkg)
        continue

[twangboy]

While the second .lstrip() was likely added to catch any new leading spaces exposed after removing punctuation, this structure leaves a silent bug if a line contains trailing punctuation.

To guarantee that the string is completely tightened on both ends regardless of what characters or whitespace get shifted around by the punctuation drop, the final call should be a full .strip().

line = line.strip().lstrip(string.punctuation).strip()

[ggiesen]

@gregoster thanks for the go-ahead. I've put the 3006.x backport up as #69813, with you as co-author.

Since 3006.x's group_info already carries the #60276 multi-word member-group @-fallback (which master doesn't have yet), I grafted your dnf5 support onto the current 3006.x functions rather than cherry-picking, so that fix is preserved.

@twangboy I also folded in your three review points there:

• group_list now tokenizes the row (ID + the trailing yes/no) instead of the lazy regex, so a name that contains or ends in "yes"/"no" isn't mistaken for the status column -- and as a side effect it no longer drops a row with trailing whitespace (which the regex did).
• group_info pulls the dnf5 inline first package into a dedicated variable instead of overwriting line.
• The section-line cleaning is a full .strip() on both ends.

Validated end to end against live dnf5 5.4.2.1 on Fedora 44: group_list parses all 157 groups and group_info drops no packages (41 parsed == 41 raw package lines for development-tools), and it reproduces your test_67975_dnf5_group_info expected output exactly, @gregoster.

This master PR still stands on its own -- happy to help forward-port the same three fixes here if that's useful.

[twangboy]

Closing in favor of: #69813
Those fixes will be merged forward in to later branches and will eventually end up on master.