> 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
