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

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

Merged
merged 1 commit into from
May 11, 2024

Conversation

4nd3r
Copy link
Member

@4nd3r 4nd3r commented May 10, 2024

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.

@4nd3r 4nd3r requested a review from sideeffect42 May 10, 2024 11:14
@sideeffect42
Copy link
Member

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
Copy link
Member Author

4nd3r commented May 11, 2024

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

Want to dig your memory?

@sideeffect42
Copy link
Member

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

Want to dig your memory?

Haha, yes 😆
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
Copy link
Member Author

4nd3r commented May 11, 2024

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.

@4nd3r 4nd3r merged commit 2a532ba into main May 11, 2024
@4nd3r 4nd3r deleted the 4nd3r/__package_apt/rprovides_present branch May 11, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants