Hi Simo,

Am Sonntag, den 22.06.2008, 23:00 -0400 schrieb simo:
> On Thu, 2008-06-19 at 07:52 -0500, Stefan Metzmacher wrote:
> > @@ -34,6 +34,7 @@
> > #include "replace.h"
> > #include "events.h"
> > #include "events_internal.h"
> > +#include "events_util.h"
> > #include
> > #include
> I was going to say that this is unnecessary because events_util.h was
> included in events_internal.h but then I saw Jelmer's patch that removed
> events_util.h from events_internal.h
>
> Jelmer,
> I think your patch is wrong, events_util.h defines only internally used
> helper functions, so the fix should have been to not install
> events_util.h imo, not to untangle it from events_internal.h, can you
> fix that, or should I revert and fix it myself ?
Without that patch, we were breaking external software that was using
events_internal.h, which is one of the headers that is being installed.
We have never and still are not installing events_util.h.

events_internal.h needs to be installed because other backends for the
events subsystem (such as the GLib-based one that is part of Samba-GTK)
need it. It broke because it was including events_util.h that was not
being installed.

I don't like installing events_util.h because it duplicates code in the
util library and so I think that code rather than the code in
events_util.h should be used.

Cheers,

Jelmer

--
Jelmer Vernooij - http://samba.org/~jelmer/
Jabber: jelmer@jabber.fsfe.org

On Mon, 2008-06-23 at 06:37 +0200, Jelmer Vernooij wrote:
> Hi Simo,
>
> Am Sonntag, den 22.06.2008, 23:00 -0400 schrieb simo:
> > On Thu, 2008-06-19 at 07:52 -0500, Stefan Metzmacher wrote:
> > > @@ -34,6 +34,7 @@
> > > #include "replace.h"
> > > #include "events.h"
> > > #include "events_internal.h"
> > > +#include "events_util.h"
> > > #include
> > > #include
> > I was going to say that this is unnecessary because events_util.h was
> > included in events_internal.h but then I saw Jelmer's patch that removed
> > events_util.h from events_internal.h
> >
> > Jelmer,
> > I think your patch is wrong, events_util.h defines only internally used
> > helper functions, so the fix should have been to not install
> > events_util.h imo, not to untangle it from events_internal.h, can you
> > fix that, or should I revert and fix it myself ?
> Without that patch, we were breaking external software that was using
> events_internal.h, which is one of the headers that is being installed.
> We have never and still are not installing events_util.h.
>
> events_internal.h needs to be installed because other backends for the
> events subsystem (such as the GLib-based one that is part of Samba-GTK)
> need it. It broke because it was including events_util.h that was not
> being installed.
>
> I don't like installing events_util.h because it duplicates code in the
> util library and so I think that code rather than the code in
> events_util.h should be used.

Ah, I guess I got it backwards, it is very strange to install an
"internal" header, should we split it into an 'internal' and a 'plugins'
one so that we expose only what should be used by plugins ?

Simo.

--
Simo Sorce
Samba Team GPL Compliance Officer
Senior Software Engineer at Red Hat Inc.

Am Montag, den 23.06.2008, 09:46 -0400 schrieb simo:
> On Mon, 2008-06-23 at 06:37 +0200, Jelmer Vernooij wrote:
> > Am Sonntag, den 22.06.2008, 23:00 -0400 schrieb simo:
> > > On Thu, 2008-06-19 at 07:52 -0500, Stefan Metzmacher wrote:
> > > > @@ -34,6 +34,7 @@
> > > > #include "replace.h"
> > > > #include "events.h"
> > > > #include "events_internal.h"
> > > > +#include "events_util.h"
> > > > #include
> > > > #include
> > > I was going to say that this is unnecessary because events_util.h was
> > > included in events_internal.h but then I saw Jelmer's patch that removed
> > > events_util.h from events_internal.h
> > >
> > > Jelmer,
> > > I think your patch is wrong, events_util.h defines only internally used
> > > helper functions, so the fix should have been to not install
> > > events_util.h imo, not to untangle it from events_internal.h, can you
> > > fix that, or should I revert and fix it myself ?
> > Without that patch, we were breaking external software that was using
> > events_internal.h, which is one of the headers that is being installed.
> > We have never and still are not installing events_util.h.
> >
> > events_internal.h needs to be installed because other backends for the
> > events subsystem (such as the GLib-based one that is part of Samba-GTK)
> > need it. It broke because it was including events_util.h that was not
> > being installed.
> >
> > I don't like installing events_util.h because it duplicates code in the
> > util library and so I think that code rather than the code in
> > events_util.h should be used.
> Ah, I guess I got it backwards, it is very strange to install an
> "internal" header, should we split it into an 'internal' and a 'plugins'
> one so that we expose only what should be used by plugins ?
Yeah, that would probably be a bit less confusing :-) Feel free to
change the names, but please let me know when you do so I can update
samba-gtk as well.

Cheers,

Jelmer

--
Jelmer Vernooij - http://samba.org/~jelmer/
Jabber: jelmer@jabber.fsfe.org