I'll take this one to somewhat atone for my sins.

So this bug is a bit nasty.  The generated error enum actually depends on the version of the dbus core it was compiled against, specifically the changes in dbus-protocol.h.  Here's the annotate:

d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	392)/** We failed to setup the environment correctly. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	393)#define DBUS_ERROR_SPAWN_SETUP_FAILED         "org.freedesktop.DBus.Error.Spawn.FailedToSetup"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	394)/** We failed to setup the config parser correctly. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	395)#define DBUS_ERROR_SPAWN_CONFIG_INVALID       "org.freedesktop.DBus.Error.Spawn.ConfigInvalid"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	396)/** Bus name was not valid. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	397)#define DBUS_ERROR_SPAWN_SERVICE_INVALID      "org.freedesktop.DBus.Error.Spawn.ServiceNotValid"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	398)/** Service file not found in system-services directory. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	399)#define DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND    "org.freedesktop.DBus.Error.Spawn.ServiceNotFound"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	400)/** Permissions are incorrect on the setuid helper. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	401)#define DBUS_ERROR_SPAWN_PERMISSIONS_INVALID  "org.freedesktop.DBus.Error.Spawn.PermissionsInvalid"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	402)/** Service file invalid (Name, User or Exec missing). */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	403)#define DBUS_ERROR_SPAWN_FILE_INVALID         "org.freedesktop.DBus.Error.Spawn.FileInvalid"
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	404)/** Tried to get a UNIX process ID and it wasn't available. */
7d11c1da	(Richard Hughes	2007-07-24 11:27:09 +0000	405)#define DBUS_ERROR_SPAWN_NO_MEMORY            "org.freedesktop.DBus.Error.Spawn.NoMemory"
15ef0ef6	(Havoc Pennington	2006-10-20 03:05:00 +0000	406)/** Tried to get a UNIX process ID and it wasn't available. */
c9c0adce	(David Zeuthen	2004-07-19 20:55:58 +0000	407)#define DBUS_ERROR_UNIX_PROCESS_ID_UNKNOWN    "org.freedesktop.DBus.Error.UnixProcessIdUnknown"
15ef0ef6	(Havoc Pennington	2006-10-20 03:05:00 +0000	408)/** A type signature is not valid. */
54a2e9f7	(Colin Walters	2005-02-24 16:03:56 +0000	409)#define DBUS_ERROR_INVALID_SIGNATURE          "org.freedesktop.DBus.Error.InvalidSignature"
15ef0ef6	(Havoc Pennington	2006-10-20 03:05:00 +0000	410)/** A file contains invalid syntax or is otherwise broken. */
d8155bf5	(Havoc Pennington	2006-10-01 03:18:47 +0000	411)#define DBUS_ERROR_INVALID_FILE_CONTENT       "org.freedesktop.DBus.Error.InvalidFileContent"
15ef0ef6	(Havoc Pennington	2006-10-20 03:05:00 +0000	412)/** Asked for SELinux security context and it wasn't available. */
9a94a135	(Colin Walters	2005-07-16 17:34:08 +0000	413)#define DBUS_ERROR_SELINUX_SECURITY_CONTEXT_UNKNOWN    "org.freedesktop.DBus.Error.SELinuxSecurityContextUnknown"
d7fbcb5e	(Simon McVittie	2007-10-10 11:38:34 +0100	414)/** There's already an object with the requested object path. */
d7fbcb5e	(Simon McVittie	2007-10-10 11:38:34 +0100	415)#define DBUS_ERROR_OBJECT_PATH_IN_USE         "org.freedesktop.DBus.Error.ObjectPathInUse"

I think what we need to do is pick a version of DBus that was shipped in most recent free operating systems, and use it as *the* base ABI.  Looking at Fedora, the gold dbus-glib shipped with Fedora 8 was built against DBus 1.1.2:

http://koji.fedoraproject.org/packages/dbus-glib/0.73/4.fc8/data/logs/i386/root.log

That corresponds to 2007-07-27.  This means that we would need to break the enum here:

d7fbcb5e	(Simon McVittie	2007-10-10 11:38:34 +0100	414)/** There's already an object with the requested object path. */
d7fbcb5e	(Simon McVittie	2007-10-10 11:38:34 +0100	415)#define DBUS_ERROR_OBJECT_PATH_IN_USE         "org.freedesktop.DBus.Error.ObjectPathInUse"

Basically my proposal is to remove the generation scripts, add in that error enum *after* the last DBUS_GLIB_ERROR_REMOTE_EXCEPTION.

What I could use here is some people giving me data from the Debian-based releases.

The other, simpler alternative is to break ABI for everyone by moving DBUS_GLIB_ERROR_REMOTE_EXCEPTION to be first.

Word on the street is Debian lenny's dbus-glib was built against DBus 1.1.1, which according to ChangeLog was released 2007-06-18.  That unfortunately means it has a different error enum, from before Richard's service activation error enums.

