Skip to content

Preserve multiarch apt package names in pkg.installed #68933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TobiPeterG wants to merge 1 commit into
base: 3006.x
Choose a base branch
from
Open

Preserve multiarch apt package names in pkg.installed #68933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/68932.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed pkg.installed(update_holds=True) for APT multiarch packages by preserving arch-qualified package names through install target parsing and verification.
32 changes: 31 additions & 1 deletion salt/modules/aptpkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ def install(
reinstall=False,
downloadonly=False,
ignore_epoch=False,
normalize=True,
Comment thread
TobiPeterG marked this conversation as resolved.
**kwargs,
):
"""
Expand Down Expand Up @@ -737,12 +738,21 @@ def install(

.. versionadded:: 2018.3.0

normalize : True
Normalize the package name by removing the architecture. Set this to
``False`` to preserve explicitly arch-qualified package names such as
``name:amd64``.

Multiple Package Installation Options:

pkgs
A list of packages to install from a software repository. Must be
passed as a python list.

For multiarch packages, use the arch-qualified package name (for
example ``name:amd64``) when you need Salt to target that exact APT
package name.

CLI Example:

.. code-block:: bash
Expand Down Expand Up @@ -828,7 +838,11 @@ def install(

try:
pkg_params, pkg_type = __salt__["pkg_resource.parse_targets"](
name, pkgs, sources, **kwargs
name,
pkgs,
sources,
normalize=normalize and kwargs.get("split_arch", True),
**kwargs,
)
except MinionError as exc:
raise CommandExecutionError(exc)
Expand Down Expand Up @@ -1362,6 +1376,10 @@ def hold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W0613
name
The name of the package, e.g., 'tmux'

For multiarch packages, dpkg may record the held package name with an
architecture suffix such as ``:amd64``. In those cases, the
arch-qualified package name may need to be used.

CLI Example:

.. code-block:: bash
Expand All @@ -1371,6 +1389,10 @@ def hold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W0613
pkgs
A list of packages to hold. Must be passed as a python list.

For multiarch packages, dpkg may record the held package name with an
architecture suffix such as ``:amd64``. In those cases, the
arch-qualified package name may need to be used.

CLI Example:

.. code-block:: bash
Expand Down Expand Up @@ -1427,6 +1449,10 @@ def unhold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W06
name
The name of the package, e.g., 'tmux'

For multiarch packages, dpkg may record the held package name with an
architecture suffix such as ``:amd64``. In those cases, the
arch-qualified package name may need to be used.

CLI Example:

.. code-block:: bash
Expand All @@ -1436,6 +1462,10 @@ def unhold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W06
pkgs
A list of packages to unhold. Must be passed as a python list.

For multiarch packages, dpkg may record the held package name with an
architecture suffix such as ``:amd64``. In those cases, the
arch-qualified package name may need to be used.

CLI Example:

.. code-block:: bash
Expand Down
26 changes: 20 additions & 6 deletions salt/states/pkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,9 @@ def _find_install_targets(
if any((pkgs, sources)):
if pkgs:
# pylint: disable=not-callable
desired = _repack_pkgs(pkgs, normalize=normalize)
desired = _repack_pkgs(
pkgs, normalize=normalize and kwargs.get("split_arch", True)
)
# pylint: enable=not-callable
elif sources:
desired = __salt__["pkg_resource.pack_sources"](
Expand Down Expand Up @@ -904,12 +906,15 @@ def _verify_install(desired, new_pkgs, ignore_epoch=None, new_caps=None):
cver = new_pkgs.get(pkgname, new_pkgs.get(pkgname.split("/")[-1]))
elif __grains__["os"] == "OpenBSD":
cver = new_pkgs.get(pkgname.split("%")[0])
elif __grains__["os_family"] == "Debian":
cver = new_pkgs.get(pkgname.split("=")[0])
else:
cver = new_pkgs.get(pkgname)
Comment on lines -907 to -910
Copy link
Copy Markdown
Contributor

@bdrx312 bdrx312 Apr 15, 2026

Choose a reason for hiding this comment

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

Why were these 2 conditions combined?

Copy link
Copy Markdown
Author

@TobiPeterG TobiPeterG Apr 16, 2026

Choose a reason for hiding this comment

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

I combined them because both branches were doing the same installed-package lookup pattern, and the new normalization fallback needed to apply to both. I kept the Debian-specific split("=")[0] behavior via the lookup_name variable, but merged the rest to avoid duplicating the fallback logic.
I can also split them again, then it would look like this:

        elif __grains__["os_family"] == "Debian":
            lookup_name = pkgname.split("=")[0]
            cver = new_pkgs.get(lookup_name)
            if not cver and "pkg.normalize_name" in __salt__:
                normalized_name = __salt__["pkg.normalize_name"](lookup_name)
                if normalized_name != lookup_name:
                    cver = new_pkgs.get(normalized_name)
        else:
            cver = new_pkgs.get(pkgname)
            if not cver and "pkg.normalize_name" in __salt__:
                normalized_name = __salt__["pkg.normalize_name"](pkgname)
                if normalized_name != pkgname:
                    cver = new_pkgs.get(normalized_name)
            if not cver and pkgname in new_caps:
                cver = new_pkgs.get(new_caps.get(pkgname)[0])

So only the initial lookup key differs. Is this the prefered solution? :)

Copy link
Copy Markdown
Contributor

@bdrx312 bdrx312 Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Another option would be:

        else
            if __grains__["os_family"] == "Debian":
                lookup_name = pkgname.split("=")[0]
            else:
                lookup_name = pkgname
            cver = new_pkgs.get(lookup_name)
            if not cver and "pkg.normalize_name" in __salt__:
                normalized_name = __salt__["pkg.normalize_name"](lookup_name)
                if normalized_name != lookup_name:
                    cver = new_pkgs.get(normalized_name)
            if not cver and pkgname in new_caps:
                cver = new_pkgs.get(new_caps.get(pkgname)[0])

but I think it is fine to always split on the equal sign for both like your code is already doing since = is not a valid character in rpm names either. So I think what you have is ideal if both need to do this normalization.

I don't think yum/dnf supports multi architecture packages though, so it isn't clear from the issue/PR description why yum package logic is changing. Maybe you can update the descriptions to elaborate.

Copy link
Copy Markdown
Author

@TobiPeterG TobiPeterG Apr 20, 2026

Choose a reason for hiding this comment

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

YUM/DNF already supports explicit RPM arch-qualified package names such as .x86_64 and Salt already has split_arch handling in the YUM install path.
This change is not adding new YUM package-manager behavior, it just makes the generic pkg.installed verification logic consistent with that existing behavior, so explicit arch-qualified package names do not falsely fail verification.

Should I add this to the description of the PR?

Copy link
Copy Markdown
Contributor

@bdrx312 bdrx312 Apr 20, 2026

Choose a reason for hiding this comment

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

I think that would be a good idea. I am just a user though and not a maintainer/approver.

Copy link
Copy Markdown
Author

@TobiPeterG TobiPeterG Apr 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Alright, do you know when to expect a review? Am I missing something in this PR? :)

And thank you for your review comments :)

Copy link
Copy Markdown
Contributor

@bdrx312 bdrx312 Apr 21, 2026

Choose a reason for hiding this comment

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

The reviews tend to come in large batches when they have time. This PR appears to have all the necessary items.

if not cver and pkgname in new_caps:
cver = new_pkgs.get(new_caps.get(pkgname)[0])
lookup_name = pkgname.split("=")[0]
cver = new_pkgs.get(lookup_name)
if not cver and "pkg.normalize_name" in __salt__:
normalized_name = __salt__["pkg.normalize_name"](lookup_name)
if normalized_name != lookup_name:
cver = new_pkgs.get(normalized_name)
if not cver and lookup_name in new_caps:
cver = new_pkgs.get(new_caps.get(lookup_name)[0])

if not cver:
failed.append(pkgname)
Expand Down Expand Up @@ -1481,6 +1486,10 @@ def installed(
package, the held package(s) will be skipped and the state will fail.
By default, this parameter is set to ``False``.

Package naming rules for held packages may vary by package manager.
See the documentation for your platform's ``pkg`` module for any
provider-specific requirements.

Supported on YUM/DNF & APT based systems.

.. versionadded:: 2016.11.0
Expand Down Expand Up @@ -1712,6 +1721,7 @@ def installed(
ignore_epoch=ignore_epoch,
reinstall=reinstall,
refresh=refresh,
split_arch=False,
**kwargs,
)

Expand Down Expand Up @@ -3876,6 +3886,10 @@ def unheld(name, version=None, pkgs=None, all=False, **kwargs):
For ``unheld`` there is no need to specify the exact version
to be unheld.

Package naming rules for held packages may vary by package manager.
See the documentation for your platform's ``pkg`` module for any
provider-specific requirements.

:param bool all:
Force removing of all existings locks.
By default, this parameter is set to ``False``.
Expand Down
34 changes: 34 additions & 0 deletions tests/pytests/unit/modules/test_aptpkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,40 @@ def test_install(install_var):
assert expected_call in mock_call_apt.mock_calls


def test_install_preserves_multiarch_pkg_names_when_split_arch_is_false():
"""
Test aptpkg.install preserves explicit multiarch package names.
"""
patch_kwargs = {
"__salt__": {
"pkg_resource.parse_targets": MagicMock(
return_value=({"libnvidia-cfg1-570-server:amd64": None}, "repository")
),
"pkg_resource.sort_pkglist": MagicMock(),
"pkg_resource.stringify": MagicMock(),
"cmd.run_stdout": MagicMock(return_value=""),
}
}
mock_call_apt = MagicMock(return_value={"retcode": 0, "stdout": "", "stderr": ""})
mock_parse_targets = patch_kwargs["__salt__"]["pkg_resource.parse_targets"]

with patch.multiple(
aptpkg, list_pkgs=MagicMock(side_effect=[{}, {}]), **patch_kwargs
):
with patch(
"salt.modules.aptpkg.get_selections", MagicMock(return_value={"hold": []})
):
with patch("salt.modules.aptpkg._call_apt", mock_call_apt):
aptpkg.install(
pkgs=["libnvidia-cfg1-570-server:amd64"],
refresh=False,
split_arch=False,
)

assert mock_parse_targets.call_args.kwargs["normalize"] is False
assert "libnvidia-cfg1-570-server:amd64" in mock_call_apt.mock_calls[0].args[0]


def test_remove(uninstall_var):
"""
Test - Remove packages.
Expand Down
111 changes: 109 additions & 2 deletions tests/pytests/unit/states/test_pkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ def test_installed_with_single_normalize():
"pkg.install": yumpkg.install,
"pkg.list_pkgs": list_pkgs,
"pkg.normalize_name": yumpkg.normalize_name,
"pkg_resource.check_extra_requirements": MagicMock(return_value=True),
"pkg_resource.version_clean": pkg_resource.version_clean,
"pkg_resource.parse_targets": pkg_resource.parse_targets,
}
Expand Down Expand Up @@ -859,7 +860,7 @@ def test_installed_with_single_normalize():
)
call_yum_mock.assert_called_once()
assert (
"weird-name-1.2.3-1234.5.6.test7tst.x86_64-20220214-2.1"
"weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch-20220214-2.1"
in call_yum_mock.mock_calls[0].args[0]
)
assert ret["result"]
Expand All @@ -873,13 +874,119 @@ def test_installed_with_single_normalize():
)
call_yum_mock.assert_called_once()
assert (
"weird-name-1.2.3-1234.5.6.test7tst.x86_64"
"weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch"
in call_yum_mock.mock_calls[0].args[0]
)
assert ret["result"]
assert ret["changes"] == expected


