feat: Add trans_add_pkg_by_name and trans_remove_pkg_by_name

[pfeifferj]

These convenience methods perform database lookups internally, avoiding borrow checker conflicts when holding &Package references across transaction operations like trans_prepare().
trans_remove_pkg_by_name looks up package in localdb; trans_add_pkg_by_name searches all registered sync databases.

For extra context, I was working on adding orphan cleanup features to cockpit-pacman and notice that when removing packages, the typical pattern hits a borrow checker conflict like e.g.:

let orphans: Vec<&Package> = handle.localdb().pkgs()
    .iter()
    .filter(|p| /* ... */)
    .collect();

handle.trans_init(TransFlag::empty())?;
for pkg in &orphans {
    handle.trans_remove_pkg(*pkg)?;
}
handle.trans_prepare()?;  // ERROR: cannot borrow handle mutably

The &Package references borrow from the handle, preventing trans_prepare(&mut self). Users must work around this by collecting package names as owned strings first.

These new methods encapsulate the lookup, so no &Package escapes:

let names: Vec<String> = orphans.iter().map(|p| p.name().to_string()).collect();

handle.trans_init(TransFlag::empty())?;
for name in &names {
    handle.trans_remove_pkg_by_name(name.as_str())?;
}
handle.trans_prepare()?;  // Works

I thought it would make sense to upstream this, maybe. Looking forward to hearing your thoughts.

[Morganamilo]

I cant say exactly why you're running into this issue before seeing more of your code and seeing how you've structured the project generally. I don't think this PR is the way to work around this. Rather the code should be structured in a way that avoids this all together.

I've written some example code that walks through why it works this way and how the situation can be avoided. If your code is substantially more complicated and it's difficult to avoid the borrow checker, I'm happy to take a look and see if there's a nice way to implement it.

use alpm::*;

fn main() -> Result<()> {
    // initialize alpm
    let mut alpm = Alpm::new("/", "./db")?;

    // set the event call back so we can see what alpm is doing
    alpm.set_event_cb((), |e, _| {
        if let Event::PackageOperationStart(e) = e.event()
            && let PackageOperation::Remove(e) = e.operation()
        {
            println!("  removing {}", e.name())
        }
    });

    // select the first 10 packages that are not required by anything
    let orphans: Vec<&Package> = alpm
        .localdb()
        .pkgs()
        .iter()
        .filter(|p| p.required_by().is_empty())
        .take(10)
        .collect();

    // initialize a transaction
    // this does not require &mut self as creating a new transaction just allocates an internal
    // struct. and thus can't effect anything we may currently have a reference to.
    alpm.trans_init(TransFlag::DB_ONLY | TransFlag::NO_HOOKS | TransFlag::NO_SCRIPTLET)?;

    // add our orphans to the transaction
    orphans.iter().try_for_each(|p| alpm.trans_remove_pkg(p))?;

    // prepare the transaction.
    //
    // This function must take &mut self as it frees alpm->trans->add when it runs.
    // it's possible users may have taken a reference to some of the values in this list
    // so we need to make sure no references to any packages exist when we call this.
    //
    // however once we've given our packages to trans_{add, remove} we no longer hold any references
    // to them so this just works (as long as we don't try to use `orphans` after this call)
    alpm.trans_prepare()?;

    // this would be a compile error.
    // print!("{}", orphans.len());

    println!("packages to remove...");

    // we could go and re fetch our packages from the local db now that we've called
    // trans_prepare().
    // however we can also get them from the transaction itself
    let orphans = alpm.trans_remove();
    orphans.into_iter().for_each(|p| println!("  {}", p.name()));

    println!("\ncomitting transaction...");

    // this takes &mut self too
    // we can't reference `orphans` again after this point, alpm deallocates them during the
    // transaction
    alpm.trans_commit()?;
    Ok(())
}

morganamilo@starlight ~git/alpmtest % cargo run
packages to remove...
  fwupd
  fuseiso
  fuse-zip
  fprintd
  fossil
  flex
  fish
  firefox
  figlet
  ffmpeg4.4

comitting transaction...
  removing fwupd
  removing fuseiso
  removing fuse-zip
  removing fprintd
  removing fossil
  removing flex
  removing fish
  removing firefox
  removing figlet
  removing ffmpeg4.4

---

alpm.rs aims to be 1:1 with alpm. If the function doesn't exist in libalpm it shouldn't go in here. The alpm-utils crate is where all the convenience functions go.

The implementation in the PR lacks the ability to choose what repo you actually get the package from. This is something that any user should care about. alpm-utils already has the Target type for this, so integrating with that would probably fit quite well.

I'm not convinced that these functions would be useful for borrow checker reasons. Though they may be useful just as general convenience functions.

[pfeifferj]

for posterity, here's the context from a side conversation we had:

my TransactionGuard abstraction[1] takes &mut Alpm, which holds the mutable borrow for the guard's entire lifetime. meanwhile, alpm.rs intentionally designed trans_init to take &self (not &mut self) specifically to sidestep this kind of borrow conflict. when creating a transaction it just allocates an internal struct and doesn't touch existing refs.
so basically i built an abstraction that fights the api's actual design.

thanku for the detailed breakdown + example code

[1] https://github.com/pfeifferj/cockpit-pacman/blob/main/backend/src/alpm/transaction.rs