> On Apr 2, 2018, at 5:35 AM, Aleksei Nikiforov <darktemp...@altlinux.org> > wrote: > > Hi > > 30.03.2018 18:35, Jeff Johnson пишет: >>> On Mar 30, 2018, at 10:11 AM, Aleksei Nikiforov <darktemp...@altlinux.org> >>> wrote: >>> >>> Hi >>> >>> 29.03.2018 15:55, Aleksei Nikiforov пишет: >>>> Hi >>>> 29.03.2018 15:51, Jeff Johnson пишет: >>>>> >>>>> >>>>>> On Mar 29, 2018, at 4:46 AM, Aleksei Nikiforov <darktemp...@altlinux.org> >>>>> >>>>> ... >>>>> >>>>>>>>> Adding AUTOINSTALLED through a transaction like this is far more >>>>>>>>> complex than necessary, particularly since rpm itself does not need >>>>>>>>> nor use that tag for any purpose >>>>>>>> I've looked at librpm API documentation a few moments ago and found >>>>>>>> rpmdbSetIteratorRewrite + rpmdbSetIteratorModified. Did you mean these >>>>>>>> interfaces or something else I missed which could be used for changing >>>>>>>> headers in rpm db? >>>>>>> Yes, those interfaces. Note that those interfaces are traversed only in >>>>>>> the case of a replaced file, which only rarely happens (multilib and >>>>>>> sloppy packaging are the two common cases that come to mind). >>>>>> >>>>> >>>>> Apologies for not being precise. >>>>> >>>>> Within rpm code, those interfaces are traversed only in the case of a >>>>> replaced file, and >>>>> hence might be considered "lightly tested" or buggy. >>>>> >>>>>> Patch 2 is used to provide interface which can be used for >>>>>> implementation of utilities like 'apt-mark auto' or 'apt-mark manual'. >>>>>> In that case no files are changed, no packages are actually reinstalled, >>>>>> only 1 header tag value is changed (or added, if tag AUTOINSTALLED was >>>>>> missing). >>>>>> >>>>>>>> As for doing it within transaction, why not? As far as I can see >>>>>>>> almost every action on RPM db is done on open transaction, including >>>>>>>> rpmtsRebuildDB and rpmdbSetIteratorRewrite + rpmdbSetIteratorModified. >>>>>>>> Atomicity and consistency of transaction wouldn't hurt either. >>>>>>> Here are the reasons not to implement as you have done: >>>>>>> 0) rpm transactions are atomic/consistent only within very narrow >>>>>>> meanings that have little to do with traditional meanings of ACID >>>>>>> database transactions. E.g. The underlying Berkeley DB uses a CDB, not >>>>>>> a transactional model. There are no logs nor rollbacks nor >>>>>>> commit/discard events. Atomicity is only within an exclusive fcntl >>>>>>> write lock, and consistency is only de facto, as in there is exactly >>>>>>> one add/del operation per transaction element, with no well defined way >>>>>>> to hide, say, an add/del operation until the rpm transaction is >>>>>>> committed. >>>>>>> 1) the rpmte interfaces aren't all that useful or compelling as an API. >>>>>>> I can't think of any application that has used/needed access to rpmte >>>>>>> transaction elements, because applications already have the headers >>>>>>> from which the transaction was composed. >>>>>>> 2) There isn't any known usage case within rpm for AUTOINSTALL, nor is >>>>>>> there any means to compute the value within rpm which has no depsolver. >>>>>>> Why should rpm start managing apt+rpm (my guess at what alt is doing) >>>>>>> data? If apt wishes AUTOINSTALL, then apt, not rpm, should manage that >>>>>>> data, and there are already sufficient API calls to perform that >>>>>>> management task. >>>>>> >>>>>> Yes, depsolver is external, outside of librpm, but it is desired to keep >>>>>> all information in one database, including AUTOINSTALL tag, and this tag >>>>>> may be needed to be changed after package installation. >>>>>> >>>>>> How can it be implemented other way? If rpmdbSetIteratorRewrite + >>>>>> rpmdbSetIteratorModified only works when files are changed, then this >>>>>> wouldn't work for this use-case. >>>>>> >>>>> >>>>> Just do what markReplacedFile() does, but in apt-mark, outside of rpm >>>>> code. >>>>> >>>> Thanks for clarification, I'll try doing it and see if it works for me. >>> >>> I've tried using rpmdbSetIteratorRewrite + rpmdbSetIteratorModified and it >>> didn't work for me. >>> I've tried calling headerDel + headerPutUint32, headerGet + headerGetUint32 >>> + writing new value to the pointed memory, headerMod. Every time it fails >>> to write modified header, function headerExport hits following condition: >>> >> Hmmm ... here's some context: >> There are 3 cases for tags in a header, named akin to stalagmite formation >> in a cave. >> 1) within the immutable region created when building a package (aka "rock") >> 2) appended to the immutable region (aka "dribble") >> 3) newly added tag data (aka "drip"). >> When a header is unloaded, a "drip" is appended, and thereby becomes a >> "dribble" when next loaded from an rpmdb. >> Newly added "drips" can be freely added/deleted until a header is unloaded >> (I.e. saved in the rpmdb), "dribbles" can be modified through the pointer >> (if careful), and changes >> to the immutable region "rock" can be done. >> Similar to changing file states in markReplacedFile(), AUTOINSTALLED is a >> "dribble" when retrieved from an rpmdb, and you can modify by >> headerGet(MINMEM) and changing the value directly through the returned >> pointer. >> The data type of AUTOINSTALL from int32 to int8, same as file states, to >> avoid alignment/padding issues with tag data. That's what usually breaks in >> headerExport() > > I've used rpmtdGetChar instead of headerGetUint32 and it finally worked as > expected. Thanks!
Good: that confirms that there is something screwy with "drip" alignment when unloading headers for re-writing through an iterator. (aside) I am not surprised: this was incredibly nasty code to implement way back when, and has never been used for anything but rewriting file states since. But is likely fixable with a little bit of work. > I think patch 2 may be dropped since it's no longer required. But patch 1 > looks still needed. Can patch 1 be merged? > >>> if (count != (rdlen + entry->info.count + drlen)) >>> goto errxit; >>> } >>> and returns NULL to function miFreeHeader. >>> >> Can you get some values for those variables? My guess is that the problem is >> in darken computed across "dribbles" earlier in headerExport(). > > It's no longer relevant since it works now, but in case it might be needed > here are values I got before using rpmtdGetChar: > > i = 0, count = 2408, rdlen = 3424, entry->info.count = 16, drlen = 156, sum = > 3596 (count != sum) > Thanks! >>> I was hitting similar issue when implementing current patches and worked >>> around it by making a copy of header and working with it: >>> >>> case TR_CHANGED: >>> tmp_h = rpmteDBHeader(te); >>> h = headerCopy(tmp_h); >>> headerFree(tmp_h); >>> break; >>> But I can't find a way to modify header returned by rpmdbNextIterator() and >>> make rpmdbFreeIterator() successfully save changes, or a way to replace >>> whole header from rpmdbMatchIterator. Am I missing some way to modify or >>> replace header and successfully write changes to rpm db? >>> >> An alternative approach is to retrieve the header, modify AUTOINSTALLED, and >> replace. > > Is there an API to replace whole header? I didn't find any yet. I thought > about copying all tags to temporary header, changing values there as needed, > cleaning all tags from original header and copying modified tags back. I > didn't try doing it yet though. > There is an API to retrieve the "immutable header region" which is exactly what was in the *.rpm package. The immutable header region is the plaintext blob on which rpm digests/signatures are based. So changing the header (or changing a "rock" tag) isn't just deleting tags, the immutable header region is wired throughout rpm package security validation. 73 de Jeff >> hth >> 73 de Jeff >>>>> 73 >>>>>>> 73 de Jeff >>>>>>>> Best regards >>>>>>>> Aleksei Nikifo >>>> Best regards >>>> Aleksei Nikiforov >>>> _______________________________________________ >>>> Rpm-maint mailing list >>>> Rpm-maint@lists.rpm.org >>>> http://lists.rpm.org/mailman/listinfo/rpm-maint >>> >>> Best regards >>> Aleksei Nikiforov > > Best regards > Aleksei Nikiforov _______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
