Skip to content
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.

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

[BUG] pkg.installed (aptpkg latest_version) UnboundLocalError on foreign-arch package #66940

Open
scy opened this issue Oct 1, 2024 · 5 comments
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@scy
Copy link

scy commented Oct 1, 2024

Description
When installing WINE under Debian on an amd64 machine, you usually need to pull i386 packages, too. However, the pkg.installed state seems to struggle with packages only available in foreign architectures, at least when you specify them without their architecture name.

Setup

  • on-prem machine
  • onedir packaging
  • masterless

Steps to Reproduce the behavior
On an amd64 machine, try a state like this:

WINE packages:
  cmd.run:
    - unless:
        # Don't run if already enabled.
        - dpkg --print-foreign-architectures | grep -Fqx i386
    - name: dpkg --add-architecture i386 && apt update

  pkg.installed:
    - require:
        - cmd: WINE packages
    - pkgs:
        - fonts-wine
        - libwine
        - libwine:i386
        - wine
        - wine32
        - wine64

This will fail as follows:

          ID: WINE packages
    Function: pkg.installed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/state.py", line 2428, in call
                  ret = self.states[cdata["full"]](
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 160, in __call__
                  ret = self.loader.run(run_func, *args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1269, in run
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1284, in _run_as
                  return _func_or_method(*args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1317, in wrapper
                  return f(*args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/states/pkg.py", line 1934, in installed
                  pkg_ret = __salt__["pkg.install"](
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 160, in __call__
                  ret = self.loader.run(run_func, *args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1269, in run
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1284, in _run_as
                  return _func_or_method(*args, **kwargs)
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/modules/aptpkg.py", line 819, in install
                  _latest_version = latest_version(
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/modules/aptpkg.py", line 507, in latest_version
                  candidates[this_pkg] = candidate
              UnboundLocalError: local variable 'this_pkg' referenced before assignment
     Started: 16:01:22.614404
    Duration: 50.036 ms
     Changes:

The code in question is in aptpkg.latest_version():

    cmd = ["apt-cache", "-q", "policy"]
    cmd.extend(names)
    if repo is not None:
        cmd.extend(repo)
    out = _call_apt(cmd, scope=False)

    short_names = [nom.split(":", maxsplit=1)[0] for nom in names]

    candidates = {}
    for line in salt.utils.itertools.split(out["stdout"], "\n"):
        if line.endswith(":") and line[:-1] in short_names:
            this_pkg = names[short_names.index(line[:-1])]
        elif "Candidate" in line:
            candidate = ""
            comps = line.split()
            if len(comps) >= 2:
                candidate = comps[-1]
                if candidate.lower() == "(none)":
                    candidate = ""
            candidates[this_pkg] = candidate

As you can see, the this_pkg variable is set in the if branch, but referenced in the elif branch. This is at least brittle, if not outright wrong.

The package causing this issue is wine32, since it doesn't exist in the amd64 architecture and thus apt-cache prints its name with an architecture suffix:

# apt-cache -q policy wine32
wine32:i386:
  Installed: 8.0~repack-4
  Candidate: 8.0~repack-4
  Version table:
 *** 8.0~repack-4 500
        500 http://deb.debian.org/debian bookworm/main i386 Packages
        100 /var/lib/dpkg/status

line.endswith(":") and line[:-1] in short_names doesn't match that.

salt-call pkg.latest_version wine32 is also broken, with the same UnboundLocalError.

Changing the line in the YAML from - wine32 to - wine32:i386 allows the state to apply cleanly.

In contrast, salt-call pkg.latest_version wine32:i386 still doesn't work.

Expected behavior
Not quite sure actually. Maybe cut off the architecture suffix when parsing the apt-cache output? Or give a warning like package 'wine32' was not found, are you missing an architecture suffix (e.g. 'wine32:i386')??

In any case, the parsing loop should be modified in order to not try and parse Candidate: lines when this_pkg has not yet been set.

Versions Report

salt --versions-report
Salt Version:
          Salt: 3007.1
 
Python Version:
        Python: 3.10.14 (main, Apr  3 2024, 21:30:09) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.16.0
      cherrypy: 18.8.0
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.4
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 23.1
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: 0.16.0
         smmap: Not Installed
       timelib: 0.3.0
       Tornado: 6.3.3
           ZMQ: 4.3.4
 
Salt Package Information:
  Package Type: onedir
 
System Versions:
          dist: debian 12.7 bookworm
        locale: utf-8
       machine: x86_64
       release: 6.1.0-25-amd64
        system: Linux
       version: Debian GNU/Linux 12.7 bookworm
@scy scy added Bug broken, incorrect, or confusing behavior needs-triage labels Oct 1, 2024
@dmurphy18
Copy link
Contributor

@scy Part of the problem may be that Salt doesn't support 32-bit packages on Linux and hasn't for some time.

@scy
Copy link
Author

scy commented Oct 1, 2024

@scy Part of the problem may be that Salt doesn't support 32-bit packages on Linux and hasn't for some time.

I don't think that should keep it from supporting multi-arch package installation in general though. Even if Salt itself does not support 32-bit anymore, Debian still does, and the issue here basically boils down to simple string matching. Also, multi-arch is not just about x86 vs x64, it also applies to different ARM subarchitectures or even cross-platform emulation with things like QEMU and binfmt.

@dmurphy18
Copy link
Contributor

@scy FYI dropped support for armfh too. With the recent changes in the Salt Core Team, limited resources to address 32-bit items.
Of course when Windows moves to a Linux kernel and runs Wine for Windows support, perhaps we will have to look further 🤣 , joking of course.
A PR with the fix, changelog and pytest unit tests exercising the code will get this fixed much quicker, otherwise this will be on the TODO list, sorry but limited resources.

@scy
Copy link
Author

scy commented Oct 1, 2024

@scy FYI dropped support for armfh too. With the recent changes in the Salt Core Team, limited resources to address 32-bit items.
[…]
A PR with the fix, changelog and pytest unit tests exercising the code will get this fixed much quicker, otherwise this will be on the TODO list, sorry but limited resources.

That's alright and understandable, all good. :) I have outlined a simple workaround in the original report above; this is therefore probably a low priority issue.

However, I still think we're somehow miscommunicating here. This issue has nothing to do with running Salt on 32-bit machines. Instead, this is about Debian's APT package manager being able to install 32-bit packages on 64-bit machines, which are completely unrelated to Salt's supported architectures. In fact, I could even install ARM packages on my AMD64 machine and run them transparently with QEMU, Linux supports these kinds of shenanigans. This issue is just about Salt not stumbling over cross-architecture packages that I wish to install.

@dmurphy18
Copy link
Contributor

@scy Understand, have done all manner of crazy things on top of Linux, and older arches too (even used QEMU in a past life) What you want is a corner case, hence it would be on TODO, with the few of us left, living in fire-fighting mode is taking a lot of the time allotted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

No branches or pull requests

2 participants