void-linux / xbps

The X Binary Package System (XBPS)
https://voidlinux.org/xbps/
Other
821 stars 124 forks source link

xbps-remove removes directories it shouldn't #317

Open ericonr opened 4 years ago

ericonr commented 4 years ago

Steps to reproduce

$ xbps-install hplip # /usr/share/cups/model dir is created by make_dirs trigger
$ xbps-install gutenprint # adds files into /usr/share/cups/model/gutenprint
$ xbps-remove gutenprint # removes /usr/share/cups/model completely
$ hp-setup # now crashes because it can't find /usr/share/cups/model

Proposed fix

Track directories from make_dirs in XBPS, as a property that is checked when removing a package and pruning directories. Possibly even implement make_dirs inside of XBPS instead of as a trigger, which seems like a better place for it.

Related to https://github.com/void-linux/void-packages/issues/20689

Ping @ahesford @bobertlo

ericonr commented 4 years ago

This is currently worked around by a few templates that add an .empty file in the created directory, which isn't a very clean solution.

ahesford commented 4 years ago

grafana touches /var/lib/grafana/.empty to make sure the directory survives at least as long as the package is installed. I just modified hplip to touch /usr/share/cups/model/.hplip to avoid the scenario in the above example. I'm not sure if there are other instances of this kind of workaround.

Duncaen commented 4 years ago

A native make_dirs would be recorded in files.plist to avoid separate code paths for packages owning normal directories and empty directories created with make_dirs. There is also no real point in adding this data to the repository index other than working around the real problem.

The problem is that xbps stores the files a package owns in separate plist files, making it inefficient to read them all for every single installed package (so it just tries to delete every directory it might have created). This is not only a problem with checking if a directory is obsolete on removal, but also more importantly the reason file conflicts between packages in the transaction and packages already installed can't be detected before starting the transaction and/or stating every single file.

The only real solution is to replace per-package with a local and possibly a repository files database which can be opened at once and potentially optimize lookups and size by maintaining a tree and compress the file on disk.

ericonr commented 4 years ago

A native make_dirs would be recorded in files.plist to avoid separate code paths for packages owning normal directories and empty directories created with make_dirs.

I see. One potential issue with a native make_dirs, now that I think about it, is for directories owned by specific users, since it's also a trigger that's responsible for creating users. Perhaps simply storing make_dirs to avoid deleting the wrong directories is a good enough solution, but the directory creation can be left to the trigger.

So this is related to #287, too.

ericonr commented 4 years ago

This would also require XBPS to perform the groups/users/accounts triggers itself, instead of depending on xbps-triggers for it.