skonfig / base

explorer and types for general use
GNU General Public License v3.0
4 stars 4 forks source link

__package_apt: only check rprovides if requested state is "present" #104

Closed 4nd3r closed 6 months ago

4nd3r commented 6 months ago

When trying to remove already absent package which has reverse provides defined, and reverse provide is installed, then that gets removed.

For example with vim:

$ dpkg -l | awk '/vim/ {print $2}'
vim-common
vim-gtk3
vim-gui-common
vim-runtime
$ echo __package_apt vim --state absent | skonfig localhost -i - -n 
INFO: localhost: Starting dry run
INFO: localhost: Processing __package_apt/vim
INFO: localhost: Finished dry run in 1.44 seconds
$ skonfig -d localhost | grep code-remote
object/__package_apt/vim/.cdist-r4dzn92m/code-remote: DEBIAN_FRONTEND=noninteractive apt-get --quiet --yes -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" remove  'vim-gtk3'

Proposed change fixes this.

sideeffect42 commented 6 months ago

First of all, I think this change is something we want. The behaviour shown in the OP looks wrong to me, too.

I think we should use $name here instead of $name_is.


I tried to wrap my head around this Reverse Provides logic and in my head it only makes sense for virtual packages like editor. But then __package_apt editor does not make any sense either, because APT does not select an implementation on its own (--state present) and virtual packages cannot be removed (--state absent).

In other words: what is the reverse provides logic useful for? do we even want it at all?

4nd3r commented 6 months ago

@sideeffect42 you had some fun with it here: https://github.com/ungleich/cdist/pull/781

Want to dig your memory?

sideeffect42 commented 6 months ago

@sideeffect42 you had some fun with it here: ungleich/cdist#781

Want to dig your memory?

Haha, yes :laughing: If you look at the diff closely you'll see that the logic is unchanged really, I just did some reformatting and tried to document $rprovides.

With $rprovides you could use e.g. __package_apt editor --state present to ensure any editor is installed, but what I hadn't noticed back then is that if no editor is installed it will error out.

Is this useful?

If we conclude it is not useful, I think we'd have to add another parameter --virtual-choice (or something) where the user can pick which package to install if none of the options of a virtual package are installed. But then I wonder what would be the desired outcome of __package_apt some_virtual --state absent --virtual-choice x?

4nd3r commented 6 months ago

Okay, let's merge it for now, since this fixes my problem, but in general, if this "feature" starts to limit us any way, I see no problem removing it.