apt.install should install dependencies when directly installing .deb files

[tweenk]

Let's say I have a state file like this:

some-package:
  pkg.installed:
    - sources:
      - some-package: salt://path/to/some-package.deb

This will fail if some dependencies of package.deb are not already installed on the target system. As a result, a lot of unnecessary boilerplate is needed to install the dependencies first, and they won't be reported as no longer needed when some-package is removed.

apt.install should call apt-get install -fy after installing the package to resolve dependencies.

[tedski]

This would require adding --force-depends to the current command dpkg -i --force-confold. We /could/ then parse the output from dpkg and search for text indicating unresolved deps. However, running apt-get install -fy doesn't seem like the right way to resolve these. We don't know the state of apt before running that and we might do more than we want to. It feels uncomfortable. Discuss.

[tweenk]

If using apt-get install -fy is not desired, the 100% correct algoithm would be like this:

1. Parse the Depends line in the control file, extracted with dpkg-deb --control.
2. Save the list of installed packages to memory or to a tempfile, obtained with e.g. dpkg -l | grep '\sinstall' | cut -f 1
3. Install the dependencies with apt-get install.
4. Save the list of installed packages.
5. Install the .deb file with dpkg -i
6. Compare the lists from 2 and 4. Mark newly installed packages as automatically installed with apt-mark auto

[tedski]

I like it! After my previous post, I started to dig through apt a bit and a lot of what you scope is already there. I'll get to work on this!! Thanks for the input.

[tweenk]

Addendum: the parsing in step 1 should also consider the Pre-Depends line - it contains dependencies which must be configured before installation. For testing, you can use any package with unsatisfied dependencies from the standard repos, obtained with apt-get download package-name - this will download the .deb file to the current directory.

[tedski]

I think these edits would resolve this, but I want to get someone in IRC to talk about it first (and freenode is under attack... again).

https://github.com/tedski/salt/compare/10107

Instead of worrying about Pre-Depends and Depends differences and having to write a control file parser -- parsing the Depends line would be a bit of work -- I just use the apt module. It's not Priority: required, but the majority of working systems have it installed. This was the easiest way to add this feature without changing a bunch of already-functioning things -- in my opinion at least.

Comments welcome.

[tweenk]

This looks fairly good.

After the first call to cmd.run, I would add a second call with cmd = ['apt-mark', 'auto'] + missing_deps. This way if the installed .deb package is later removed, the automatically installed dependencies can also be removed via apt-get autoremove. Ideally, this should be done after the deb package is installed.

[tweenk]

Also, you probably want to pass kwargs to _resolve_deps().

[tedski]

Awesome, good catches. Updating now.

[tedski]

I've updated that branch, refresh the comparison link above. I added the marking as auto and some better error handling.

[tweenk]

Line 1552, for pkg_file in pkg_params: - that should be for pkg_file in pkgs:. Otherwise it looks great.

[tedski]

Right on. Let me take care of testing now.

[tedski]

I'm having some issues with lint and I think they can be safely ignored. However, I'd like to check with the dev team before I use # pylint: disable=E0611 and freenode continues to be under ddos.

[cachedout]

Ted, be sure that you're using the pylintrc provided with the Salt
distribution for doing your linting instead of the pylint defaults.

There are a couple of comments to disable pylint here and there throughout
the code-base, but they're fairly rare.

-mp

On Mon, Feb 3, 2014 at 8:00 AM, Ted Strzalkowski
[email protected]:

I'm having some issues with lint and I think they can be safely ignored.
However, I'd like to check with the dev team before I use # pylint:
disable=E0611 and freenode continues to be under ddos.

Reply to this email directly or view it on GitHubhttps://github.com//issues/10107#issuecomment-33962101
.

[tedski]

@cachedout the lint issues were raised by Jenkins here: http://jenkins.saltstack.com/job/salt-pr-lint/825/violations/file/salt/modules/apt.py/

I'd appreciate any input you can give on if I'm doing things correctly with the intra-module import there.

[tweenk]

I think the problem is that you are trying to import a module which has the same name as the current module. If this code works as expected, it might be one of those rare cases where disabling pylint is the correct thing to do.

[tedski]

Yeah. I talked with @s0undt3ch on IRC and he guided me in the right direction. I'll need to rename salt.modules.apt.py to handle this so I can properly import the dist-package apt.

[tedski]

Per @s0undt3ch 's advice, I've renamed the apt module to apt_mod (it's renamed via virtual dunder) to allow me to import python-apt.

[tweenk]

Wouldn't it be more descriptive to rename to apt_pkg? Salt already has modules such as linux_sysctl, mac_sysctl, pw_user, mac_user, solaris_user and so on - the pattern seems to be platform_functionality.

[tedski]

Once again, thanks for the good idea, @tweenk

I rebased my fork and it's now apt_pkg.

[tedski]

It hit me that I'm not covering the case where there are no missing dependencies. #10183 fixes that. I also renamed apt_pkg to aptpkg for continuity's sake (see: yumpkg).

[cachedout]

@tedski I see some of your PRs got merged. Do you still want this issue open or shall we go ahead and close it?

[tedski]

I think we've satisfied the requests in this issue. We should close this one. Any other features can be request in a future issue and prioritized accordingly.

[cachedout]

Super! Thanks for all your help here, @tedski. Much appreciated.

[moloney]

It appears that this does not handle dependencies recursively, just the immediate dependencies of the .deb file.

[tweenk]

@moloney - it does. The .deb file has dependencies satisfied from the repositories by apt-get. There is no need to recursively evaluate dependencies since apt-get will do that for you.