cassava / repoctl

Make it easy to manage your local Arch Linux repository.
MIT License
125 stars 14 forks source link

Allow removing packages from DB without deleting old cache files #30

Closed maximbaz closed 6 years ago

maximbaz commented 6 years ago

I'm trying to bring repoctl into my workflow, and I noticed that repoctl update is incompatible with paccache - paccache intentionally keeps 3 latest package versions in cache, but repoctl update purges everything except the newest one.

I'd like to use repoctl update to cleanup packages that repoctl status marked as removal, but do nothing with those that are marked as obsolete(N). Ideally I'd also prefer to make repoctl status also ignore obsolete packages, because I use paccache and I know that they are not obsolete.

Reference: https://github.com/AladW/aurutils/issues/208#issuecomment-369037890

cassava commented 6 years ago

Hi maximbaz. That means that you manage the obsolete packages yourself, but repoctl should remove packages no longer in the database?

cassava commented 6 years ago

I would make the following suggestion: repoctl allows you to define a backup directory where packages are moved to. Normally, this path is backup, relative to the repository. If you set the backup option to true and the backup_directory to . or an empty string, then repoctl will leave obsolete files alone. It will also print caching: package_file\n in debug mode, but otherwise nothing. Does this sound like it would solve the problem?

maximbaz commented 6 years ago

Hi maximbaz. That means that you manage the obsolete packages yourself, but repoctl should remove packages no longer in the database?

That is exactly right, I manage cache files myself via paccache, but I want to use repoctl to manage package database - if there are no cache files, clean up my database, if there are new cache files, add them to my database.


I tried the workaround that you suggested with both backup_directory set to . and an empty string, but for some reason it doesn't work for me:

❯ repoctl version
repoctl version 0.17 (31 January, 2018)
Copyright 2016-2018, Ben Morgan <neembi@gmail.com>

You may find repoctl on the Internet at
    https://github.com/cassava/repoctl
Please report any bugs you may encounter.

The source code of repoctl is licensed under the MIT license.

Current configuration:
    repo        = "/var/cache/pacman/maximbaz-aur/maximbaz-aur.db.tar"
    add_params  = []
    rm_params   = []
    ignore_aur  = []
    backup      = true
    backup_dir  = ""
    interactive = true
    columnate   = false
    color       = "auto"
    quiet       = false
    debug       = false
    pre_action  = ""
    post_action = ""

❯ repoctl status
On repo maximbaz-aur

        alacritty-git: obsolete(3)

❯ pwd
/var/cache/pacman

❯ ll maximbaz-aur/alacritty-git-*
.rw-r--r-- 4.9M root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.729.g59b561b-1-x86_64.pkg.tar
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.729.g59b561b-1-x86_64.pkg.tar.sig
.rw-r--r-- 4.9M root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.730.gb82622e-1-x86_64.pkg.tar
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.730.gb82622e-1-x86_64.pkg.tar.sig
.rw-r--r-- 4.9M root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.731.g53a5a96-1-x86_64.pkg.tar
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.731.g53a5a96-1-x86_64.pkg.tar.sig
.rw-r--r-- 5.5M root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.736.g03f9e0c-1-x86_64.pkg.tar
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.736.g03f9e0c-1-x86_64.pkg.tar.sig

❯ repoctl update
backing up: /var/cache/pacman/maximbaz-aur/alacritty-git-0.1.0.731.g53a5a96-1-x86_64.pkg.tar
backing up: /var/cache/pacman/maximbaz-aur/alacritty-git-0.1.0.730.gb82622e-1-x86_64.pkg.tar
backing up: /var/cache/pacman/maximbaz-aur/alacritty-git-0.1.0.729.g59b561b-1-x86_64.pkg.tar

❯ ll maximbaz-aur/alacritty-git-*
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.729.g59b561b-1-x86_64.pkg.tar.sig
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.730.gb82622e-1-x86_64.pkg.tar.sig
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.731.g53a5a96-1-x86_64.pkg.tar.sig
.rw-r--r-- 5.5M root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.736.g03f9e0c-1-x86_64.pkg.tar
.rw-r--r--  566 root 28 Feb 11:07 maximbaz-aur/alacritty-git-0.1.0.736.g03f9e0c-1-x86_64.pkg.tar.sig

But to be fair, to have a better integration with tools like pacman and aurutils I would much prefer having a flag for repoctl status and repoctl update that ignores obsolete packages than using such a workaround.

pacman provides paccache with the sole purpose to carefully manage cached files, and it is very powerful, more powerful than your implementation - e.g. it allows configuring the number of most recent versions to keep, it allows several ways of printing obsolete cache files, it correctly handles signature files.

