Skip to content

Add list_repo_pkgs to modules/pkgng.py - #70321

Open
Jsollvander wants to merge 2 commits into
saltstack:masterfrom
Jsollvander:pkgng_list_repo_pkgs
Open

Jsollvander wants to merge 2 commits into
saltstack:masterfrom
Jsollvander:pkgng_list_repo_pkgs

Conversation

@Jsollvander

Copy link
Copy Markdown

What does this PR do?

See discussion for details.

What issues does this PR fix or reference?

Fixes #70317.

Previous Behavior

salt-call pkg.install zrepl test=true
local:
    ----------

New Behavior

salt-call pkg.install zrepl test=true
local:
    ----------
    zrepl:
        ----------
        new:
            0.7.0_8
        old:

Merge requirements satisfied?

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

Commits signed with GPG?

Yes

This is my first time contributing code here. Let me know if I need to fix anything.

@Jsollvander
Jsollvander requested a review from a team as a code owner September 24, 2026 12:53
@welcome

welcome Bot commented Sep 24, 2026

Copy link
Copy Markdown

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:

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 twangboy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1. Bug in Output Parsing (ValueError: not enough values to unpack):

Lines 2380–2382:

for line in salt.utils.itertools.split(out, "\n"):
    pkg, version = line.strip().rsplit("-", 1)
    ret.setdefault(pkg, []).append(version)

Problem: If FreeBSD's pkg search outputs an empty line, a malformed line, or if out is empty, line.strip().rsplit("-", 1) will raise a ValueError when unpacking into pkg, version. Additionally, package names or version strings could theoretically lack a -.

Fix: Guard against empty lines and ensure rsplit produced two elements:

for line in salt.utils.itertools.split(out, "\n"):
    line = line.strip()
    if not line or "-" not in line:
        continue
    pkg, version = line.rsplit("-", 1)
    ret.setdefault(pkg, []).append(version)

2. Inaccurate Docstring Example:

The docstring example (lines 2340–2345) shows Ubuntu/Debian version strings (4.3-14ubuntu1.1). Because pkgng is specific to FreeBSD, updating the example to reflect FreeBSD package versions (e.g., bash-5.2.21, nginx-1.26.1,2) would keep documentation accurate.

3. Missing Unit Test for Multiple Arguments / Globs:

test_list_repo_pkgs currently only tests passing a single package ("vim"). Adding a unit test that verifies passing multiple package arguments ("vim", "zsh") or empty output parsing edge cases would ensure full coverage of the execution path.

@twangboy twangboy added the test:full Run the full test suite label Sep 24, 2026
@twangboy twangboy added this to the Potassium v3009.0 milestone Sep 24, 2026
Make docstring more accurateg.
Handle empty lines better and check for "-" before rsplit().
Add two more tests, one with multiple args and one for globs.

This branch was successfully deployed

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

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants