Master port #50773: zypper's locks to holds

[oeuftete]

Port of #50773.

---

What does this PR do?

Deprecates add_lock and remove_lock in favour of hold and unhold functions.

What issues does this PR fix or reference?

Fixes: #56922 (as part of making the original PR work)

Merge requirements satisfied?

• Docs
• Changelog
• Tests written/updated*: There are no new tests, but moving the functionality here to hold/unhold activated a handful of package integration tests (which flushed out some bugs)

Commits signed with GPG?

No

[oeuftete]

I wanted to get this master port up as part of addressing #56922 and customer support issues related to the hold/unhold functionality, which is already part of the SuSE-provided Salt packages. I can not currently take responsibility for writing the required test cases. @saltstack/team-suse, maybe?

[brejoc]

I'm missing a bit of context here. Seems like #50773 was originally merged but then reverted again!? Was it incomplete?

[brejoc]

Ah, sorry! I missed that that was develop for #50773. Okay, got it now.

[ScriptAutomate]

The functions being deprecated here should include something similar to the following at the top of their respective function docstrings:

.. deprecated:: 3002,3001.1
    This function is deprecated. Please use ``unhold()`` / ``hold()`` instead.

I opened a different issue related to how this guidance should be included in our Deprecating Code guidance: #58208

[ScriptAutomate]

@oeuftete Can you resolve the conflicts in salt/modules/zypperpkg.py? I'd like to get this in front of some reviewers to see how close this is to getting merged.

[twangboy]

We still need to create a reverse map here. Should be:

    u_name_map = {}
    name_map = get_repo_data(saltenv).get("name_map", {})
    for k in name_map:
        u_name_map[k] = name_map[k]
    return u_name_map

[oeuftete]

Good catch. Even from my commit comment, I'm not 100% sure why I had to mess with this file four months ago (and then messed it up), but I believe it was to satisfy pre-commit requirements at the time. This file has no business being changed in this PR if it doesn't need to, so I'm going try to just rip the commit changing this out.

Interesting that no existing tests caught my mistake, this might be something to address.

[oeuftete]

If it's meant to return the reverse map, I believe it was broken before...

-    u_name_map = {}
-    name_map = get_repo_data(saltenv).get("name_map", {})
-
-    if not six.PY2:
-        return name_map
-
-    for k in name_map:
-        u_name_map[k] = name_map[k]
-    return u_name_map
+    return get_repo_data(saltenv).get("name_map", {})

The reverse map would never be returned under py3. So that's why this was taken out when I de-sixed it.

[oeuftete]

@twangboy It looks like the original change is no longer required (the Windows tests pass), so I don't need to touch this module in this PR. If the lack of the reverse map is a bug, that should be addressed in a separate PR.

[oeuftete]

https://jenkins.saltproject.io/job/pr-opensuse15-py3-pytest/job/PR-56935/4/testReport/tests.integration.states.test_pkg/PkgTest/test_pkg_017_installed_held_equals_false/

tests.integration.states.test_pkg.PkgTest.test_pkg_017_installed_held_equals_false is now failing, which is related to the changes here.

E           AssertionError: 'held' not found in 'The following packages were installed/updated: lynx'

[oeuftete]

Ready for review again. Thanks to @garethgreenaway for helping sort out the latest test failure.

[OrangeDog]

Why were these renamed in the first place? The other two lock functions (clean_locks and list_locks) were not renamed.

Zypper has no concept of "hold". It calls them package locks.