Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4 - Samba

This is a discussion on Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4 - Samba ; 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 ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4

  1. Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4

    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 ?

    Simo.

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


  2. Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4

    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


  3. Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4

    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.


  4. Re: [SCM] Samba Shared Repository - branch v4-0-test updated -release-4-0-0alpha4-105-g87385e4

    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


+ Reply to Thread