Fix API breakage relating to lifetime tweaks and bitflags attributes

[SapphirusBeryl]

When lifetime annotations were removed from the Package struct, this change necessitated borrowing in code downstream. For example, alpm.rs also returns &Package objects via the pkg function in the Db struct.

This breaks trans_remove_pkg because the function signature only accepts owned Package objects, which makes it seemingly impossible to use.

Further, the update to bitflags also incurred some breakage downstream in my code with regards to the ergonomics of handling TransFlag bitflags attributes in the alpm.rs API. Without the Copy, Clone attributes derived, the only other option is to convert bitflags into an integer with the bits function, and then parse it back into a unique, owned TransFlag object.

Perhaps modifying trans_init to accept a borrowed TransFlag could also be done?

Herein I've included an initial set of patches to fix both of these issues in the interim.

[Morganamilo]

You're right the signature should be &Package. However IntoPkgAdd is fine is as and only applies to add.

A loaded package free's it's memory on drop. But if you pass a package to trans_add_pkg alpm internally takes over the memory management of it so we make sure it's we don't drop it when it's successfully added.

This doesn't apply for remove because you can only remove a package from the local database. Removing a LoadedPackage doesn't make sense. This is why it only takes a Package.

[Morganamilo]

Also you need to run cargo fmt for the CI to pass.

But the core change of using &Package and the bitflags are correct.

[SapphirusBeryl]

You're right the signature should be &Package. However IntoPkgAdd is fine is as and only applies to add.

A loaded package free's it's memory on drop. But if you pass a package to trans_add_pkg alpm internally takes over the memory management of it so we make sure it's we don't drop it when it's successfully added.

This doesn't apply for remove because you can only remove a package from the local database. Removing a LoadedPackage doesn't make sense. This is why it only takes a Package.

From my understanding of this, mem::forget ensures that the pointer doesn't get inadvertently dropped with the &Package object on the Rust side. Since the lifetime annotations have been removed, it's impossible now to pass ownership of the object in safely, since the object now needs to be borrowed. Rust cannot take what alpm does or doesn't do into consideration here, and it may be possible for a use after free to occur.

Even though, it might be fine, perhaps it'd be better to err on the side of caution in this regard?

See: https://doc.rust-lang.org/stable/alloc/vec/struct.Vec.html#method.as_ptr

[Morganamilo]

the Package type come from the alpm database and the memory is already managed by alpm. It has no drop Impl. This drop is only relevant for LoadedPackage which is a package directly loaded from a tarball.

This is why IntoPkgAdd is implemented for &Package but only an owned LoadedPackage.

[SapphirusBeryl]

the Package type come from the alpm database and the memory is already managed by alpm. It has no drop Impl. This drop is only relevant for LoadedPackage which is a package directly loaded from a tarball.

This is why IntoPkgAdd is implemented for &Package but only an owned LoadedPackage.

Fair enough. I'm still very much learning about the more lower-level stuff, so I just had to clarify.

* I've moved the branch back with the formatting fixed to pass CI.

[Morganamilo]

You still need to revert the change to PackageAdd trait. trans_remove_pkg should take &Package instead of Package but everything else on that side should stay as is.

[SapphirusBeryl]

You still need to revert the change to PackageAdd trait. trans_remove_pkg should take &Package instead of Package but everything else on that side should stay as is.

I've force pushed the head back to #e44d7b4 to remove those additions.