If I were you I would probably not even implement the code that handles obsolete cache files at all, but I guess the functionality is already there and it's too late for such feedback 😃 But at least for those users who make use of more powerful paccache I'd make a flag in repoctl so that it doesn't interfere with it.

cassava commented 6 years ago

Sorry, I was proposing a solution that I hadn't implemented yet.

cassava commented 6 years ago

I'm going to have to have a look at paccache, haven't come across it till now.

maximbaz commented 6 years ago

Ah I see, I didn't catch that it isn't implemented yet 🙂

Since something has to be implemented anyway, my vote is even stronger for a new flag that ignores manipulation of cache files 🙂

Let me know what you think about all of this once you get familiar with paccache 👍

cassava commented 6 years ago

I'll do that. In the mean time you could glance at a few changes I made in the devel branch.

maximbaz commented 6 years ago

Yup, already tested those - the files are not being removed 👍 But seeing obsolete in repoctl status and a bunch of backing up: xxx in repoctl update is a bit annoying, thus created #32 for you to look at 😉.

cassava commented 6 years ago

Hi @maximbaz, could you have a look at current devel state. In particular, I prefer that the less invasive changes to the code (don't quite like passing keepCacheFiles bool around) and the fact that it picks up your "default". For me, that's the most important thing -- if you treat obsolete packages as cache files, then repoctl should treat them as such across the board, instead of requiring you to remember to pass a flag everytime you run update. The one time you forget you will curse me, and where would that get me? ;-) The one thing I imagine might be annoying is that it prints cached: filename, but that could be silenced. When running repoctl status you can add the -c or --cached flag, this then shows how many packages are cached. It just renames obsolete(3) to cached(3), actually. But by default it doesn't show them.

cassava commented 6 years ago

What should be done when using repoctl rm pkgname? Should it delete the cache files, or leave them alone? Currently it had used the same behavior as backup specified. But if caching is enabled, then that results in:

$ repoctl rm cantata-git
removing package from database: cantata-git
caching: /home/ben/mnt/cantata-git-2.0.50.r6190.5bbe73d-1-x86_64.pkg.tar.xz
$ repoctl update               
adding package to database: /home/ben/mnt/cantata-git-2.0.50.r6190.5bbe73d-1-x86_64.pkg.tar.xz

A bit pointless, don't you think.

This could be rectified by running:

$ repoctl rm --backup=false cantata-git
removing package from database: cantata-git
deleting: /home/ben/mnt/cantata-git-2.0.50.r6190.5bbe73d-1-x86_64.pkg.tar.xz

Which, by the way, also works for update.

So maybe specifying --backup=false when you want to delete something is acceptable, or maybe it isn't.

maximbaz commented 6 years ago

Thanks, just tested!

The fact that repoctl status doesn't print cached files unless -c is specified is actually even better that what I proposed, definitely like it 👍

Printing cached: filename is indeed annoying, and actually just meaningless - it is not "caching" anything, it just "skips" those files.

Can we remove the printf alltogether (or hide it under the debug level output)?

Other than that, it is very very good!

maximbaz commented 6 years ago

What should be done when using repoctl rm pkgname

Well, specifying --backup=false does sound acceptable, because who will see caching in the first place - those people, who intentionally configured their backup dir to be an empty string, and in that case they should be okay "disabling backup" if they want to cleanup cache files.

cassava commented 6 years ago

I agree. When removing a file, you want to see the cached: filename message, but then it will also be shown when running repoctl update. This is unfortunate. If I hide it, it will also be hidden when removing. Possiblities:

Currently, I am leaning towards the last option, even though it's not entirely optimal.

maximbaz commented 6 years ago

Actually, do you really want to print cached: filename when removing a file though? Maybe I don't fully understand what's happening there, but to me printing cached: filename sounds as if we are printing not doing anything to: filename - why even print this then?

If you do want to print cached: filename on rm, then I guess I would also pick the last option 👍

cassava commented 6 years ago

I implemented the last option on the devel branch. Since normally we only print something when we actually do something, you're right, it doesn't really make sense to print it. I guess it's useful as a reminder. So I'm printing a warning when removing a package from database but not deleting the file, as I can't imagine why anyone would do this => it's probably a mistake then, which is why the warning is useful.

maximbaz commented 6 years ago

Awesome, thanks for the quick implementation 👍 I'm fine with closing the ticket, unless you want to keep it around until you publish a next release.

cassava commented 6 years ago

Sure! Thanks for your involvement!

maximbaz commented 6 years ago

P.S. Consider also documenting somewhere that BackupDir can now accept empty string and this is useful if you manage cached files using different tools, such as pacman -Sc or paccache -r 👍

cassava commented 6 years ago

Already done. :-D