Hi

06.04.2018 15:39, Jeff Johnson пишет:



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.


Yes, I understood that it's proposed to not patch rpm library but use existing interfaces in such unexpected way instead. I just agree that proposed approach is obscure. I'd prefer to have more clean and straightforward API for this task.

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

Best regards
Aleksei Nikiforov
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to