I suspect that breaking the enum value I added wouldn't actually affect anyone - it's mostly useful for binding authors, and I doubt binding authors want to depend on > 1.0 at this point. So if there needs to be an ABI change, it should be my ObjectPathInUse error.

There is only one function which can return this error, which was added in the same patch, and can be used to detect who's going to break if the enum value changes. As far as I can remember, that function can only return two errors (OOM or ObjectPathInUse), so in practice callers will probably treat any other return as if it was either OOM or ObjectPathInUse.

I can get you dbus-glib builds from Debian and Ubuntu if that would help, or you can use packages.debian.org and packages.ubuntu.com to get current .deb files for every supported or under-development version[1] - note that Debian lenny has not yet released, but Ubuntu hardy is currently in freeze.

Long-term, this mechanism needs to be fixed, either in libdbus (by treating the order of the error #defines as ABI, and probably moving the enum from dbus-glib there in order to remind people) or in dbus-glib (by having the enum maintained by hand). I'd vote for the latter, it's more obvious.

[1] For any non-Debian users who need to unpack Debian packages - .deb file is an ar archive containing two gzipped tarballs, control.tar.gz and data.tar.gz. data.tar.gz contains files like usr/include/dbus-1.0/dbus-glib.h. That's it; no need to use dpkg.

(In reply to comment #5)
> Word on the street is Debian lenny's dbus-glib was built against DBus 1.1.1,
> which according to ChangeLog was released 2007-06-18.  That unfortunately means
> it has a different error enum, from before Richard's service activation error
> enums.

As long as the enum in Debian's dbus-glib is a prefix of the enum in every other  dbus-glib, that's fine. Adding new members to this enum is considered to be a compatible change, AFAIK (i.e. users of the enum are meant to cope with unknown values gracefully).

(In reply to comment #7)
 
> As long as the enum in Debian's dbus-glib is a prefix of the enum in every
> other  dbus-glib, that's fine. Adding new members to this enum is considered to
> be a compatible change, AFAIK (i.e. users of the enum are meant to cope with
> unknown values gracefully).

It's not a prefix, unfortunately; here's a diff between Debian etch's version and the Fedora 8 version:

--- usr/include/dbus-1.0/dbus/dbus-glib-error-enum.h	2006-11-06 17:50:37.000000000 -0500
+++ /usr/include/dbus-1.0/dbus/dbus-glib-error-enum.h	2008-03-19 12:10:29.000000000 -0400
@@ -16,6 +16,7 @@
 DBUS_GERROR_DISCONNECTED,
 DBUS_GERROR_INVALID_ARGS,
 DBUS_GERROR_FILE_NOT_FOUND,
+DBUS_GERROR_FILE_EXISTS,
 DBUS_GERROR_UNKNOWN_METHOD,
 DBUS_GERROR_TIMED_OUT,
 DBUS_GERROR_MATCH_RULE_NOT_FOUND,
@@ -27,6 +28,7 @@
 DBUS_GERROR_SPAWN_FAILED,
 DBUS_GERROR_UNIX_PROCESS_ID_UNKNOWN,
 DBUS_GERROR_INVALID_SIGNATURE,
+DBUS_GERROR_INVALID_FILE_CONTENT,
 DBUS_GERROR_SELINUX_SECURITY_CONTEXT_UNKNOWN,
 DBUS_GERROR_REMOTE_EXCEPTION
 #ifndef DBUS_INSIDE_DBUS_GLIB_H

But there is a more fundamental problem - even if it was a strict prefix, every time someone has added an error, the definition of DBUS_GERROR_REMOTE_EXCEPTION has changed, because we always add it at the end.

Here's my suggested plan: 

1) Generate a list of error enums from the current git head of both dbus/dbus-glib.  
2) Remove the generation scripts, and make the current output the new standard ABI.  
3) Move the DBUS_GERROR_REMOTE_EXCEPTION to the beginning of the list.  
4) Add a comment in the dbus/dbus-protocol.h that when adding errors, to double check bindings.  What do the Python bindings do about this BTW?

(In reply to comment #8)
> But there is a more fundamental problem - even if it was a strict prefix, every
> time someone has added an error, the definition of DBUS_GERROR_REMOTE_EXCEPTION
> has changed, because we always add it at the end.

Oh my god. How did anyone ever expect this to work?!

DBUS_GERROR_REMOTE_EXCEPTION is fundamental to dbus-glib's API - you are not allowed to call dbus_g_error_get_name() (it will assert) unless the error code is DBUS_GERROR_REMOTE_EXCEPTION. So this is actually much more significant than it might seem - everything that uses the symbol dbus_g_error_get_name() will need recompilation against the new dbus-glib ABI.

> Here's my suggested plan: 
> 
> 1) Generate a list of error enums from the current git head of both
> dbus/dbus-glib.  
> 2) Remove the generation scripts, and make the current output the new standard
> ABI.  
> 3) Move the DBUS_GERROR_REMOTE_EXCEPTION to the beginning of the list.  
> 4) Add a comment in the dbus/dbus-protocol.h that when adding errors, to double
> check bindings.  What do the Python bindings do about this BTW?