def test_installed_preserves_apt_multiarch_pkg_names_for_update_holds():
"""
Test pkg.installed keeps explicit APT multiarch package names intact.
"""
pkg_name = "zlib1g:amd64"
pkg_version = "1:1.3.dfsg-3.1ubuntu2.1"
install_mock = MagicMock(
return_value={pkg_name: {"old": "1:1.3.dfsg-3.1ubuntu2", "new": pkg_version}}
)
list_pkgs_mock = MagicMock(
side_effect=[
{pkg_name: ["1:1.3.dfsg-3.1ubuntu2"]},
{"zlib1g": [pkg_version]},
{"zlib1g": [pkg_version]},
]
)

salt_dict = {
"pkg.install": install_mock,
"pkg.list_pkgs": list_pkgs_mock,
"pkg.normalize_name": lambda pkg: (
pkg.rsplit(":", 1)[0] if pkg.endswith(":amd64") else pkg
),
"lowpkg.unpurge": MagicMock(return_value={}),
"pkg_resource.check_extra_requirements": MagicMock(return_value=True),
"pkg_resource.version_clean": pkg_resource.version_clean,
}

with patch.dict(pkg.__salt__, salt_dict), patch.dict(
pkg_resource.__salt__, salt_dict
), patch.dict(
pkg.__grains__, {"os": "Ubuntu", "os_family": "Debian", "osarch": "amd64"}
), patch.dict(
pkg_resource.__grains__, {"os": "Ubuntu", "os_family": "Debian"}
):
ret = pkg.installed(
"test_install",
pkgs=[{pkg_name: pkg_version}],
skip_suggestions=True,
update_holds=True,
)

