new: x11/libdbus-c++

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

new: x11/libdbus-c++

Stefan Sperling-5
Attached is a port of libdbus-c++.

I need this for an upcoming port of TextSuggest, which I'll mail out
soon, too.

OK?


libdbus-c++.tar.gz (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stuart Henderson
On 2019/03/03 14:45, Stefan Sperling wrote:
> Attached is a port of libdbus-c++.
>
> I need this for an upcoming port of TextSuggest, which I'll mail out
> soon, too.
>
> OK?
>

Please start from

SHARED_LIBS =   dbus-c++-1      0.0

..mostly so we can see that the version number is properly under control
of ports infrastructure. And please add

COMPILER =              base-clang ports-gcc base-gcc

so that it's built against the same C++ standard library as other ports
(this is generally needed for all C++ ports unless there are special
circumstances, mostly things on the path to building ports-gcc).

Otherwise OK.

Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stefan Sperling-5
On Sun, Mar 03, 2019 at 02:01:55PM +0000, Stuart Henderson wrote:

> On 2019/03/03 14:45, Stefan Sperling wrote:
> > Attached is a port of libdbus-c++.
> >
> > I need this for an upcoming port of TextSuggest, which I'll mail out
> > soon, too.
> >
> > OK?
> >
>
> Please start from
>
> SHARED_LIBS =   dbus-c++-1      0.0
>
> ..mostly so we can see that the version number is properly under control
> of ports infrastructure. And please add
>
> COMPILER =              base-clang ports-gcc base-gcc
>
> so that it's built against the same C++ standard library as other ports
> (this is generally needed for all C++ ports unless there are special
> circumstances, mostly things on the path to building ports-gcc).
>
> Otherwise OK.
>
Here's a new version with some changes on top of your suggestions:

1) Add more patches, and add comments to all patches.
   In particular, fix poll(2) event mask which resulted in unhandled
   POLLHUP events in the main event loop which made textsuggest-server
   spin on the CPU. As far as I understand, the "unlock" file descriptors
   are only interested in POLLIN events. They can be triggered to wake the
   event loop while a thread sleeps in poll(2). This feature is used by the
   library's teardown code. POLLOUT makes no sense here.
   textsuggest uses much less CPU becomes a lot more responsive with this fix.

2) Enable glib integration. This adds another shared library which is not
   used by textsuggest but will perhaps be useful if other consumers of
   libdbus-c++ show up.

