Handle dnf immutable mutable datatypes

[maxamillion]

Signed-off-by: Adam Miller admiller@redhat.com

SUMMARY

In DNF < 3.0 are lists, and modifying them works
In DNF >= 3.0 < 3.6 are lists, but modifying them doesn't work
In DNF >= 3.6 have been turned into tuples, to communicate that
modifying them doesn't work

Further explanation of this is available via Adam Williamson (@AdamWill) from the Fedora QA Team.

https://www.happyassassin.net/2018/06/27/adams-debugging-adventures-the-immutable-mutable-object/

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

dnf

ANSIBLE VERSION

$ ansible --version
ansible 2.8.0.dev0 (modules/dnf 79edc96ac9) last updated 2018/09/26 12:24:29 (GMT -500)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/admiller/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/admiller/src/dev/ansible/lib/ansible
  executable location = /home/admiller/src/dev/ansible/bin/ansible
  python version = 2.7.15 (default, May 16 2018, 17:50:09) [GCC 8.1.1 20180502 (Red Hat 8.1.1-1)]

[abadger]

+1 to this.

[maxamillion]

rebuild_merge

[AdamWill]

well, I have a bit more for you.

exclude is kind of an old name for excludepkgs, and excludepkgs is - since rpm-software-management/dnf@74ef3ff - one of these weird things DNF has called an OptionStringListAppend (now, it used to be ListAppendOption in 2.x). What that means is that 'setting' it results in the value you set getting appended. Watch:

>>> import dnf.base
>>> base = dnf.base.Base()
>>> base.conf.exclude
[]
>>> base.conf.exclude = ['foo']
>>> base.conf.exclude
['foo']
>>> base.conf.exclude = ['bar']
>>> base.conf.exclude
['foo', 'bar']
>>> 

(yes, this is crazy). For any DNF since that change, you can actually just do this:

if self.exclude:
    conf.exclude = self.exclude

and that will do what you want, assuming self.exclude is a list - it will append the values from self.exclude to the conf.exclude config option. I've tested on DNF 2.7.5 from Fedora 28, DNF 3.5.1, and DNF 3.6. If you want to be compatible with DNF versions from before that ListAppendOption commit, you'd have to either just add a tuple (see below) and not care about the duplication that ensues, or do something a bit fancier.

disable_excludes isn't one of those crazyballs things, so we really do want to append it. I found a simpler way to do that which seems to work, again, across at least 2.7.5, 3.5.1 and DNF 3.6:

if self.disable_excludes:
    conf.disable_excludes += (self.disable_excludes,)

Haven't tested it with a DNF older than 2.7.5, though.

En passant, this seems kinda like a suboptimal way to offer this setting? From the DNF command line you could disable excludes for multiple repos, I think, because it combines when you pass the same arg multiple times:

dnf --disableexcludes=repo1 --disableexcludes=repo2 --disableexcludes=repo3 ...

and I think that'd wind up setting the base object's conf.disable_excludes to ('repo1', 'repo2', 'repo3'). But if I'm reading it right, that's not possible with the ansible interface, right? You only allow it to be set once, to one of three values. So you can only disable excludes from main or all or somerepo, for any given ansible dnf operation...

[AdamWill]

conf.foo += ('bar',) seems to work at least back to 2dbba7a99a6fdddd0136b804a249dcbcba9c69b9 , which is about three rewrites of the config engine ago, in 2004. So I think we're probably good with that.

[maxamillion]

@AdamWill we still test dnf all the way back to Fedora 24 so I'm alright leaving it with the case that works for all known versions even if it is slightly less elegant. I really appreciate all the information you've shared, it's been valuable beyond words. 👍

[AdamWill]

Well, just using the conf.foo += ('bar',) form ought to work. But it's not too important. Note that for the exclude case, the code you merged will be causing duplication, though in practice the duplication probably doesn't hurt anything.

Stay tuned, though, as more fun is coming...

[maxamillion]

@AdamWill oh fair point on the duplication, I'll make a new PR to update.

[AdamWill]

I may try and talk 'em into throwing out the whole duplication mess upstream, too. It's really awful. I think any fix would unfortunately have to lose compatibility, though :/