Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add support for RPM ndb databases #159

Merged
merged 4 commits into from
May 15, 2023
Merged

feat: add support for RPM ndb databases #159

merged 4 commits into from
May 15, 2023

Conversation

RubixDev
Copy link
Contributor

@RubixDev RubixDev commented May 14, 2023

This closes #154.

As mentioned by @CarterLi in #154 (comment), fastfetch uses librpm to count RPM packages.
There are Rust bindings to librpm (librpm-sys), however depending on that crate proved to introduce many build complications, and, more importantly, would require librpm to be installed on every system that macchina, or other binaries depending on libmacchina, are run on.

So I instead created a new small crate called rpm-pkg-count which directly links to librpm and uses fastfetch's method of retrieving the package count. To mitigate the second issue I make use of the libloading crate, which allows me to try and load the librpm library at runtime and simply return None if it is not found.

The way I currently added rpm-pkg-count to libmacchina is behind a new feature called rpm. With the feature disabled (as it is by default), libmacchina continues to use the sqlite solution. If you wish, I could also replace the current sqlite implementation with the new one. Note, however, that the new method requires librpm to be installed on systems for all users that want rpm package count support, even on distros that use the sqlite backend, unlike the current manual sqlite solution (this may need to be documented somewhere).

Edit: I removed the rpm feature again and now libmacchina will always try the existing sqlite method first, as that seems to be faster than using librpm (see #159 (comment)). Only if that fails, librpm will be used.

@Gobidev
Copy link
Contributor

Gobidev commented May 14, 2023

It looks like your current implementation only runs the detection using your crate when the rpm feature is enabled.
It would be better if count_rpm() also tries the previous detection method using sqlite when this detection fails to also provide support for systems that work with sqlite and do not have librpm installed.

Edit: I did some testing and found that if both methods work for a system, the sqlite implementation is noticeably faster than using librpm, so we should probably always try that first and if it fails also try the librpm method.

@RubixDev
Copy link
Contributor Author

The sqlite implementation is now always run first and only if it fails librpm is used. imo the extra feature isn't really required at this point.

@Gobidev
Copy link
Contributor

Gobidev commented May 14, 2023

imo the extra feature isn't really required at this point.

I agree, the only reason to disable it now would be to reduce the amount of dependencies, which was also not done for sqlite.

src/linux/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Gobidev Gobidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Works as expected on Fedora and openSUSE Tumbleweed

@grtcdr
Copy link
Member

grtcdr commented May 15, 2023

Looks good to me as well.

Thanks to everyone for taking their time to review this PR and for leaving the helpful comments and remarks in order to improve it -- and thank you @RubixDev for all your effort!

@grtcdr grtcdr merged commit 238d539 into Macchina-CLI:main May 15, 2023
@RubixDev
Copy link
Contributor Author

and thank you @RubixDev for all your effort!

You're welcome!

Just one extra note. I noticed the CI build failing on two targets so I pushed an update to rpm-pkg-count (now v0.2.1) that should fix this by relying on type inference.

@Gobidev
Copy link
Contributor

Gobidev commented May 15, 2023

The builds pass with v0.2.1!

@grtcdr can you make a new release with this feature? That would help out a lot^^

We should probably also add a note somewhere that makes clear that librpm needs to be installed for RPM package count to work on certain distributions.

@grtcdr
Copy link
Member

grtcdr commented May 15, 2023

@grtcdr can you make a new release with this feature? That would help out a lot^^

Sure thing!

We should probably also add a note somewhere that makes clear that librpm needs to be installed for RPM package count to work on certain distributions.

Would you like to add that to the README under a section titled "Notes"?

grtcdr pushed a commit that referenced this pull request May 15, 2023
* feat: add support for RPM `ndb` databases

* chore: always try sqlite before librpm

* chore: remove `rpm` feature

* fix: librpm call not lazily evaluated
Rolv-Apneseth pushed a commit to Rolv-Apneseth/libmacchina that referenced this pull request May 30, 2023
* feat: add support for RPM `ndb` databases

* chore: always try sqlite before librpm

* chore: remove `rpm` feature

* fix: librpm call not lazily evaluated
grtcdr added a commit that referenced this pull request Sep 28, 2024
* Fix Armv7 build (#158)

* Add armv7 build target

* Fix mismatched types for pointer width 32

* Use `sh` instead of `python` for `which` assertion

* Replace mach with mach2 (#157)

* feat: add support for RPM `ndb` databases (#159)

* feat: add support for RPM `ndb` databases

* chore: always try sqlite before librpm

* chore: remove `rpm` feature

* fix: librpm call not lazily evaluated

* Add `librpm` dependency note to README (#160)

* Update the changelog

* Bump version to 7.1.0

* Fix i686 build (#162)

closes #161

* Add Sonoma to macos_version_to_name (#163)

* BREAKING CHANGE: Change BatteryReadout::health return value to u8

There's no particular reason why one should allocate 64 bits for a
value that can only be <= 100.

As a bonus, ceil() the return value before finally casting to u8.

* Update BatteryReadout::health function signature

for other operating systems besides Linux.

* Added general detection for wayland compositors (#164)

* Upgrade dependencies to their latest versions

* Update changelog

Make some formatting changes as well.

* Add missing generic argument

sqlite's changed a bit between releases.

* Bump version to 7.2.0

* Add missing second generic argument to sqlite read() call

* Remove unneeded argument to unistd::gethostname function call

* Remove unused variable

* Refactor obsolete find_ifa function

* Bump version to 7.2.1

* Bump vergen version

* Use gitcl feature of vergen

This depends on the git binary, more ubiquituous than the libgit2 bindings, so
it should technically work on every platform we support.

* Refactor the old vergen interface

* Add new entry to the changelog

* Update version to 7.2.2

* Replace flatten() calls with map_while(Result::ok)

* Fix Readouts struct's network field type

Closes: #168

* Improve CI workflow (#169)

* Replace discontinued actions-rs

* Split cargo fmt and clippy into their own CI job

* Faster package count on Alpine Linux (#170)

* Bump version to 7.2.3

* added macos 15 version name (#171)

https://www.apple.com/newsroom/2024/06/macos-sequoia-takes-productivity-and-intelligence-on-mac-to-new-heights/

* Removed panic if local gpu db is not able to be read (#173)

* Add support for the Nix package manager (#172)

Added support for the Nix package manager using Nix' SQLite database.

* Bump version and update changelog

* linux: Safely exit when homebrew is not installed

* Improve linuxbrew keepme safeguard

* Remove unused import

* Bump version to 7.3.1

* Allow disk_space function to accept a path argument (#156)

BREAKING CHANGE: allow disk_space function to accept a path argument
- Bump version and update changelog
- Change disk_space path argument to be of type &Path and check path exists in shared::disk_space
- Add missing import for openwrt

---------

Co-authored-by: Adrian Groh <50576978+Gobidev@users.noreply.github.com>
Co-authored-by: Silas Groh <silas.groh@t-online.de>
Co-authored-by: grtcdr <tahaaziz.benali@esprit.tn>
Co-authored-by: Rex Ng <rex.ky.ng@gmail.com>
Co-authored-by: Absolpega <106615943+Absolpega@users.noreply.github.com>
Co-authored-by: grtcdr <ba.tahaaziz@gmail.com>
Co-authored-by: Matthias Baer <matthiasbaer@bluewin.ch>
Co-authored-by: Rex Ng <timescam@duck.com>
Co-authored-by: coolGi <57488297+coolGi2007@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support for ndb (and bdb) RPM backend(s)
4 participants