install_mock.assert_called_once_with(
name=None,
refresh=False,
version=None,
fromrepo=None,
skip_verify=False,
pkgs=[{pkg_name: pkg_version}],
sources=None,
reinstall=False,
normalize=True,
update_holds=True,
ignore_epoch=None,
split_arch=False,
allow_updates=False,
saltenv="base",
)
assert ret["result"]
assert ret["changes"] == {
pkg_name: {"old": "1:1.3.dfsg-3.1ubuntu2", "new": pkg_version},
}


def test_verify_install_normalizes_debian_multiarch_names():
"""
Test _verify_install matches Debian native-arch package names correctly.
"""
desired = {"zlib1g:amd64": "1:1.3.dfsg-3.1ubuntu2.1"}
new_pkgs = {"zlib1g": ["1:1.3.dfsg-3.1ubuntu2.1"]}

with patch.dict(
pkg.__salt__,
{
"pkg.normalize_name": lambda name: (
name.rsplit(":", 1)[0] if name.endswith(":amd64") else name
),
"pkg_resource.version_clean": pkg_resource.version_clean,
},
), patch.dict(pkg.__grains__, {"os": "Ubuntu", "os_family": "Debian"}):
ok, failed = pkg._verify_install(desired, new_pkgs)

assert ok == ["zlib1g:amd64"]
assert failed == []


def test_verify_install_normalizes_yum_arch_names():
"""
Test _verify_install matches YUM package names using normalized names.
"""
desired = {"weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch": "20220214-2.1"}
new_pkgs = {"weird-name-1.2.3-1234.5.6.test7tst.x86_64": ["20220214-2.1"]}

with patch.dict(
pkg.__salt__,
{
"pkg.normalize_name": yumpkg.normalize_name,
"pkg_resource.version_clean": pkg_resource.version_clean,
},
), patch.dict(pkg.__grains__, {"os": "CentOS", "os_family": "RedHat"}):
ok, failed = pkg._verify_install(desired, new_pkgs)

assert ok == ["weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch"]
assert failed == []


def test_removed_with_single_normalize():
"""
Test pkg.removed with preventing multiple package name normalisation
Expand Down
Loading