> On Apr 6, 2018, at 8:14 AM, Aleksei Nikiforov <darktemp...@altlinux.org> > wrote: > > Hi > > 06.04.2018 14:54, Jeff Johnson пишет: >> Sent from my iPad >>> On Apr 6, 2018, at 3:39 AM, Aleksei Nikiforov <darktemp...@altlinux.org> >>> wrote: >>> >>> Hi >>> >>> 05.04.2018 23:17, Jeff Johnson пишет: >>>> Sent from my iPad >>>>> On Apr 4, 2018, at 9:25 AM, Aleksei Nikiforov <darktemp...@altlinux.org> >>>>> wrote: >>>>> >>>>> Hi >>>>> >>>>> Since patch 2 is no longer needed, could you please provide a feedback >>>>> about patch 1? It'd be great if this patch could be merged. >>>> I can try ... >>>> The patch seems (from examining the patch only with limited context) to be >>>> propagating AUTOINSTALL from the old -> new header at the rpmte layer in >>>> order to ensure that the appended AUTOINSTALL tag value is persistent >>>> across package upgrades. >>>> As before, managing AUTOINSTALL persistence for a depsolver in rpmlib >>>> like this is rather awkward. E.g. what code should provide the >>>> add/modify/delete operations for AUTOINSTALL, particularly if/when the >>>> value is wrong? >>>> I suspect (from memory, I've not checked) that there are some odd corner >>>> cases, particularly if/when a package is reinstalled. But perhaps dropping >>>> AUTOINSTALL is a feature, not a bug, in that case. >>>> You likely should force a default value for AUTOINSTALL if/when the tag is >>>> not present because that is 1 fewer error returns to worry about. See how >>>> EPOCHNUM is implemented, likely as another header extension. You likely >>>> can use AUTOINSTALL as both an extension (mostly used by headerFormat) and >>>> as a tag-in-a-header (used by headerGet et al at the lower level) if you >>>> are careful. >>> >>> No, this patch is needed for adding this tag AUTOINSTALLED on package >>> installation. When package is added to transaction via functions >>> rpmtsAddInstallElement or rpmtsAddReinstallElement, it takes headers as one >>> of arguments. But it doesn't save header itself, it saves just enough info >>> to find package's file to read header from it later, when transaction would >>> be running. Header passed to function is not saved, i.e. added/modified >>> tags are not saved. Since rpm file doesn't contain AUTOINSTALLED tag, and >>> shouldn't contain it due to the dynamic nature of it's value, the >>> AUTOINSTALLED tag has to be saved there as well, so the value can be >>> restored later. >> Ah, my bad: apologies for misanalyzing your patch (I'm on an iPad without >> grep, sigh). >>> So, this change only saves AUTOINSTALLED tag passed via functions >>> rpmtsAddInstallElement and rpmtsAddReinstallElement. It's up to caller of >>> those functions to add these headers, but if this tag isn't added function >>> headerGetNumber returns 0 (i.e. sane default value). And later, when >>> transaction is running, the value of this tag is restored from saved >>> location and added to header read from file. >>> >>> Since on package upgrade or reinstall the header is read from file again, >>> it doesn't contain tag AUTOINSTALLED, and adding this tag should be safe. >>> >>> Managing the value of this tag AUTOINSTALLED on package installation, >>> reinstallation, upgrade or any other scenario is left to the user of >>> library, except for setting to default value "zero" (meaning "manually >>> installed") if no other value was provided. >>> >>>> Generally, I'd like to see RPM permit rpmlib users (like depsolvers) be >>>> able to freely append tags (arbitrary tagno, type, or reserved value) as >>>> needed without patching rpm itself. >>> >>> Well, yes, it's due to lack of a method to freely add tags to installed >>> packages' headers or to mark additionally added headers to be copied from >>> passed header to header saved into rpmdb (that's currently 2 different >>> headers, even if they're similar) that I had to make this change. >> Well there is a set of methods that can be used to add AUTOINSTALL, just >> abstract and obscure. >> The RPMCALLBACK_* notify callback used to push progress bars can likely be >> extended to add AUTOINSTALL (or any other tag) before the header is saved in >> an rpmdb. >> Use rpmteHeader() to get the header within the callback, and pass the >> AUTOINSTALL (or any other tags to be added) state into the callback. >> See rpmShowProgress() in lib/rpminstall.c for what the rpm CLI does. > > Well, that looks like a hack I wouldn't want to implement. >
Hack? I am not suggesting patching lib/rpminstall.c to add AUTOINSTALL. I am suggesting that you write a custom callback that adds AUTOINSTALL when a specific event is received, and pointed you at the CLI example. There is another example in rpm-python methods. The advantage to the notify callback is that all known rpm depsolvers already are using the callback. > If generic solution is desired for current change, it might be a good idea to > modify functions rpmtsAddInstallElement and rpmtsAddReinstallElement and add > new argument to these functions. Or add new versions of these functions and > keep current ones for backwards compatibility. This argument would be some > sort of container of tags and their values which would be added to header > saved to rpmdb. > There are many possible implementations that add a tag to a header somewhere, including your original patch. 73 de Jeff >> hth >> 73 de Jeff >>>> Yes, the header unload needs to be fixed to support more than just an int8 >>>> data type for full generality. >>>> hth >>>> 73 de Jeff >>>>> 28.03.2018 14:58, Aleksei Nikiforov пишет: >>>>>> Signed-off-by: Aleksei Nikiforov <darktemp...@altlinux.org> >>>>>> --- >>>>>> lib/rpmte.c | 8 ++++++++ >>>>>> 1 file changed, 8 insertions(+) >>>>>> diff --git a/lib/rpmte.c b/lib/rpmte.c >>>>>> index 40aa5e9..238c8b6 100644 >>>>>> --- a/lib/rpmte.c >>>>>> +++ b/lib/rpmte.c >>>>>> @@ -39,6 +39,7 @@ struct rpmte_s { >>>>>> char * arch; /*!< Architecture hint. */ >>>>>> char * os; /*!< Operating system hint. */ >>>>>> int isSource; /*!< (TR_ADDED) source rpm? */ >>>>>> + uint32_t autoinstalled; /*! Indicates whether package was >>>>>> installed just as dependency satisfier or not */ >>>>>> rpmte depends; /*!< Package updated by this package >>>>>> (ERASE te) */ >>>>>> rpmte parent; /*!< Parent transaction element. */ >>>>>> @@ -191,6 +192,8 @@ static int addTE(rpmte p, Header h, fnpyKey key, >>>>>> rpmRelocation * relocs) >>>>>> if (p->type == TR_ADDED) >>>>>> p->pkgFileSize = headerGetNumber(h, RPMTAG_LONGSIGSIZE) + 96 + 256; >>>>>> + p->autoinstalled = headerGetNumber(h, RPMTAG_AUTOINSTALLED); >>>>>> + >>>>>> rc = 0; >>>>>> exit: >>>>>> @@ -576,6 +579,11 @@ static int rpmteOpen(rpmte te, int reload_fi) >>>>>> rc = 1; >>>>>> } >>>>>> + if (rc) >>>>>> + { >>>>>> + rc = (headerPutUint32(h, RPMTAG_AUTOINSTALLED, >>>>>> &(te->autoinstalled), 1) == 1); >>>>>> + } >>>>>> + >>>>>> rpmteSetHeader(te, h); >>>>>> headerFree(h); >>>>>> } >>>>> >>>>> Best regards >>>>> Aleksei Nikiforov >>> >>> 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