The Python bindings are fine, they just use the strings.

dbus-glib ought to be robust against unknown errors in the D-Bus namespace (it could just use DBUS_GERROR_REMOTE_EXCEPTION like it does for errors in other namespaces), although I could well believe that it isn't.

I've extracted the current .deb files from an assortment of Debian-based distributions, taking Debian testing/unstable (lenny/sid - they currently contain the exact same version of dbus-glib) as a baseline.

--- lenny.h	2008-04-14 11:53:39.000000000 +0100
+++ etch.h	2008-04-14 11:58:15.000000000 +0100
@@ -16,7 +16,6 @@
 DBUS_GERROR_DISCONNECTED,
 DBUS_GERROR_INVALID_ARGS,
 DBUS_GERROR_FILE_NOT_FOUND,
-DBUS_GERROR_FILE_EXISTS,
 DBUS_GERROR_UNKNOWN_METHOD,
 DBUS_GERROR_TIMED_OUT,
 DBUS_GERROR_MATCH_RULE_NOT_FOUND,
@@ -28,7 +27,6 @@
 DBUS_GERROR_SPAWN_FAILED,
 DBUS_GERROR_UNIX_PROCESS_ID_UNKNOWN,
 DBUS_GERROR_INVALID_SIGNATURE,
-DBUS_GERROR_INVALID_FILE_CONTENT,
 DBUS_GERROR_SELINUX_SECURITY_CONTEXT_UNKNOWN,
 DBUS_GERROR_REMOTE_EXCEPTION
 #ifndef DBUS_INSIDE_DBUS_GLIB_H

(i.e. lenny's dbus-glib is - currently! - the same as Fedora 8).

Ubuntu edgy is the same as Debian etch.

Ubuntu feisty, gutsy and hardy are all the same as Debian lenny.

So, there are two equivalence classes:

Debian etch = Ubuntu edgy

Debian lenny*, sid* = Ubuntu feisty, gutsy, hardy* = Fedora 8

and I think we should standardize on the ABI of F8/lenny/... when we freeze it.

(Distributions marked * in the equivalence classes are beta versions of unreleased distros, except for Debian sid which will never be released - it's the equivalent of Rawhide. Distributions not so marked have already released.)

Red Hat people: if you can ensure that Fedora 9 releases with the same ABI that was in Fedora 8 and Ubuntu gutsy, we'll do the same for Debian lenny.

Hopefully we can get a fixed package into Ubuntu hardy too (it's in freeze, but perhaps we can get an exception - sjoerd's going to go and shout at their D-Bus maintainer). As it stands, the version in hardy right now is OK, but I suspect that if they recompiled it with no changes, it'd become incompatible.

Thanks to you all for the investigations so far, and to Sjoerd for pinging me (representing Ubuntu development here).

So, did I get that right: dbus itself is considered fine (we have 1.1.20 in hardy), and dbus-glib's auto-enum-generation magic should be dropped and replaced by a static enum list?

I'd like to put this into hardy proper, otherwise we have a time bomb for providing security/critical bug fix updates for hardy-updates. Is there already a fix in progress, or shall we cowboy this by copy&paste'ing the current header files and fixating them?

Thanks, Martin

Created attachment 15894 [details] [review]
Freeze error ABI at the version seen in Fedora 8 / Ubuntu 7.10

I'm fixing this in the Debian package and upstream. Here's a patch against git dbus-glib (which can also be found in http://people.freedesktop.org/~smcv/git/dbus-glib.git). It applies cleanly to 0.74-1 - I'm preparing a Debian version 0.74-2 now, so Ubuntu should just need to check and sync that.

The patch I've attached is deliberately uninvasive in the sense that it does not add or remove files or change a Makefile.am, so it can be applied by distros without having to autoreconf.

Obviously, this leaves some dead code that we can and should clean up (make-*.sh don't even need to exist, let alone get run) after this critical bug has been fixed, but IMO that can wait!

Retitling to avoid the "git head" red herring.

Ok, patch looks good to me.  I'll take care of making sure Fedora 9 releases with this ABI.

Ah, that patch is pretty much what I had in mind, too.

For the record, this is now in Ubuntu 8.04 (hardy) and in Debian unstable, too.

Thanks everyone!

Can you commit this patch?

I'll assume that Colin's comment means "reviewed, looks good" and commit this.

I've verified that the Fedora 9 header is identical to this, but have added this patch into Fedora anyways: http://koji.fedoraproject.org/koji/buildinfo?buildID=46268

Since this patch is now committed, closing the bug.

commit 8fe656de630e851bc512bbd7e7a6c18c53aecb61
Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date:   Mon Apr 14 12:56:43 2008 +0100

    Freeze error ABI at the ABI used in Fedora 8 and Ubuntu gutsy.
    
    This avoids getting a different ABI depending on the version of libdbus
    we're compiled against. fd.o #15430, Debian #476080.