Bug 72958

dbus/doc is a trap, the specification is actually dbus/core (Bug #67034) and I don't get bugmail for dbus/doc.

Either invalidates or true would be entirely valid for these properties too: the signal is emitted whenever they change (because a false predicate implies anything: it's equally valid to say "your computer catches fire whenever they change"). The patch seems fine, though.

(In reply to comment #1)

> Either invalidates or true would be entirely valid for these properties too:
> the signal is emitted whenever they change (because a false predicate
> implies anything: it's equally valid to say "your computer catches fire
> whenever they change"). The patch seems fine, though.

Well, from the client side code generator's view "true", "const" and "invalidates" could probably be treated the same way. But for the same reason we have seperate values "true" and "invalidates" already we should also distuingish "const", since server-side code generators do or could care. (libsystemd-bus cares, for example).

Does your "patch seems fine" mean that you'd be Ok if I push this?

Please hold off until next week, at least - I'm about to do a 1.8.x stable-branch, so dbus master is somewhat frozen.

There are two considerations here:

a) we should give server-side code generators a hint that the thing is constant (they can not emit a notify_thing_changed() function I suppose?), give client-side code generators a hint that they can cache it (as they could for "true" and perhaps "invalidates", and give documentation readers that information

b) we shouldn't break clients that don't know what to do with a "const" value for this annotation (they would presumably fall back to treating it as "false" which is exactly what we don't want)

With hindsight, this should probably have been several boolean annotations: EmitsChanged, EmitsInvalidated, and now Constant. Then adding new ones would be a simple matter of "adding more of these annotations gives more guarantees".

I would personally lean towards adding a parallel Constant annotation, which gives us both (a) and (b).

If people don't like that, I'd tolerate a new "const" value, which gives us (a) but breaks (b).

Any opinions from the other maintainers?

Concrete proposal (slightly pseudocode, I can't remember exactly how the XML works):

<property name="Pi" access="read" type="d">
  <annotation name="org.freedesktop.DBus.Property.Constant"
    value="true"/>
  <!-- Optionally, for backwards compat: this is saying "if this property
       ever changes (which it doesn't) we will emit the signal with
       the value" so that client bindings know they can cache it.
       Client bindings SHOULD NOT require this, but older ones will. -->
  <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal"
    value="true"/>
</property>

I think "true" is slightly better than "invalidates" for this purpose: you can claim either with a straight face, and a binding that wants to guarantee to get new values synchronously might conceivably ignore "invalidated".

(In reply to comment #3)
> Please hold off until next week, at least - I'm about to do a 1.8.x
> stable-branch, so dbus master is somewhat frozen.
> 
> There are two considerations here:
> 
> a) we should give server-side code generators a hint that the thing is
> constant (they can not emit a notify_thing_changed() function I suppose?),
> give client-side code generators a hint that they can cache it (as they
> could for "true" and perhaps "invalidates", and give documentation readers
> that information
> 
> b) we shouldn't break clients that don't know what to do with a "const"
> value for this annotation (they would presumably fall back to treating it as
> "false" which is exactly what we don't want)
> 
> With hindsight, this should probably have been several boolean annotations:
> EmitsChanged, EmitsInvalidated, and now Constant. Then adding new ones would
> be a simple matter of "adding more of these annotations gives more
> guarantees".
> 
> I would personally lean towards adding a parallel Constant annotation, which
> gives us both (a) and (b).
> 
> If people don't like that, I'd tolerate a new "const" value, which gives us
> (a) but breaks (b).

I tried to find code uses this annotation and only found telepathy, and that code doesn't really care much about any new value. Also, I don't think taking an unknown value as "false" wouldn't be much of a problem. I mean, it might make things a bit slower, since client code would not cache the value, but it would still work correctly. And if it took it for "true" or for "invalidates" nothing would happen either.

I'd really like to keep this simple, and just add to the existing annotation rather than a new one... But I am happy with either...

Heya, would like to resurrect this, the discussion somehow petered out. I still think the original patch is the best option... ;-)

Simon, any word on this? Would love to just commit this...

(In reply to comment #3)
> I would personally lean towards adding a parallel Constant annotation, which
> gives us both (a) and (b).

I still think this (Comment #4), but...

> If people don't like that, I'd tolerate a new "const" value, which gives us
> (a) but breaks (b).

... I do see Lennart's point about simplicity, so I'm not going to veto Lennart's patch if people prefer it.

Does anyone else have an opinion? thiago? walters? davidz?

(In reply to Lennart Poettering from comment #6)
> Heya, would like to resurrect this, the discussion somehow petered out. I
> still think the original patch is the best option... ;-)

I still think my "separate annotation" proposal is the best option, each of us seems to be prepared to tolerate the other's proposal, and apparently nobody else has any opinion whatsoever.

To resolve the deadlock, I've applied your patch - worse is better, and all that. Fixed in git for specification v0.25 (dbus 1.9.2).

Thanks for commiting this. sd-bus carries an implementation of this already (though it's not exported officially yet).