Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
For now I've moved everything under a GitHub organization: https://github.com/rustrpm The Rust bindings to librpm are now located at: https://github.com/rustrpm/librpm-rs -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-396061666___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@dmach would you consider creating a repository under https://github.com/rpm-software-management/ where I can push my Rust bindings? Alternatively I could open a PR to https://github.com/rpm-software-management/rpm containing them Or if you'd like to wait until they're a bit more mature, I can keep working on them in my own personal repo -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-385393688___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@tarcieri If you give @dmach access, he can move it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-383634458___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
I think it's a bit hard to move things that way (without making me an admin), but given it's a brand new repo I can either: 1) Just push all the same code to a newly created repo under `rpm-software-management` or 2) Give someone admin access to `librpm-rs` so they can move it -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-383634217___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@dmach Can we get @tarcieri the ability to move`librpm-rs` to the rpm-software-management organization? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-383629787___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
I've renamed the project to librpm.rs: https://crates.io/crates/librpm I've extracted it into its own Git repo here, with everything licensed as LGPLv2.1+: https://github.com/tarcieri/librpm-rs I've also changed the binding to move everything relating to match iterators and headers into an `internal` module which is only accessible from within the `librpm` crate itself: https://github.com/tarcieri/librpm-rs/tree/master/src/internal The database iterator now works over "owned" data which is copied to Rust's heap, and exposes much higher level information about packages: https://github.com/tarcieri/librpm-rs/blob/master/src/package.rs -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-383609541___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
> Alright, perhaps I can take a step back from the reference-based stuff and > take a look at creating "owned" values... although my preferred way to do > that in Rust would be for Rust to make a copy itself, as opposed to having > rpm allocate the memory and me having to call rpm again to free it. That way > Rust owns the memory and I can leverage Rust's built-in destructors. Yup, that's more or less what the Python bindings do as well: the data is converted to native presentation right after headerGet(), and once that is done you can free any allocations done by rpm to return that data (eg string arrays and extensions always allocate something), the rest is managed by Python. See https://github.com/rpm-software-management/rpm/blob/05b233ddbe3a64089001b3d912c9a9080806953e/python/header-py.c#L464 and https://github.com/rpm-software-management/rpm/blob/master/python/rpmtd-py.c for the details. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-383525199___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@tarcieri: I am not particularly fond of XML either, but using headerFormat() queryformat to export data from a header simplifies bindings for what is usually desired: masses of RO metadata about rpm packages, an entirely well understood programming technique. Note that rpm -q --xml is also WYSIWYG, and displays everything in a header, if you wish to familiarize yourself with rpm package metadata. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-382025752___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq can't say I'm super thrilled with the prospects of replacing access to an in-memory data structure with something based on XML >As for handling tag data, you probably don't want to expose rpmtd to rust >directly but convert to native representation [...] > Worse, any data addition to a header can make the data retrieved with > HEADERGET_MINMEM invalid behind your back. HEADERGET_MINMEM should really be > considered a historical leftover and an rpm internal thing at best, it's not > something you want to build anything on. Alright, perhaps I can take a step back from the reference-based stuff and take a look at creating "owned" values... although my preferred way to do that in Rust would be for Rust to make a copy itself, as opposed to having rpm allocate the memory and me having to call rpm again to free it. That way Rust owns the memory and I can leverage Rust's built-in destructors. This was my original plan when I was using a `StreamingIterator`: make `Header` and `rpmtd` access internal to the library only, and expose a real Rust `Iterator` which makes copies of the data it iterates over when exposing them to the end user of the library. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-382019238___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@tarcieri , headers exist outside database iterators and such, and you really dont want to require keeping potentially huge headers (tens of megabytes possibly) in memory just in order to remember, say, a name of a package. Worse, any data addition to a header can make the data retrieved with HEADERGET_MINMEM invalid behind your back. HEADERGET_MINMEM should really be considered a historical leftover and an rpm internal thing at best, it's *not* something you want to build anything on. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381909280___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
There's also an alternative way to buffer all rpmlog() output. If rust has some reasonable XML support, then data can be exported from a header into whatever structure you wish rather effortlessly. E.g. try "rpm -q --xml bash". The --xml is just a queryformat disguised as a CLI option using a popt alias. Per-tag queryformats could be used. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381822461___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Overriding qva->qva_showpackage with a short routine that does str = headerFormat() is what I was recalling having implemented. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381815368___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Ah my bad: yes, the output is delivered to stout/stderr. And yes rpmcliQuery. Meanwhile, all the output for each header ends up in a single buffer and a single fprintf iirc. So it would not be hard to deliver the headerFormat output for each header through a callback. Delivery of textual info from rpmcliQuery would make bindings rather easy. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381811098___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq where is `rpmQuery` defined? (is it `rpmcliQuery`?) This is all I could find, and it wasn't immediately obvious how to read results from it: http://ftp.rpm.org/api/4.12.0.1/group__rpmcli.html#ga2dbfa3886628a79646b2c286ea5c56d9 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381684450___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Again, look at headerFormat() bindings which is kind of like asprintf(3) for headers and you can then parse the string into whatever rust types you wish within rust. I previously suggested you might want to bind the high level rpmQuery() same as the CLI rpm --query --queryformat ... command to avoid having to use a match iterator or headers (which would be used only in rpmQuery). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381645193___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
See lib/rpmtag.h: where the tags are defined. The usual type is appended in the comment. There is also rpm -vv --querytags which displays the known tags and types (if RPM4 doesn't display the types, then an RFE is appropriate) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381640962___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Garrison "... possibly with count 1". Specifically a RPM_CHAR_TYPE returns a pointer to data and count is the number of chars in that array. A CHAR scalar is returned as an array with count 1, ditto for INT* types. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381639532___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq I am not... definitely good to know as the current code is bogus. What'd be really helpful in regard to testing tag data is a tag type which corresponds to each of the tag data types, particularly arrays/vectors of the integer types. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381638452___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
(After perusing the td.rs code: perhaps I am misreading some assertions) Hmmm ... you do realize that only BIN/STRING/I18NSTRING types are scalars? Specifically, that means that CHAR and INT* types return a pointer to an array with count items. Of course BIN is an array of octets, and STRING is an array of chars: but CHAR and INT* are arrays possibly of size 1. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381632025___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@pmatilai using "owned" memory instead of borrowed, in Rust terminology, is an alternative, and one that could potentially eliminate any possible use-after-free vulnerabilities. However, building zero copy APIs on top of things like `HEADERGET_MINMEM` is something of Rust's raison d'etre, and one for which it has a number of neat tricks, like the `StreamingIterator` I was using previously. That approach would cause the compiler to reject any programs where the match iterator's `next()` method is called while there are still references sourced from the previous header are still alive. I'm not sure in what other languages something like that is even possible, especially as a compile-time guarantee. The PR I just linked goes from the very conservative memory model I was just describing to one that makes many more assumptions which I'm not entirely sure are correct. As you note I may be incorrectly handling string arrays. Perhaps there's a happy medium that more tightly bounds the lifetimes and then makes fewer assumptions. I can at least guarantee the following in the current binding based on the Rust lifetimes: - Nothing will ever outlive the global transaction set - MatchIterators acquire a global mutex and do not release it until they have been "dropped" - Headers increment the reference count with `headerLink` and do not decrement it until they are "dropped". They are `!Sync` and `!Send` (i.e. not thread safe or moveable between threads) and therefore "pinned" to the thread in which they are created (the same goes for MatchIterators) - Tag data is guaranteed to never outlive the header from which it is fetched, ensuring the header's reference count is never decremented (via `headerFree`) until there are no longer references to its tag data (this is enforced via a [Rust lifetime](https://github.com/iqlusion-io/crates/blob/master/rpmlib/src/td.rs#L11) but is a fairly similar approach to what you'd use in a GC'd language by having tag data mark the header from which it is fetched). > FWIW, I don't see a reason why the bindings could not be "hosted" in the > rpm-software-management group, it'd be only a good thing to have these things > under the same umbrella. That'd be great. I've been thinking about splitting the current code into its own repo and relicensing it as LGPL 2.1+ to move towards something like that. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381610231___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
I'd really recommend avoiding HEADERGET_MINMEM entirely because of all the hidden implications, just use HEADERGET_ALLOC always so you can trust the data isn't going to vanish or move underneath you, like n3npq said you need to manage unallocation anyway for string arrays. As for handling tag data, you probably don't want to expose rpmtd to rust directly but convert to native representation - that's what the "native" python bindings do. I started adding an actual python binding for the rpmtd object at some point and it just didn't make any sense at all. Encoding generally speaking anybody's guess, but packages built with rpm >= 4.13.0 carry RPMTAG_ENCODING with value of "utf-8" if the strings in the package were all valid utf-8. FWIW, I don't see a reason why the bindings could not be "hosted" in the rpm-software-management group, it'd be only a good thing to have these things under the same umbrella. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381572167___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
I've opened a PR which implements a bunch of @n3npq's suggestions: https://github.com/iqlusion-io/crates/pull/28/files Namely: - No more `Database` type. Instead `db.rs` is a handful of static functions for querying the database - `TransactionSet` is no-longer part of the public API (although it could be re-added in the future). Instead a mutex-guarded global transaction set is created the first time any function is invoked, and stored in a "lazy static". `rpmtsClean` is called immediately before releasing the global mutex. - I've made my `Header` type take a reference with `headerLink` (and likewise call `headerFree` on drop). This let me switch from a `StreamingIterator` to a regular Rust `Iterator` like we were discussing, which is very nice. All in all I think these changes make the API *much* more ergonomic: ### Before ```rust let mut txn = Txn::create().unwrap(); let mut db = Database::open( txn, false).unwrap(); let mut matches = db.find(Tag::NAME, "rpm-devel"); let headers = matches.next().unwrap(); ``` ### After ```rust let mut matches = rpmlib::db::find(Tag::NAME, "rpm-devel"); let headers = matches.next().unwrap(); ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381417575___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
In practice, there is usually a ref count held on a header by a higher level container like a ts or a mi. In C it's usually quick to use valgrind, or compile with -fsanitize=address to fix occasional mistakes. I don't know enough about rust tools used to check memory problems at runtime, though it seems like there's a strong attempt to prevent memory issues within the language itself. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381344505___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
There are 3-4 levels of memory usage with header tag data using MINMEM:
1) the rpmtd container with malloc/free, non-refcounted
2) (string arrays only) the array of pointers to strings, free'd on release
3) the data, protected by the header ref count
4) (if used) string pools to memoize/uniqify tag data
Simplest approach is not to use data outside of the narrow access window:
always copy.
Not using MINMEM is another approach.
(aside)
Disclaimer: I've only a little bit of experience with rpmtd memory usage, none
with string pools. I chose a different/simpler approach ("Always malloc tag
data") to solve identical issues as MINMEM and rpmtd solve in RPM4.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381343922___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq I'm working on simplifying the API and also switching from a `StreamingIterator` to a regular `Iterator` by using reference counting on header data. I had a question about this though: > The trickiest problem with header tag data and MINMEM is that the header > reference count sometimes ends up protecting header tag data, whether by > intent or by accident is arguable. What's the best way to model the lifetime of the tag data? It sounds like you're describing what I'd like to do: bound the lifetime of the header on the tag data, i.e. as long as someone holds on to a reference to tag data, they will also have to hold a reference to the header, and the header's reference count won't be decremented until they release both. Is that a good approach, or can I do reference counting on the tag data itself? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381339992___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq yeah I've been meaning to add some sort of higher level facade around `Header` which makes it easy to do the obvious things like get the package's name and description, but I wasn't quite sure what it would look like yet. @Conan-Kudo thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381275603___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@tarcieri The rpm.org/rpm4 documentation can be found here: http://ftp.rpm.org/api/4.14.0/ -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381272794___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re hiding header objects You an also hide headers in rust bindings. There are 3 places that headers become visible: 1) when read from packages in order to create a transaction This can be handled by passing *.rpm file names to rpmInstall or package names to rpmErase 2) Most usage cases for an rpmdb match iterator are interested in returning query information textually: you could pass in the queryformat and concarenate the strings from headers. Alternatively, you could bind the top level query/verify routines. 3) the transaction callback which is mostly used for pushing progress bars (you haven't worried about that: the default callback is mostly good enough) By hiding all of the information, and adding a version comparison, that is likely 90% of what users want from rpm in their own native language. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381256734___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Great! With that I can add a wrapper type for accessing it which acquires the lock, and when dropped calls `rpmtsClean()` as the last thing before it releases the lock. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381254960___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re a global rpmts There is rpmtsClean() to make the global rpmts reusable -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381249671___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
> What I am suggesting (but I know nothing serious about rust) is running an > array of header references during an iteration using *Link() that have > *Free() called when the rust Iterator goes out of scope. Yep, that's exactly what I'd like to do. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381246496___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re use of rust's regular Iterator type Since you are programming with rust+ffi there are some games you may be able to play with ref counting in C to adjust the lifetime of headers returned from an Iterator. What I am suggesting (but I know nothing serious about rust) is running an array of header references during an iteration using *Link() that have *Free() called when the rust Iterator goes out of scope. No idea whether that is possible, and be forewarned: a full serial iteration over Packages will load 100+ Mytes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381244609___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
There are almost no interesting usage cases for multiple transactions (at least until one attempts threaded installs, which are currently prevented by an exclusive fcntl lock in RPM4 anyways). What that means for bindings is that you can take out a static global transaction which can be used where needed w/o a need to pass an rpmts through all the methods. That's basically what yum through rpm-Python bindings, once you dig through all 3 layers of sub classing that yum developers deem to be necessary. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381240803___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
> Do an additional ref++ using *Link() if you want to make a header persistent > for further processing outside of an iteration (but don't forget the ref-- > using *Free() or you will have a huge memory leak ;-) Are there any additional lifetimes to worry about here, e.g. the lifetime of the `TransactionSet`? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381236606___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re the RPM5 doco That do yen info hasn't been updated for almost a decade (but not much has changed since last century). (aside) Meanwhile there are some significant differences in RPM5 match iterator S, including lazy index generation, partial matches on patterns applied to Bree index keys to improve performance, and header data is guaranteed to be immutable by using mmap RO protection, none of which is pertinent to RPM4 rust bindings. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381236177___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re using rust iterator S w ref counting The match iterator does a ref++ followed by a ref-- at the end of an iteration which guarantees that the header and MINMEM tag data exist for each iteration. Do an additional ref++ using *Link() if you want to make a header persistent for further processing outside of an iteration (but don't forget the ref-- using *Free() or you will have a huge memory leak ;-) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381234557___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Aha, so if I call this: http://rpm5.org/docs/api/group__header.html#g4f07e8040ed3195e374a44919ffe97c2 ...and `headerFree()` on drop, it sounds like I could switch to a normal Rust iterator which iterates over immutable references. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381233551___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Re: ref counting. Yes, an increment is exactly a *Link() call, and a decrement (and possible delayed/lazy de-allocation) is a * Free() call. There is also a *Unlink() method that is strictly a decrement w/o a deallocation, but that is really only useful when debugging, or when some snarly ref counting bug necessitates a Big Hammer Hack: all usual cases should use *Free() instead. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381232511___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Actually, rpmtsInitIterator() is/was an attempt to simplify retrievals by hiding the DB handle getter within a pass through lookup. Acceptance and updating code has been glacially slow all century. Note that rpmdbOpen's calls have had some very peculiar side effects to accomodate user demands (I'm not sure what the current state of affairs is: I tend to avoid *all* rpmdb questions in public). Usually, the rpmdb is opened (and reopened RO -> RW) lazily when needed during rpmts processing. The peculiar side effect I mention is/was -- if the application/user chose to explicitly open an rpmdb -- then that also disabled all opens.reopens that are usually handled when/where needed during rpmts handling. The rationale for the peculiarity is/was solely a principle of least surprise at the time. There has been too little usage of alternative back ends to understand whether an explicit DB handle might be useful. In general however, I believe your rust bindings would be improved by _NOT_ binding rpmdb open/close/init/delete/verify operations: attach methods to a rpmts object and use the rpmdb attached to a rpmts instead. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381230556___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@n3npq a few questions about something you said earlier: > RPM object (like ts, h, db) is ref-counted, but the counter increment is not > atomic. When are the reference counts incremented/decremented, and can I bump them externally? If so, I could have Rust increment the reference count when it takes a reference, and decrement it on drop. > With MINMEM, the tag data should be treated as if it were read-only (but > during header load/unload integer data will be swabbed). I have the read-only part covered in that I am giving out immutable references, however I'm still a bit unclear on what happens with header load/unload and in particular how that relates to taking the next item in a match iterator. If possible I think it'd be great if I could use Rust's regular [Iterator](https://doc.rust-lang.org/std/iter/trait.Iterator.html) type for RPM's match iterators, however for that to work all data the iterator references must live at least as long as the iterator itself. Is that possible? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381203705___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
(random comments on db.rs retrievals) Note that mire (match iterator regular expressions) can be rather expensive if patterns are used. rpm itself almost always uses strings through an index for performance. A proper implementation for mire patterns would need to do a partial match on the beginning part of the pattern and then proceed from there applying the full pattern. Just a warning to focus on what bindings are actually useful. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381195419___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
I would have used `rpm`, unfortunately someone else took it for something completely unrelated (and won't give it up, I already asked): https://crates.io/crates/rpm -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381148719___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
IMO it makes sense to use `rpm` + `rpm-sys` as a crate names. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381036107___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
> These days, we tend to call it librpm rather than rpmlib Haha oh really! What's great about that is I started down the road of naming things `librpm` then changed to `rpmlib` after noticing all the references to it: https://crates.io/crates/librpm-sys I can re-release the crate as `librpm` instead. I'd be happy to relicense the code as `LGPL-2.1+` as well as extract all of the code into a repository managed by **@rpm-software-management** (it's presently in a shared repo along with a number of other crates) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381016679___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
@tarcieri A few things: * These days, we tend to call it `librpm` rather than `rpmlib`, though of course, the API still calls it `rpmlib` all over the place... * That old document from Fedora is a terrible reference for how `librpm` can be used. Some better examples can be found in the code for the RPM Python bindings (in this repository), as well as the [libdnf](https://github.com/rpm-software-management/libdnf) library. * It'd be ideal if your crate was licensed `LGPL-2.1+`, as `librpm` is. * CLA signing is not required, but it would be nice if we can get this binding to live in the @rpm-software-management organization. @ffesti, @pmatilai, @dmach, is there something we can do here so that this can be made to happen? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-381005842___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Thanks for all of the feedback @n3npq! I will go through it and try to fix up my library accordingly. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380945817___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
(From a quick perusal of your rust code) You may need bindings for a FD_t since certain routines in the rpmlib API take a FD_t. There is also (in RPM4) a key ring object, basically a container for pubkeys. However RPM avoided handling private keys last century "munitions" and externalized signing operations in a helper. The attempts to do hardware signing for *.rpm are pretty much ad hoc slice-and-dice in external custom code. There are some hard design issues locating a key store when chroot(2) is routinely used as in rpm. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380867813___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
FWIW, there is little reason to expose a DB object per se in bindings. Instead wrap the mi "match iterator" methods. There's very little that can be done outside of rpm with a DB object because Berkeley DB is not simple programming, and all known replacements for Berkeley DB are exactly that, a replacement, not a standalone API. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380863227___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
RPM object (like ts, h, db) is ref-counted, but the counter increment is not atomic. With MINMEM, the tag data should be treated as if it were read-only (but during header load/unload integer data will be swabbed). The trickiest problem with header tag data and MINMEM is that the header reference count sometimes ends up protecting header tag data, whether by intent or by accident is arguable. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380861052___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
* STRING is ASCII, I18N tends to be UTF-8. * (my guess) count is the same as count in header data (I will verify/correct if my guess is wrong) * count is the number of octets for binary data * the CHAR data type should be treated as a uint8/int8. There are very few (perhaps only one) tag with that data type: see the C comments in rpmtag.h for the usual type for each tag. * STRING_ARRAY is mostly like argv, an array of pointers to strings, with some differences in allocation. With MINMEM, the strings are not allocated, and in some other cases, the strings are chunked so that freeing the array also frees the strings. (Aside) Lots of RPM tags are STRING_ARRAY, you will need to implement. The cryptographic primitives of digest/signing are pretty seriously twisted throughout rpm code, so I doubt you can do anything serious by replacing with, say, a tpm. Signing is done by invoking gpg however, so you can write a gpg-like wrapper to other devices. The binary format and verification depend heavily on RFC 2440/4880 conventions. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380858227___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Yep, I'll go through and fix things up to use that instead. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380824014___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
By the way, RPM stands for RPM Package Manager nowadays. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/429#issuecomment-380788143___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Rust bindings for rpmlib (#429)
Hello, I've written a nascent Rust binding for rpmlib: https://crates.io/crates/rpmlib First of all, I would love any feedback. This is my first deep dive into the internals of RPM, and while I found ample documentation about certain things, a lot of what I was doing was guesswork. I'm also not super up-to-speed on RPM nomenclature but I've largely tried to mirror the structure and terminology rpmlib itself uses. I've already gotten some feedback that it's "Red Hat", not "RedHat", and I probably shouldn't be calling it the "RedHat Package Manager", so it seems I could use some advice on appropriate project branding. Is "rpmlib" a good name for the Rust crate? I'd also be interested in upstreaming this if there's interest. I'm presently the sole author and am happy to relicense everything under LGPL/GPL, sign CLAs, transfer copyright or what have you, as well as transfer control of the packages on https://crates.io ## Lifetimes If there's one thing in particular I'd like clarification and feedback on it's my understanding rpmlib's memory model. Rust's claim to fame is the compiler's ability to prove properties about the lifetime relationships of objects in the program, and thereby provide safe "zero cost" (i.e. zero copy) abstractions which might otherwise be deemed "risky" in practically any other language due to the potential for use-after-free errors. I am trying to apply this approach in my Rust binding. In RPM nomenclature I am using `HEADERGET_MINMEM` with the goal of directly accessing memory owned by RPM and thereby providing a zero-copy API but also providing safe abstractions for doing so. Doing that correctly involves describing the precise memory relationships to the Rust compiler. First I'll say Rust's notion of memory safety applies to multithreaded programs, and in that regard I have just thrown a big mutex across the whole rpmlib FFI. In particular right now creating a transaction set also acquires the mutex and does not release it until it is complete. tl;dr: assume sequential/single-threaded access for now. I've largely been following this guide which provided much of the lifetime information I was looking for for things like transaction sets and iterators: https://docs-old.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch15s04.html (The combination of `docs-old` and `Draft` doesn't bode particularly well, but...) If I have one immediate takeaway: transaction sets everywhere! Which is something I can get on board with. In almost all cases (aside from reading the initial config/rpmrc and configuring macro contexts) everything seems to being with a transaction set, so that is the initial lifetime of importance from a Rust perspective. Regarding that, the referenced documentation suggests things like: > When you are done with a transaction set, call rpmtsFree: > rpmts rpmtsFree(rpmts ts); Rust has a trait that automatically frees things for you at the end of their lifetime called `Drop`, and it's the sort of RAIA pattern you might expect. I have implemented free for both transaction sets and iterators using this trait based on this documentation. Where things got a bit hazy was actually using `HEADERGET_MINMEM`. Quoting the linked documentation: > You do not need to free the Header returned by rpmdbNextIterator. Also, the > next call to rpmdbNextIterator will reset the Header. What I took this to mean, in Rustier nomenclature, is that the lifetime of a `Header` referenced by an iterator is valid until the next item is requested from the iterator. This is a bit different from a typical Rust `Iterator`, which provides a read-only view of a collection, where it's safe to have references to multiple items in the collection at once. There is a Rust pattern for what I think this is describing though, which is a `StreamingIterator` and the one I chose to use: https://docs.rs/streaming-iterator/ The idea of `StreamingIterator` is you iterate by borrowing a value from the iterator. Before you move on, you must give that value back. From a nitty gritty perspective, it actually does this by splitting up the iteration into two steps: one where you ask the iterator to mutate itself and "preload" the value into a buffer, and another where you immutably borrow that value from the iterator, with a lifetime guaranteed to end before you request the next item. The relevant code is here: https://github.com/iqlusion-io/crates/blob/master/rpmlib/src/db.rs#L219 The two relevant lifetimes are `'db` and `'ts` for "database" and "transaction set" respectively, with transaction set having the longest lifetime. In my usage of `HEADERGET_MINMEM` I have assumed the lifetime of a borrowed `Header` is only valid until the next one is requested, and users of the API must drop any references to the previous `Header` value before requesting the next. More nitty gritty details: it does this by means of Rust's affine type system:
