Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-11-25 Thread Marek Blaha
I did some testing in dnfdaemon and the patch is working for us. Just (as Panu 
mentioned) for consistency I'd like to change CompleteTransaction signal name 
to EndTransaction - the signal is emitted regardless the transaction was 
successful and "Complete" in the name suggests the success.
I was also thinking whether the result of the transaction should be part of the 
signal but having rpmdb cookie is perfectly fine with us to decide whether 
there actually was any change.
I suppose the plugin will be released as a separate sub-package so that the 
dnfdaemon  could depend on 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/pull/1255#issuecomment-733608943___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-11-24 Thread Marek Blaha
>From dnf-daemon point of view this plugin looks good and definitely useful. 
>I've just build rpm with this plugin enabled and I'm going to test how it 
>works. Thanks Florian!

-- 
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/pull/1255#issuecomment-732722116___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-26 Thread Panu Matilainen
> > imposes its own signal handling on users when the db is open
> 
> Can turn that off since 
> [56f49d7](https://github.com/rpm-software-management/rpm/commit/56f49d7f5af7c1c8a3eb478431356195adbfdd25)
>  right?

API users turning off signal handling is not a solution, it's a problem in its 
own right.

> > and our ABI hasn't been particularly stable historically
> 
> Maintaining C shared libraries is indeed extremely hard. I've messed up in 
> the past and only just barely caught it before releases etc. That said there 
> are nice tools now like [libabigail](https://pagure.io/libabigail) - should 
> be easy to set that up on CI here right?
> (I still need to do that for ostree)

librpm ABI stability or lack of thereof has little to do about it being 
difficult, it's merely that rpm has historically exported all manner of crap 
that it never should have, and it's not possible to get rid off it all in one 
go, unfortunately. So for 12+ years we've done staged "exit accumulated crap, 
bump soname" releases every now and then, rpm soname bumps are an absolute PITA.

> > dbus library itself is much more stable AIUI.
> 
> Yeah, libdbus as a C shared library has been fine and will continue to be so, 
> but exposing an API over DBus is a separate thing with long term commitments.

Eh, no kidding?

> 
> If we're effectively saying most projects shouldn't be using librpm to read 
> the rpm database effectively (that seems like what you're implying) that's a 
> rather radical thing right? Maybe I am misunderstanding though.

I said no such thing. Different users have different priorities and needs, 
projects should (be able to) use what works best for them. 

Linking to librpm comes with heavy and hard to understand babbage and is way 
too intimate (think "bare metal") for various "casual" users. A DBus API puts a 
much needed insulation layer between the process that actually accesses rpmdb 
and the unsuspecting piece of software wanting to know something about packages 
on the system. With the existing librpm API, very weird things happen in dark 
corners such as gdb running as root on a 32bit process on a 64bit system, doing 
rpmdb query for debuginfo discovery.

-- 
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/pull/1255#issuecomment-650015111___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-25 Thread Colin Walters
> imposes its own signal handling on users when the db is open

Can turn that off since 56f49d7f5af7c1c8a3eb478431356195adbfdd25 right?

> and our ABI hasn't been particularly stable historically

Maintaining C shared libraries is indeed extremely hard.  I've messed up in the 
past and only just barely caught it before releases etc.  That said there are 
nice tools now like [libabigail](https://pagure.io/libabigail) - should be easy 
to set that up on CI here right?
(I still need to do that for ostree)

> dbus library itself is much more stable AIUI.

Yeah, libdbus as a C shared library has been fine and will continue to be so, 
but exposing an API over DBus is a separate thing with long term commitments.

If we're effectively saying most projects shouldn't be using librpm to read the 
rpm database effectively (that seems like what you're implying) that's a rather 
radical thing right?  Maybe I am misunderstanding though.  



-- 
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/pull/1255#issuecomment-649535595___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-25 Thread Panu Matilainen
One important aspect of dbus vs librpm API is that not everybody wants to link 
to librpm, which imposes its own signal handling on users when the db is open, 
and our ABI hasn't been particularly stable historically, dbus library itself 
is much more stable AIUI.

-- 
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/pull/1255#issuecomment-649251979___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-17 Thread Michael Schroeder
(Colin, see issue #1124 for a solution using a named pipe)

-- 
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/pull/1255#issuecomment-645250342___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-17 Thread Panu Matilainen
@cgwalters , the use-case is not dnf itself but a daemon which will need to 
refresh it's view of rpmdb whenever *someone else* changes the rpmdb. See #1124 
for some background.

-- 
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/pull/1255#issuecomment-645185825___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-16 Thread Colin Walters
> This isn't just for dnf's benefit. It's a mechanism that anybody who needs to 
> do so is free to listen, and everybody else can ignore.

Speaking as a former DBus maintainer upstream here and I have written many DBus 
APIs...

It seems dramatically simpler to me to go with the inotify approach, the 
requirement is basically for librpm to `touch` the toplevel directory of the 
database path.  It's near zero cost if there is nothing listening.

And most crucially it avoids creating a new API for things to try to 
query/monitor the rpm database state.  There's already an API for that via 
librpm.  This signal isn't extensible - if e.g. a new field (say something 
related to modules) becomes interesting then we'd have to define a new one.  
Handling that requires going down to a "bag of properties" i.e. `a{sv}` in DBus 
type code model.  (We do this in many places in rpm-ostree) - but it loses some 
of the elegance.

A DBus API is a serious commitment, particularly once you have things listening 
to it.

That said a particular strength of DBus (really the reason it was created) is 
to be able to to cross privilege boundaries by having unprivileged processes 
(originally the desktop) sanely monitor and interact with system services.  But 
in this case since the consumer is (theoretically) dnf they're in the same 
privilege level (and in fact the same process!), so that doesn't apply.

FWIW here's the libostree code to bump mtime: 
https://github.com/ostreedev/ostree/blob/62632511f149adfc186da7f0e976006845591c8f/src/libostree/ostree-sysroot.c#L357

And here's where rpm-ostree uses it: 
https://github.com/coreos/rpm-ostree/blob/e6907d209b258388339b26e58a6da8cd022d1081/src/daemon/rpmostreed-sysroot.c#L788

That's how rpm-ostreed gets notified if e.g. an admin directly runs `ostree 
admin `, which is a lot like the situation where someone is using a 
traditional rpm system with a DBus daemon like dnfdragora/PackageKit or 
whatever and an admin uses `rpm -ivh` or so.


-- 
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/pull/1255#issuecomment-645103007___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
This isn't just for dnf's benefit. It's a mechanism that anybody who needs to 
do so is free to listen, and everybody else can ignore.

-- 
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/pull/1255#issuecomment-642408715___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Colin Walters
> Well, we add this on request of the DNF team that need to be notified if 
> something else (rpm cli) changes the rpmdb underneath their daemon. 

Ah, I see.  Well, nothing else can/should be changing the rpmdb on rpm-ostree 
based systems (see also https://github.com/coreos/rpm-ostree/pull/1789 where 
rpm-ostree will start wrapping `/usr/bin/rpm`).

> So this is going to be a plugin that can be disabled or just plainly not be 
> installed in the first place. We will probably not integrate this into the 
> systemd-inhibit plugin but have it as a separate plugin that is very similar 
> and comes in it's own sub package.

OK that seems fine, then we can not install it in the container image and 
rpm-ostree based systems.

> Can you please elaborate on why sending some DBus signals should be 
> problematic?

A few things.  First, rpm-ostree updates are *offline* (generating a new root) 
by default - so anything listening and responding to these DBus signals would 
be confused because it's not affecting the *current* rpm database but a new one 
it can't see.

Second, and related to that - which exact component would be listening to these 
signals?  Would it be `dnf` or `libdnf`?  I really hope it's not the library 
because then rpm-ostree would be talking to *itself* over DBus which...yeah I 
would like not to have that happen :smile: 

Do we really need DBus for this versus just e.g. having interested processes 
use `inotify()` on the path to the rpmdb's directory, and ensuring that RPM 
does `touch(/path/to/rpmdb)` whenever it makes a change?  This is basically 
what we do for ostree changes.  (And yes in some cases rpm-ostreed does catch 
inotify updates from its own changes but that's kind of intentional and much 
less heavy-weight than talking to oneself over dbus)

-- 
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/pull/1255#issuecomment-642022674___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> + fprintf(stderr, "Connection Error (%s)\n", err.message);
+   dbus_error_free();
+   }
+   if (state->bus) {
+   rc = dbus_bus_request_name(state->bus, "org.rpm.announce",
+ DBUS_NAME_FLAG_REPLACE_EXISTING , );
+   }
+   if (dbus_error_is_set()) {
+   //fprintf(stderr, "Name Error (%s)\n", err.message);
+   dbus_error_free();
+   }
+   if (DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER != rc) {
+   dbus_connection_close(state->bus);
+   dbus_connection_unref(state->bus);
+   state->bus = NULL;
+   state->logging = 0;

Unify this with the code in cleanup hook, maybe close_dbus() helper or such.

-- 
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/pull/1255#pullrequestreview-427927188___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +return RPMRC_OK;
+}
+
+static rpmRC dbus_announce_tsm_pre(rpmPlugin plugin, rpmts ts)
+{
+int rc;
+
+rc = open_dbus(plugin, ts);
+if (rc != RPMRC_OK)
+   return rc;
+return send_ts_message(plugin, "StartTransaction", ts, RPMRC_OK);
+}
+
+static rpmRC dbus_announce_tsm_post(rpmPlugin plugin, rpmts ts, int res)
+{
+return send_ts_message(plugin, "CompleteTransaction", ts, res);

This is perhaps in the bike-shedding department, but "complete" by definition 
seems to indicate success, whereas this can have an error condition as well. So 
maybe "EndTransaction" would be closer to pairing with Start in these context.

-- 
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/pull/1255#pullrequestreview-427924424___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +char * dbcookie;
+
+if (!state->logging) {
+   return RPMRC_OK;
+}
+
+msg = dbus_message_new_signal("/org/rpm/Transaction", // object name
+ "org.rpm.Transaction", // interface name
+ name); // name of the signal
+if (NULL == msg) {
+   return RPMRC_FAIL;
+}
+
+dbcookie = rpmdbCookie(rpmtsGetRdb(ts));
+if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, )) {
+   return RPMRC_FAIL;

Probably should also add the transaction ID to these messages.

-- 
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/pull/1255#discussion_r438021380___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +char * dbcookie;
+
+if (!state->logging) {
+   return RPMRC_OK;
+}
+
+msg = dbus_message_new_signal("/org/rpm/Transaction", // object name
+ "org.rpm.Transaction", // interface name
+ name); // name of the signal
+if (NULL == msg) {
+   return RPMRC_FAIL;
+}
+
+dbcookie = rpmdbCookie(rpmtsGetRdb(ts));
+if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, )) {
+   return RPMRC_FAIL;

This leaks memory in the failure case.

-- 
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/pull/1255#pullrequestreview-427912875___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +
+msg = dbus_message_new_signal("/org/rpm/Transaction", // object name
+ "org.rpm.Transaction", // interface name
+ name); // name of the signal
+if (NULL == msg) {
+   return RPMRC_FAIL;
+}
+
+dbcookie = rpmdbCookie(rpmtsGetRdb(ts));
+if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, )) {
+   return RPMRC_FAIL;
+}
+dbcookie = _free(dbcookie);
+
+if (!dbus_connection_send(state->bus, msg, )) {
+   return RPMRC_FAIL;

Hmm, failing the entire transaction if dbus announce fails seems a bit 
dramatic. Then again, if people are actually relying on these signals... 

-- 
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/pull/1255#pullrequestreview-427912576___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +static rpmRC dbus_announce_init(rpmPlugin plugin, rpmts ts)
+{
+struct dbus_announce_data * state = rcalloc(1, sizeof(*state));
+rpmPluginSetData(plugin, state);
+return RPMRC_OK;
+}
+
+static rpmRC open_dbus(rpmPlugin plugin, rpmts ts)
+{
+struct stat st;
+DBusError err;
+int rc = 0;
+struct dbus_announce_data * state = rpmPluginGetData(plugin);
+
+/* Assume we are logging but... */
+state->logging = 1;

This logging member seems redundant to me, we simply shouldn't open the bus at 
all for test- and chroot-transactions. The less state there is to worry about, 
the better.

-- 
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/pull/1255#pullrequestreview-427909419___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> + if (dbus_error_is_set()) {
+   //fprintf(stderr, "Name Error (%s)\n", err.message);
+   dbus_error_free();
+   }
+   if (DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER != rc) {
+   dbus_connection_close(state->bus);
+   dbus_connection_unref(state->bus);
+   state->bus = NULL;
+   state->logging = 0;
+   return RPMRC_NOTFOUND;
+   }
+return RPMRC_OK;
+}
+}
+
+return RPMRC_NOTFOUND;

The return code logic is confusing, with multiple returns in space of just a 
few lines. 

-- 
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/pull/1255#pullrequestreview-427904266___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +
+/* ...don't log test transactions */
+if (rpmtsFlags(ts) & (RPMTRANS_FLAG_TEST|RPMTRANS_FLAG_BUILD_PROBS))
+   state->logging = 0;
+
+/* ...don't log chroot transactions */
+if (!rstreq(rpmtsRootDir(ts), "/"))
+   state->logging = 0;
+
+/* Don't open */
+if (!state->logging || state->bus)
+   return RPMRC_OK;
+
+if (lstat("/run/systemd/system/", ) == 0) {
+if (S_ISDIR(st.st_mode)) {
+   // initialise the error value

Please use `/* ... */` for comments, // is not in the rpm coding style even if 
the compiler permits 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/pull/1255#pullrequestreview-427900196___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +/* ...don't log chroot transactions */
+if (!rstreq(rpmtsRootDir(ts), "/"))
+   state->logging = 0;
+
+/* Don't open */
+if (!state->logging || state->bus)
+   return RPMRC_OK;
+
+if (lstat("/run/systemd/system/", ) == 0) {
+if (S_ISDIR(st.st_mode)) {
+   // initialise the error value
+   dbus_error_init();
+
+   state->bus = dbus_bus_get_private(DBUS_BUS_SYSTEM, );
+   if (dbus_error_is_set()) {
+   fprintf(stderr, "Connection Error (%s)\n", err.message);

This should be filtered out in case dbus isn't running to begin with, similarly 
to what commit 708e61307bc3fd027b016fdf5a1d1a5274c1843c does. Also, rpmlog() 
should be used instead of fprintf().

-- 
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/pull/1255#pullrequestreview-427899061___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-10 Thread Panu Matilainen
@pmatilai commented on this pull request.



> +state->logging = 1;
+
+/* ...don't log test transactions */
+if (rpmtsFlags(ts) & (RPMTRANS_FLAG_TEST|RPMTRANS_FLAG_BUILD_PROBS))
+   state->logging = 0;
+
+/* ...don't log chroot transactions */
+if (!rstreq(rpmtsRootDir(ts), "/"))
+   state->logging = 0;
+
+/* Don't open */
+if (!state->logging || state->bus)
+   return RPMRC_OK;
+
+if (lstat("/run/systemd/system/", ) == 0) {
+if (S_ISDIR(st.st_mode)) {

That check looks like it's "inherited" from systemd_inhibit plugin, where this 
is relevant because we're talking to systemd over dbus (see commit 
ec6495d79fa9834bca3394f56e9f6eb933030b1d). This is an entirely different case 
though, other processes might want to know about rpmdb state changes regardless 
of whether systemd is running or not.

-- 
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/pull/1255#discussion_r438000287___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-09 Thread Florian Festi
> Is there any coordination between this and the work to add dbus to libdnf in 
> [rpm-software-management/libdnf#941](https://github.com/rpm-software-management/libdnf/pull/941)
>  for example?

Well, we add this on request of the DNF team that need to be notified if 
something else (rpm cli) changes the rpmdb underneath their daemon. Note that 
this is very different from the lib dnf thing even if it also uses DBus. RPM 
will not offer a DBUS interface that can be queried or send commands to. All 
this does is sending out signals that notify who ever is interested that the 
rpmdb is changing.
I will probably even drop the second patch providing more detailed information 
until there are real use cases. So all this is going to do is sending out a 
ping at the start and the end of every transaction.

> For rpm-ostree we are already a DBus daemon, and having multiple other 
> libraries in the stack also going out and talking to DBus is going to be a 
> bit problematic.
> Binding this to the systemd-inhibit plugin makes sense because we already 
> turn that off for rpm-ostree (because it's transactional, there's no reason 
> to inhibit).

So this is going to be a plugin that can be disabled or just plainly not be 
installed in the first place. We will probably not integrate this into the 
systemd-inhibit plugin but have it as a separate plugin that is very similar 
and comes in it's own sub package.

Can you please elaborate on why sending some DBus signals should be problematic?

-- 
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/pull/1255#issuecomment-641112150___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-08 Thread Jonathan Lebon
@jlebon commented on this pull request.



> @@ -0,0 +1,25 @@
+'\" t
+.TH "RPM-DBUS-ANNOUNCE" "8" "03 Jun 2020" "Red Hat, Inc."
+.SH NAME
+rpm-plugin-dbus-announce \- DBus plugin for the RPM Package Manager
+
+.SH Description
+
+The plugin writes basic information about rpm transactions to the
+system dbus - like packages installed or removed. Other programms can

programs

-- 
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/pull/1255#pullrequestreview-426510237___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-08 Thread Colin Walters
@cgwalters commented on this pull request.



> +state->logging = 1;
+
+/* ...don't log test transactions */
+if (rpmtsFlags(ts) & (RPMTRANS_FLAG_TEST|RPMTRANS_FLAG_BUILD_PROBS))
+   state->logging = 0;
+
+/* ...don't log chroot transactions */
+if (!rstreq(rpmtsRootDir(ts), "/"))
+   state->logging = 0;
+
+/* Don't open */
+if (!state->logging || state->bus)
+   return RPMRC_OK;
+
+if (lstat("/run/systemd/system/", ) == 0) {
+if (S_ISDIR(st.st_mode)) {

DBus the protocol doesn't need systemd (it predates it) but lately integrates 
with it well.  The reason I think we do this check is it's a proxy for "is this 
system booted".

For example, there's no dbus running in a default podman/docker style container 
(neither is there systemd, or in fact usually there isn't any other processes 
at all except e.g. yum/zypper running).  So there's no reason to try announcing.

Related discussion and code: https://github.com/systemd/systemd/pull/7631


-- 
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/pull/1255#discussion_r436890836___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-08 Thread Colin Walters
Is there any coordination between this and the work to add dbus to libdnf in 
https://github.com/rpm-software-management/libdnf/pull/941 for example?

For rpm-ostree we are already a DBus daemon, and having multiple other 
libraries in the stack also going out and talking to DBus is going to be a bit 
problematic.  

Binding this to the systemd-inhibit plugin makes sense because we already turn 
that off for rpm-ostree (because it's transactional, there's no reason to 
inhibit).


-- 
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/pull/1255#issuecomment-640780302___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-08 Thread Michael Schroeder
@mlschroe commented on this pull request.



> +state->logging = 1;
+
+/* ...don't log test transactions */
+if (rpmtsFlags(ts) & (RPMTRANS_FLAG_TEST|RPMTRANS_FLAG_BUILD_PROBS))
+   state->logging = 0;
+
+/* ...don't log chroot transactions */
+if (!rstreq(rpmtsRootDir(ts), "/"))
+   state->logging = 0;
+
+/* Don't open */
+if (!state->logging || state->bus)
+   return RPMRC_OK;
+
+if (lstat("/run/systemd/system/", ) == 0) {
+if (S_ISDIR(st.st_mode)) {

Pardon my ignorance, but why is this depending on some systemd directory? Does 
dbus need systemd?

-- 
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/pull/1255#pullrequestreview-426476154___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-05 Thread Florian Festi
Split the package signals into a separate patch as they need some deeper 
discussion on how the plugin hooks should actually work and what new we need to 
add (see #1254).

-- 
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/pull/1255#issuecomment-639431665___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-05 Thread Panu Matilainen
Having slept over it: the thing is that the packages aren't actually 
installed/removed and the result code until the transaction is over, we really 
must not advertise the inconsistent state to others. The syslog plugin gets 
this wrong too...

-- 
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/pull/1255#issuecomment-639283052___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-04 Thread Florian Festi
Yeah, I had the package info as a separate patch, but I figured it'd be easier 
to just delete the functions than splitting all the changes and fixes.
Otoh I could imaging some GUI tool to not blocking it's UI completely on an 
running transaction but instead just changing package by package.


-- 
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/pull/1255#issuecomment-638790678___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-04 Thread Panu Matilainen
...noisy and like you said in the ticket, we don't want people to start 
building their own triggers outside rpm. Information about the contents of a 
transaction in progress is probably best limited to the invoking process.

-- 
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/pull/1255#issuecomment-638772962___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-04 Thread Panu Matilainen
I'm starting to think we might actually be better off without sending 
notifications for individual packages, because this will be noisy, and doubly 
so since the psm hooks can get called multiple times per package as you noted. 
Just announce that transaction is in progress and when it ends. Which makes me 
wonder if we couldn't actually merge this with the inhibit plugin somehow, as 
then beginning and end of the transaction is the most interesting thing there.

-- 
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/pull/1255#issuecomment-638767344___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Add dbus-announce plugin (#1255)

2020-06-04 Thread Florian Festi
The plugin announces start and completion of transactions and
the installation and erasure of packages.

The patch still needs some polishing:

* Names of the DBus enities may need a second thought
* Man page is still missing a description of the dbus signal sent
* DBus config is not properly integrated into Makefile

Resolves: #1249 
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1255

-- Commit Summary --

  * Add dbus-announce plugin

-- File Changes --

M doc/Makefile.am (5)
A doc/rpm-plugin-dbus-announce.8 (25)
M macros.in (1)
M plugins/Makefile.am (5)
A plugins/dbus_announce.c (198)
A plugins/org.rpm.conf (10)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1255.patch
https://github.com/rpm-software-management/rpm/pull/1255.diff

-- 
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/pull/1255
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint