|Summary:||[SPEC EXTENSION][PATCH] Add new "arg0has=" string array matches|
|Product:||dbus||Reporter:||Lennart Poettering <lennart>|
|Component:||core||Assignee:||D-Bus Maintainers <dbus>|
|Status:||RESOLVED MOVED||QA Contact:||D-Bus Maintainers <dbus>|
|i915 platform:||i915 features:|
preparation: fix whitespace in the spec XML
the actual patch
signals: factor out match_rule_matches_string_arg()
signals: add support for arg0has keyword
Description Lennart Poettering 2015-08-25 17:53:22 UTC
Created attachment 117913 [details] [review] preparation: fix whitespace in the spec XML This adds support for "arg0has=" style matches that test strings against messages containing string arrays ("as"). It applies if there's at least one string in the array that matches. This is useful for implementing udev style "tagging" of devices. This has been discussed on the ML a while back: http://lists.freedesktop.org/archives/dbus/2015-April/016666.html There's also a sd-bus PR implementing this: https://github.com/systemd/systemd/pull/1036 This bug also adds a preparatory patch containing a trailing whitespace cleanup for the spec, to make it easier to edit and patch.
Comment 1 Lennart Poettering 2015-08-25 17:53:51 UTC
Created attachment 117914 [details] [review] the actual patch
Comment 2 Simon McVittie 2015-08-25 18:04:51 UTC
Comment on attachment 117913 [details] [review] preparation: fix whitespace in the spec XML If we had always had a policy of no trailing whitespace (and no tabs), I'd certainly be fine with applying that policy like this. However, we deliberately avoid fixing whitespace that is not in or adjacent to the lines you're touching for some other reason, so that patches can backport more easily. I think this particular change is OK because we never backport the spec anyway (what would that even mean?), but please don't do the same to anything else in the tree.
Comment 3 Simon McVittie 2015-08-25 18:06:18 UTC
Comment on attachment 117914 [details] [review] the actual patch Review of attachment 117914 [details] [review]: ----------------------------------------------------------------- This looks good, and I like the new name better, but it shouldn't land until the reference implementation understands arg0has. I'm about to release 1.10.0; we can perhaps have this in 1.11.0.
Comment 4 Lennart Poettering 2015-08-25 18:13:22 UTC
Is it really necessary to add this to the reference implementation? For us (systemd upstream) this would be no priority, since we only need this for the udev usecase, but that's nothing we'll do on dbus1, but only on kdbus. Hence we are fine if this is implemented in sd-bus/kdbus only for now, as long as it is in the spec.
Comment 5 Lennart Poettering 2015-08-25 18:15:41 UTC
Also, I think dbus should enforce a stricter whitespace regime and refuse unclean commits, and clean up the existing codebase once. While this will making bakports harder across the cleanup boundary, later on it will makes things much easier! But anyway, this is of course a separate issue, and the only reason I added this here as second patch is that it annoyed me so much when I added the spec extension, and I thought it might be time to start the clean-up at least on the more auxiliary parts of the codebase.
Comment 6 Simon McVittie 2015-11-06 16:39:33 UTC
(In reply to Lennart Poettering from comment #4) > Is it really necessary to add this to the reference implementation? Well, yes. Reference implementations should support as much of the protocol/API as is technically possible, otherwise they make a poor reference :-)
Comment 7 Simon McVittie 2015-11-06 16:40:04 UTC
Created attachment 119446 [details] [review] signals: factor out match_rule_matches_string_arg()
Comment 8 Simon McVittie 2015-11-06 16:40:25 UTC
Created attachment 119447 [details] [review] signals: add support for arg0has keyword
Comment 9 Simon McVittie 2015-11-06 16:44:37 UTC
(In reply to Lennart Poettering from comment #4) > For us (systemd upstream) this would be no priority, since we only need this > for the udev usecase, but that's nothing we'll do on dbus1, but only on > kdbus. Is this still the case at this point? Do you have other use-cases for this feature? Looking for "tags" on messages seems generically useful, and it isn't actually very much code, so I wouldn't necessarily object to merging it for 1.11 even if you aren't going to use it any time soon.
Comment 10 Simon McVittie 2015-11-07 10:31:53 UTC
ssh://people.freedesktop.org/~smcv/dbus arg0has-91755 http://cgit.freedesktop.org/~smcv/dbus/log/?h=arg0has-91755
Comment 11 Simon McVittie 2015-11-07 10:42:02 UTC
Comment on attachment 117913 [details] [review] preparation: fix whitespace in the spec XML (applied)
Comment 12 Simon McVittie 2016-06-30 13:02:12 UTC
If this is something that you want, or that anyone wants, I'm still waiting for review on the implementation.
Comment 13 GitLab Migration User 2018-10-12 21:23:15 UTC
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/130.