From ebac4a8522f60032ff163bc6f8ee67db930683ae Mon Sep 17 00:00:00 2001 From: Twangboy Date: Fri, 24 Mar 2023 10:53:57 -0600 Subject: [PATCH 1/6] Use LooseVersion instead of Version --- salt/modules/win_pkg.py | 26 +++++++++++++++-------- tests/pytests/unit/utils/test_versions.py | 2 ++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/salt/modules/win_pkg.py b/salt/modules/win_pkg.py index 4aa73422f44e..21f953257c54 100644 --- a/salt/modules/win_pkg.py +++ b/salt/modules/win_pkg.py @@ -65,7 +65,7 @@ SaltInvocationError, SaltRenderError, ) -from salt.utils.versions import Version +from salt.utils.versions import LooseVersion log = logging.getLogger(__name__) @@ -1361,6 +1361,14 @@ def _get_msiexec(use_msiexec): return True, "msiexec" +def normalize_name(name): + """ + Nothing to do on Windows. We need this function so that Salt doesn't go + through every module looking for ``pkg.normalize_name``. + """ + return name + + def install(name=None, refresh=False, pkgs=None, **kwargs): r""" Install the passed package(s) on the system using winrepo @@ -1768,7 +1776,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs): # Install the software # Check Use Scheduler Option - log.debug("PKG : cmd: %s /s /c %s", cmd_shell, arguments) + log.debug("PKG : cmd: %s /c %s", cmd_shell, arguments) log.debug("PKG : pwd: %s", cache_path) if pkginfo[version_num].get("use_scheduler", False): # Create Scheduled Task @@ -1778,7 +1786,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs): force=True, action_type="Execute", cmd=cmd_shell, - arguments='/s /c "{}"'.format(arguments), + arguments='/c "{}"'.format(arguments), start_in=cache_path, trigger_type="Once", start_date="1975-01-01", @@ -1830,7 +1838,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs): else: # Launch the command result = __salt__["cmd.run_all"]( - '"{}" /s /c "{}"'.format(cmd_shell, arguments), + '"{}" /c "{}"'.format(cmd_shell, arguments), cache_path, output_loglevel="trace", python_shell=False, @@ -2126,7 +2134,7 @@ def remove(name=None, pkgs=None, **kwargs): cached_pkg = cached_pkg.replace("/", "\\") cache_path, _ = os.path.split(cached_pkg) - # os.path.expandvars is not required as we run everything through cmd.exe /s /c + # os.path.expandvars is not required as we run everything through cmd.exe /c if kwargs.get("extra_uninstall_flags"): uninstall_flags = "{} {}".format( @@ -2154,7 +2162,7 @@ def remove(name=None, pkgs=None, **kwargs): # Uninstall the software changed.append(pkgname) # Check Use Scheduler Option - log.debug("PKG : cmd: %s /s /c %s", cmd_shell, arguments) + log.debug("PKG : cmd: %s /c %s", cmd_shell, arguments) log.debug("PKG : pwd: %s", cache_path) if pkginfo[target].get("use_scheduler", False): # Create Scheduled Task @@ -2164,7 +2172,7 @@ def remove(name=None, pkgs=None, **kwargs): force=True, action_type="Execute", cmd=cmd_shell, - arguments='/s /c "{}"'.format(arguments), + arguments='/c "{}"'.format(arguments), start_in=cache_path, trigger_type="Once", start_date="1975-01-01", @@ -2181,7 +2189,7 @@ def remove(name=None, pkgs=None, **kwargs): else: # Launch the command result = __salt__["cmd.run_all"]( - '"{}" /s /c "{}"'.format(cmd_shell, arguments), + '"{}" /c "{}"'.format(cmd_shell, arguments), output_loglevel="trace", python_shell=False, redirect_stderr=True, @@ -2359,7 +2367,7 @@ def _reverse_cmp_pkg_versions(pkg1, pkg2): """ Compare software package versions """ - return 1 if Version(pkg1) > Version(pkg2) else -1 + return 1 if LooseVersion(pkg1) > LooseVersion(pkg2) else -1 def _get_latest_pkg_version(pkginfo): diff --git a/tests/pytests/unit/utils/test_versions.py b/tests/pytests/unit/utils/test_versions.py index eed780123056..417bb9e36321 100644 --- a/tests/pytests/unit/utils/test_versions.py +++ b/tests/pytests/unit/utils/test_versions.py @@ -77,6 +77,8 @@ def test_cmp_strict(v1, v2, wanted): # Added by us ("3.10.0-514.el7", "3.10.0-514.6.1.el7", 1), ("2.2.2", "2.12.1", -1), + ("2.24.0", "2.23.0.windows.1", 1), + ("2.23.0.windows.2", "2.23.0.windows.1", 1), ), ) def test_cmp(v1, v2, wanted): From 43859a3f5d632a51e3eae7dc1236e8368368f0e4 Mon Sep 17 00:00:00 2001 From: Twangboy Date: Fri, 24 Mar 2023 10:58:09 -0600 Subject: [PATCH 2/6] Add changelog --- changelog/63935.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/63935.fixed.md diff --git a/changelog/63935.fixed.md b/changelog/63935.fixed.md new file mode 100644 index 000000000000..565a7559af5f --- /dev/null +++ b/changelog/63935.fixed.md @@ -0,0 +1 @@ +Windows pkg module now properly handles versions containing strings From 5e7aac0d83fac46a133f81c1adc81da1779763a5 Mon Sep 17 00:00:00 2001 From: Twangboy Date: Fri, 24 Mar 2023 11:00:51 -0600 Subject: [PATCH 3/6] Fix pre-commit --- salt/modules/win_pkg.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/salt/modules/win_pkg.py b/salt/modules/win_pkg.py index 21f953257c54..073d373b02a0 100644 --- a/salt/modules/win_pkg.py +++ b/salt/modules/win_pkg.py @@ -1365,6 +1365,18 @@ def normalize_name(name): """ Nothing to do on Windows. We need this function so that Salt doesn't go through every module looking for ``pkg.normalize_name``. + + Args: + name (str): The name of the package + + Returns: + str: The name of the package + + CLI Example: + + .. code-block:: bash + + salt '*' pkg.normalize_name git """ return name From fdc122d7c04e8956903d3246a8e5e7980e5c14ee Mon Sep 17 00:00:00 2001 From: Twangboy Date: Fri, 24 Mar 2023 12:43:00 -0600 Subject: [PATCH 4/6] Add tests for using LooseVersion --- tests/pytests/unit/modules/test_win_pkg.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/pytests/unit/modules/test_win_pkg.py b/tests/pytests/unit/modules/test_win_pkg.py index d8551f9a85a5..b22b58389ddb 100644 --- a/tests/pytests/unit/modules/test_win_pkg.py +++ b/tests/pytests/unit/modules/test_win_pkg.py @@ -321,7 +321,7 @@ def test_pkg_install_log_message(caplog): extra_install_flags="-e True -test_flag True", ) assert ( - 'PKG : cmd: C:\\WINDOWS\\system32\\cmd.exe /s /c "runme.exe" /s -e ' + 'PKG : cmd: C:\\WINDOWS\\system32\\cmd.exe /c "runme.exe" /s -e ' "True -test_flag True" ).lower() in [x.lower() for x in caplog.messages] assert "PKG : pwd: ".lower() in [x.lower() for x in caplog.messages] @@ -540,7 +540,7 @@ def test_pkg_remove_log_message(caplog): pkgs=["firebox"], ) assert ( - 'PKG : cmd: C:\\WINDOWS\\system32\\cmd.exe /s /c "%program.exe" /S' + 'PKG : cmd: C:\\WINDOWS\\system32\\cmd.exe /c "%program.exe" /S' ).lower() in [x.lower() for x in caplog.messages] assert "PKG : pwd: ".lower() in [x.lower() for x in caplog.messages] assert "PKG : retcode: 0" in caplog.messages @@ -629,3 +629,15 @@ def test_pkg_remove_minion_error_salt(): ) assert ret == expected + + +@pytest.mark.parametrize( + "v1,v2,expected", + ( + ("2.24.0", "2.23.0.windows.1", 1), + ("2.23.0.windows.2", "2.23.0.windows.1", 1), + ) +) +def test__reverse_cmp_pkg_versions(v1, v2, expected): + result = win_pkg._reverse_cmp_pkg_versions(v1, v2) + assert result == expected, "cmp({}, {}) should be {}, got {}".format(v1, v2, wanted, res) From 932d890b720b1bdfbf3e6ee0aeb9440baaa4398d Mon Sep 17 00:00:00 2001 From: Twangboy Date: Mon, 27 Mar 2023 09:13:20 -0600 Subject: [PATCH 5/6] Fix pre-commit --- tests/pytests/unit/modules/test_win_pkg.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/pytests/unit/modules/test_win_pkg.py b/tests/pytests/unit/modules/test_win_pkg.py index b22b58389ddb..d1a222221294 100644 --- a/tests/pytests/unit/modules/test_win_pkg.py +++ b/tests/pytests/unit/modules/test_win_pkg.py @@ -636,8 +636,10 @@ def test_pkg_remove_minion_error_salt(): ( ("2.24.0", "2.23.0.windows.1", 1), ("2.23.0.windows.2", "2.23.0.windows.1", 1), - ) + ), ) def test__reverse_cmp_pkg_versions(v1, v2, expected): result = win_pkg._reverse_cmp_pkg_versions(v1, v2) - assert result == expected, "cmp({}, {}) should be {}, got {}".format(v1, v2, wanted, res) + assert result == expected, "cmp({}, {}) should be {}, got {}".format( + v1, v2, expected, result + ) From ccf5a9dc34609706d826df3896eb7d2402dca094 Mon Sep 17 00:00:00 2001 From: Twangboy Date: Mon, 27 Mar 2023 12:16:45 -0600 Subject: [PATCH 6/6] Add versionadded --- salt/modules/win_pkg.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/salt/modules/win_pkg.py b/salt/modules/win_pkg.py index 073d373b02a0..1cb2f0971245 100644 --- a/salt/modules/win_pkg.py +++ b/salt/modules/win_pkg.py @@ -1366,6 +1366,8 @@ def normalize_name(name): Nothing to do on Windows. We need this function so that Salt doesn't go through every module looking for ``pkg.normalize_name``. + .. versionadded:: 3006.0 + Args: name (str): The name of the package