3) Run regression test (there's just one, the others are just empty stubs).

Still OK?

libdbus-c++.tar.gz (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stefan Sperling-5
On Wed, Mar 06, 2019 at 11:55:17AM +0100, Stefan Sperling wrote:
>    In particular, fix poll(2) event mask which resulted in unhandled
>    POLLHUP events in the main event loop which made textsuggest-server
>    spin on the CPU.

Sorry, this should have said "POLLOUT", not "POLLHUP".

Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Klemens Nanni-2
In reply to this post by Stefan Sperling-5
WANTLIB and USE_GMAKE have a space after "=".
Please break LIB_DEPENDS into one port per line.
You can set SEPARATE_BUILD.

As per the project's README, `--enable-debug' should be passed for extra
"debugging code", so you could add that to CONFIGURE_ARGS if DEBUG is
set in case anyone cares to hack on using this port.

On Wed, Mar 06, 2019 at 11:55:17AM +0100, Stefan Sperling wrote:
> 1) Add more patches, and add comments to all patches.
Thanks, appreciated.

>    In particular, fix poll(2) event mask which resulted in unhandled
>    POLLHUP events in the main event loop which made textsuggest-server
>    spin on the CPU. As far as I understand, the "unlock" file descriptors
>    are only interested in POLLIN events. They can be triggered to wake the
>    event loop while a thread sleeps in poll(2). This feature is used by the
>    library's teardown code. POLLOUT makes no sense here.
>    textsuggest uses much less CPU becomes a lot more responsive with this fix.
Do you intend to push those upstream?

> 2) Enable glib integration. This adds another shared library which is not
>    used by textsuggest but will perhaps be useful if other consumers of
>    libdbus-c++ show up.
>
> 3) Run regression test (there's just one, the others are just empty stubs).

        $ make test
        ===>  Regression tests for libdbus-c++-0.9.0
        /usr/ports/pobj/libdbus-c++-0.9.0/libdbus-c++-0.9.0/test/functional/Test1/TestApp
        initialize DBus...
        terminating with uncaught exception of type DBus::Error: /usr/local/bin/dbus-launch terminated abnormally with the following error: No protocol specified
        Autolaunch error: X11 initialization failed.

        *** Signal SIGABRT in . (Makefile:31 'do-test')

> Still OK?
Assuming tests are an error on my site, OK kn.

Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stuart Henderson
On 2019/03/12 20:44, Klemens Nanni wrote:
> WANTLIB and USE_GMAKE have a space after "=".
> Please break LIB_DEPENDS into one port per line.

+1

It gained a BUILD_DEPENDS on gtk2mm, but it's only actually used in
one of the examples (disabled via CONFIGURE_ARGS) and they don't actually
build (for another reason) if you enable them.

I think I would drop the gtk2mm dependency; as-is, if it changes to being
a library dependency rather than just a build dependency in the future,
it wouldn't be noticed in bulk builds.


> You can set SEPARATE_BUILD.
>
> As per the project's README, `--enable-debug' should be passed for extra
> "debugging code", so you could add that to CONFIGURE_ARGS if DEBUG is
> set in case anyone cares to hack on using this port.
>
> On Wed, Mar 06, 2019 at 11:55:17AM +0100, Stefan Sperling wrote:
> > 1) Add more patches, and add comments to all patches.
> Thanks, appreciated.
>
> >    In particular, fix poll(2) event mask which resulted in unhandled
> >    POLLHUP events in the main event loop which made textsuggest-server
> >    spin on the CPU. As far as I understand, the "unlock" file descriptors
> >    are only interested in POLLIN events. They can be triggered to wake the
> >    event loop while a thread sleeps in poll(2). This feature is used by the
> >    library's teardown code. POLLOUT makes no sense here.
> >    textsuggest uses much less CPU becomes a lot more responsive with this fix.
> Do you intend to push those upstream?
>
> > 2) Enable glib integration. This adds another shared library which is not
> >    used by textsuggest but will perhaps be useful if other consumers of
> >    libdbus-c++ show up.
> >
> > 3) Run regression test (there's just one, the others are just empty stubs).
>
> $ make test
> ===>  Regression tests for libdbus-c++-0.9.0
> /usr/ports/pobj/libdbus-c++-0.9.0/libdbus-c++-0.9.0/test/functional/Test1/TestApp
> initialize DBus...
> terminating with uncaught exception of type DBus::Error: /usr/local/bin/dbus-launch terminated abnormally with the following error: No protocol specified
> Autolaunch error: X11 initialization failed.
>
> *** Signal SIGABRT in . (Makefile:31 'do-test')
>
> > Still OK?
> Assuming tests are an error on my site, OK kn.
>

Tests fail with PORTS_PRIVSEP but run successfully otherwise (this is
common for most things using X in tests, we don't have a good way to fix
that so that can be ignored in this port).


Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stefan Sperling-5
In reply to this post by Klemens Nanni-2
On Tue, Mar 12, 2019 at 08:44:29PM +0100, Klemens Nanni wrote:
> On Wed, Mar 06, 2019 at 11:55:17AM +0100, Stefan Sperling wrote:
> >    In particular, fix poll(2) event mask which resulted in unhandled
> >    POLLHUP events in the main event loop which made textsuggest-server
> >    spin on the CPU. As far as I understand, the "unlock" file descriptors
> >    are only interested in POLLIN events. They can be triggered to wake the
> >    event loop while a thread sleeps in poll(2). This feature is used by the
> >    library's teardown code. POLLOUT makes no sense here.
> >    textsuggest uses much less CPU becomes a lot more responsive with this fix.
> Do you intend to push those upstream?

Yes. I just haven't yet decided whether to try to the dead project mailing
list on sourceforce or whether I should mail the authors directly :-)

Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Stefan Sperling-5
In reply to this post by Stuart Henderson
On Tue, Mar 12, 2019 at 08:56:07PM +0000, Stuart Henderson wrote:

> On 2019/03/12 20:44, Klemens Nanni wrote:
> > WANTLIB and USE_GMAKE have a space after "=".
> > Please break LIB_DEPENDS into one port per line.
>
> +1
>
> It gained a BUILD_DEPENDS on gtk2mm, but it's only actually used in
> one of the examples (disabled via CONFIGURE_ARGS) and they don't actually
> build (for another reason) if you enable them.
>
> I think I would drop the gtk2mm dependency; as-is, if it changes to being
> a library dependency rather than just a build dependency in the future,
> it wouldn't be noticed in bulk builds.
Thanks both of you.
All your suggestions should be addressed in the attached version.

The gtk2mm dependency has been dropped but the configure script is still
checking for it. I've verified that it compiles with and without the
gtk2mm package installed.
 
> > As per the project's README, `--enable-debug' should be passed for extra
> > "debugging code", so you could add that to CONFIGURE_ARGS if DEBUG is
> > set in case anyone cares to hack on using this port.

The README is lying. The configure script has no such option.

libdbus-c++.tar.gz (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: new: x11/libdbus-c++

Klemens Nanni-2
On Wed, Mar 13, 2019 at 10:58:35AM +0100, Stefan Sperling wrote:
> The gtk2mm dependency has been dropped but the configure script is still
> checking for it. I've verified that it compiles with and without the
> gtk2mm package installed.
OK kn
 
> The README is lying. The configure script has no such option.
Oh.