Bug 71100

Summary: merge telepathy-logger into telepathy-glib
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: tp-glibAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: enhancement    
Priority: medium Keywords: patch
Version: git master   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: [tp-glib next 1/5] telepathy-glib: include dependencies in uninstalled .pc files
[tp-glib next 2/5] Bring back with-session-bus.sh
[tp-glib next 4/5] Require Automake 1.12 and force use of the parallel tests driver
[tp-glib next 5/5] Put Autotools cruft in /build-aux
[logger next 01/17] autogen.sh: don't override Automake version
[logger next 02/17] Fix out-of-tree documentation building
[logger next 03/17] Use the Automake "parallel-tests" driver, albeit in non-parallel mode
[logger next 04/17] Put Autoconf, Automake, Libtool droppings in /build-aux
[logger next 05/17] Distribute debug.h, fixing distcheck
[logger next 06/17] Use AC_PROG_MKDIR_P, MKDIR_P instead of deprecated AM_PROG_MKDIR_P, mkdir_p
[logger next 07/17] Use subdir-objects (compile a/b.c to a/b.lo, not ./b.lo)
[logger next 09/17] Replace deprecated INCLUDES with AM_CPPFLAGS
[logger next 10/17] Avoid GLib 2.32, 2.34 deprecations
[logger next 11/17] Write in C89, like telepathy-glib
[logger next 15/17] Sync tools from telepathy-glib next
[after merge 1/7] Don't remove _gen in clean, only in distclean
[after merge 2/7] fix GObject-Introspection scanning
[after merge 3/7] fix some missing documentation
[after merge 4/7] distribute Sidecars1 XML
[after merge 5/7] logger: don't change ABI with --disable-debug
[after merge 6/7] Use a less archaic format for tarballs
[after merge 7/7] Always enable compile-time debug messages
dist-hook: omit changes before tp-glib 0.22 or tp-logger 0.8

Description Simon McVittie 2013-10-31 18:12:12 UTC
The Telepathy logger seems a good candidate for merging into telepathy-glib 'next'. If it works well, we can do the same for Telepathy-Farstream, and maybe the spec and Mission Control.

This doesn't directly affect Telepathy-Qt, as far as I can see: the dependency tree is still

    Telepathy-Qt -> spec

    Telepathy-Logger-Qt -> telepathy-logger -> telepathy-glib -> spec
Comment 1 Simon McVittie 2013-10-31 18:13:35 UTC
Created attachment 88426 [details] [review]
[tp-glib next 1/5] telepathy-glib: include dependencies in uninstalled .pc  files
Comment 2 Simon McVittie 2013-10-31 18:14:08 UTC
Created attachment 88427 [details] [review]
[tp-glib next 2/5] Bring back with-session-bus.sh

telepathy-logger's installed tests still need it.
Comment 3 Simon McVittie 2013-10-31 18:15:38 UTC
[telepathy-glib next] Move docs/reference to docs/reference/telepathy-glib

This will let us merge other projects into this source tree.

---

Not attaching this one because it's huge, and benefits a *lot* from `git diff -M`. http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=beforemerge&id=8d7c3206ff2003e8095b8da357ab009011a2b2f1
Comment 4 Simon McVittie 2013-10-31 18:16:35 UTC
Created attachment 88428 [details] [review]
[tp-glib next 4/5] Require Automake 1.12 and force use of the parallel tests  driver

This means we can use the newly-recommended AM_TESTS_ENVIRONMENT
for the environment (TESTS_ENVIRONMENT is now reserved for the user,
like CFLAGS), and LOG_COMPILER for the executable that will wrap
the logs.

We also no longer need test-wrapper.sh for the tests, because
Automake has similar functionality built-in (on a buildbot or whatever,
use "make check VERBOSE=1" to cat the logs automatically).

Also use subdir-objects, to shut Automake 1.14 up.

---

I kept test-wrapper.sh in the tree, because we do use it for installed tests.
Comment 5 Simon McVittie 2013-10-31 18:16:56 UTC
Created attachment 88429 [details] [review]
[tp-glib next 5/5] Put Autotools cruft in /build-aux
Comment 6 Simon McVittie 2013-10-31 18:18:08 UTC
Created attachment 88430 [details] [review]
[logger next 01/17] autogen.sh: don't override Automake version

We now require 1.12, so explicitly selecting 1.11 makes no sense.
Comment 7 Simon McVittie 2013-10-31 18:18:21 UTC
Created attachment 88431 [details] [review]
[logger next 02/17] Fix out-of-tree documentation building
Comment 8 Simon McVittie 2013-10-31 18:18:46 UTC
Created attachment 88432 [details] [review]
[logger next 03/17] Use the Automake "parallel-tests" driver, albeit in  non-parallel mode

This seems perverse, but it means we invoke tests in the same way
I want to use for other Telepathy projects, which will help when
we want to merge their source trees together.

Now that we can rely on having the parallel-tests setup, we can use
AM_TESTS_ENVIRONMENT instead of TESTS_ENVIRONMENT (which is now
reserved for the user, like CFLAGS), and we must use LOG_COMPILER
rather than TESTS_ENVIRONMENT for "adverb" command prefixes.

We no longer need to use test-wrapper.sh for the tests, because
Automake has similar functionality built-in. On a buildbot
or similar, use "make check VERBOSE=1" to cat the logs automatically.
Comment 9 Simon McVittie 2013-10-31 18:19:03 UTC
Created attachment 88433 [details] [review]
[logger next 04/17] Put Autoconf, Automake, Libtool droppings in /build-aux

Equivalent to MC commit c3cba93.
Comment 10 Simon McVittie 2013-10-31 18:19:21 UTC
Created attachment 88434 [details] [review]
[logger next 05/17] Distribute debug.h, fixing distcheck
Comment 11 Simon McVittie 2013-10-31 18:19:41 UTC
Created attachment 88435 [details] [review]
[logger next 06/17] Use AC_PROG_MKDIR_P, MKDIR_P instead of deprecated  AM_PROG_MKDIR_P, mkdir_p

Similar to MC commit 04dd9b4.
Comment 12 Simon McVittie 2013-10-31 18:20:01 UTC
Created attachment 88436 [details] [review]
[logger next 07/17] Use subdir-objects (compile a/b.c to a/b.lo, not  ./b.lo)

Automake 2 will make this the default, and 1.14 warns about not
using it. Equivalent to MC commit 1f11065.
Comment 13 Simon McVittie 2013-10-31 18:20:54 UTC
[logger next 08/17] Move doc/ to docs/

This is consistent with telepathy-glib.

http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/next&id=a287aef2ae7c401b082dca0dfa00e9996d69b9a1
Comment 14 Simon McVittie 2013-10-31 18:21:28 UTC
Created attachment 88437 [details] [review]
[logger next 09/17] Replace deprecated INCLUDES with AM_CPPFLAGS
Comment 15 Simon McVittie 2013-10-31 18:22:47 UTC
Created attachment 88438 [details] [review]
[logger next 10/17] Avoid GLib 2.32, 2.34 deprecations
Comment 16 Simon McVittie 2013-10-31 18:23:48 UTC
Created attachment 88439 [details] [review]
[logger next 11/17] Write in C89, like telepathy-glib
Comment 17 Simon McVittie 2013-10-31 18:25:17 UTC
[logger next 12/17] Move logger daemon into telepathy-logger/

http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/next&id=59a4e79031d9498a745596746bb19eb87e5ef3e9

[logger next 13/17] Move extensions into telepathy-logger/extensions/

That's a better place for them when we merge this into telepathy-glib.

http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/next&id=553e32c176c804f0a732ea5df42fb85a03819e65

[logger next 14/17] Move Logger tests into a subdirectory

So they don't get mixed up with the telepathy-glib tests when we merge.

http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/next&id=41e5a285fccb282ca16f780017a4ac6f4589b1cf
Comment 18 Simon McVittie 2013-10-31 18:27:06 UTC
Created attachment 88440 [details] [review]
[logger next 15/17] Sync tools from telepathy-glib next

---

So they don't add/add conflict when we merge.

> -          <a href="#im.telepathy1.Properties">Telepathy
> +          <a href="#org.freedesktop.Telepathy.Properties">Telepathy

Yes that's a strange change to make, but the entire interface is dead anyway, so, whatever. If anything, we should delete that part of the file altogether.
Comment 20 Simon McVittie 2013-10-31 18:29:07 UTC
For those who prefer cgit, those parts of the branch are:

http://cgit.freedesktop.org/~smcv/telepathy-glib/log/?h=beforemerge (since origin/next)

http://cgit.freedesktop.org/~smcv/telepathy-glib/log/?h=mylogger/next (since logger/next)
Comment 22 Simon McVittie 2013-10-31 18:32:17 UTC
Created attachment 88441 [details] [review]
[after merge 1/7] Don't remove _gen in clean, only in distclean

With subdir-objects enabled, Automake dependency-tracking files are
written in _gen/.deps, and deleting them breaks the build.
Comment 23 Simon McVittie 2013-10-31 18:32:58 UTC
Created attachment 88442 [details] [review]
[after merge 2/7] fix GObject-Introspection scanning

---

This was already broken in logger/next, I think? It was trying to use TelepathyGLib-0.12.
Comment 24 Simon McVittie 2013-10-31 18:33:15 UTC
Created attachment 88443 [details] [review]
[after merge 3/7] fix some missing documentation
Comment 25 Simon McVittie 2013-10-31 18:33:34 UTC
Created attachment 88444 [details] [review]
[after merge 4/7] distribute Sidecars1 XML

---

Un-breaks distcheck.
Comment 26 Simon McVittie 2013-10-31 18:34:58 UTC
Created attachment 88445 [details] [review]
[after merge 5/7] logger: don't change ABI with --disable-debug

Rather than having functions magically disappear, it seems much easier
to have those functions exist but do nothing.

One day I'll remove ENABLE_DEBUG altogether, but until then...

---

This also broke distcheck - a function used TplDebugFlags unconditionally, and telepathy-glib distchecks with --disable-debug to check this sort of thing. There were also a bunch of unused variables, so I decided to just burn ENABLE_DEBUG altogether.
Comment 27 Simon McVittie 2013-10-31 18:35:51 UTC
Created attachment 88446 [details] [review]
[after merge 6/7] Use a less archaic format for tarballs

---

This enables longer-than-99-characters filenames, which some logger tests need I think? Also, the documentation needs them, now it has been moved into a deeper directory.
Comment 28 Simon McVittie 2013-10-31 18:36:50 UTC
Created attachment 88447 [details] [review]
[after merge 7/7] Always enable compile-time debug messages

---

I started trying to fix unused variables under !ENABLE_DEBUG, but then I thought "life's too short" and removed ENABLE_DEBUG instead. Problem solved.
Comment 29 Guillaume Desmottes 2013-11-04 08:04:39 UTC
Comment on attachment 88426 [details] [review]
[tp-glib next 1/5] telepathy-glib: include dependencies in uninstalled .pc  files

Review of attachment 88426 [details] [review]:
-----------------------------------------------------------------

++
Comment 30 Guillaume Desmottes 2013-11-04 08:05:05 UTC
Comment on attachment 88427 [details] [review]
[tp-glib next 2/5] Bring back with-session-bus.sh

Review of attachment 88427 [details] [review]:
-----------------------------------------------------------------

++
Comment 31 Guillaume Desmottes 2013-11-04 08:13:19 UTC
(In reply to comment #3)
> [telepathy-glib next] Move docs/reference to docs/reference/telepathy-glib
> 
> This will let us merge other projects into this source tree.
> 
> ---
> 
> Not attaching this one because it's huge, and benefits a *lot* from `git
> diff -M`.
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/
> ?h=beforemerge&id=8d7c3206ff2003e8095b8da357ab009011a2b2f1

++ if it distchecks
Comment 32 Guillaume Desmottes 2013-11-04 08:16:03 UTC
Comment on attachment 88428 [details] [review]
[tp-glib next 4/5] Require Automake 1.12 and force use of the parallel tests  driver

Review of attachment 88428 [details] [review]:
-----------------------------------------------------------------

++
Comment 33 Guillaume Desmottes 2013-11-04 08:16:30 UTC
Comment on attachment 88429 [details] [review]
[tp-glib next 5/5] Put Autotools cruft in /build-aux

Review of attachment 88429 [details] [review]:
-----------------------------------------------------------------

++
Comment 34 Guillaume Desmottes 2013-11-04 08:16:58 UTC
Comment on attachment 88430 [details] [review]
[logger next 01/17] autogen.sh: don't override Automake version

Review of attachment 88430 [details] [review]:
-----------------------------------------------------------------

++
Comment 35 Guillaume Desmottes 2013-11-04 08:17:11 UTC
Comment on attachment 88431 [details] [review]
[logger next 02/17] Fix out-of-tree documentation building

Review of attachment 88431 [details] [review]:
-----------------------------------------------------------------

++
Comment 36 Guillaume Desmottes 2013-11-04 08:18:08 UTC
Comment on attachment 88432 [details] [review]
[logger next 03/17] Use the Automake "parallel-tests" driver, albeit in  non-parallel mode

Review of attachment 88432 [details] [review]:
-----------------------------------------------------------------

if tests still pass then sure.
Comment 37 Guillaume Desmottes 2013-11-04 08:18:24 UTC
Comment on attachment 88433 [details] [review]
[logger next 04/17] Put Autoconf, Automake, Libtool droppings in /build-aux

Review of attachment 88433 [details] [review]:
-----------------------------------------------------------------

++
Comment 38 Guillaume Desmottes 2013-11-04 08:18:36 UTC
Comment on attachment 88434 [details] [review]
[logger next 05/17] Distribute debug.h, fixing distcheck

Review of attachment 88434 [details] [review]:
-----------------------------------------------------------------

++
Comment 39 Guillaume Desmottes 2013-11-04 08:19:02 UTC
Comment on attachment 88435 [details] [review]
[logger next 06/17] Use AC_PROG_MKDIR_P, MKDIR_P instead of deprecated  AM_PROG_MKDIR_P, mkdir_p

Review of attachment 88435 [details] [review]:
-----------------------------------------------------------------

++
Comment 40 Guillaume Desmottes 2013-11-04 08:19:15 UTC
Comment on attachment 88436 [details] [review]
[logger next 07/17] Use subdir-objects (compile a/b.c to a/b.lo, not  ./b.lo)

Review of attachment 88436 [details] [review]:
-----------------------------------------------------------------

++
Comment 41 Guillaume Desmottes 2013-11-04 08:22:10 UTC
(In reply to comment #13)
> [logger next 08/17] Move doc/ to docs/
> 
> This is consistent with telepathy-glib.
> 
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/
> next&id=a287aef2ae7c401b082dca0dfa00e9996d69b9a1

++ if it distchecks
Comment 42 Guillaume Desmottes 2013-11-04 08:22:40 UTC
Comment on attachment 88437 [details] [review]
[logger next 09/17] Replace deprecated INCLUDES with AM_CPPFLAGS

Review of attachment 88437 [details] [review]:
-----------------------------------------------------------------

++
Comment 43 Guillaume Desmottes 2013-11-04 08:25:27 UTC
Comment on attachment 88438 [details] [review]
[logger next 10/17] Avoid GLib 2.32, 2.34 deprecations

Review of attachment 88438 [details] [review]:
-----------------------------------------------------------------

::: telepathy-logger/dbus-service.c
@@ +412,4 @@
>  
>    priv = closure->service->priv;
>  
> +  packed = g_ptr_array_new_with_free_func ((GDestroyNotify) tp_value_array_free);

++ if we are depending on tp-glib 0.23.0
Comment 44 Guillaume Desmottes 2013-11-04 08:26:09 UTC
Comment on attachment 88439 [details] [review]
[logger next 11/17] Write in C89, like telepathy-glib

Review of attachment 88439 [details] [review]:
-----------------------------------------------------------------

++
Comment 45 Guillaume Desmottes 2013-11-04 08:28:16 UTC
(In reply to comment #17)
> [logger next 12/17] Move logger daemon into telepathy-logger/
> 
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/
> next&id=59a4e79031d9498a745596746bb19eb87e5ef3e9
> 
> [logger next 13/17] Move extensions into telepathy-logger/extensions/
> 
> That's a better place for them when we merge this into telepathy-glib.
> 
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/
> next&id=553e32c176c804f0a732ea5df42fb85a03819e65
> 
> [logger next 14/17] Move Logger tests into a subdirectory
> 
> So they don't get mixed up with the telepathy-glib tests when we merge.
> 
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=mylogger/
> next&id=41e5a285fccb282ca16f780017a4ac6f4589b1cf

ok
Comment 46 Guillaume Desmottes 2013-11-04 08:28:55 UTC
Comment on attachment 88440 [details] [review]
[logger next 15/17] Sync tools from telepathy-glib next

Review of attachment 88440 [details] [review]:
-----------------------------------------------------------------

++
Comment 47 Guillaume Desmottes 2013-11-04 08:30:37 UTC
(In reply to comment #21)
> Here is the Merge Commit Of Doom:
> 
> http://cgit.freedesktop.org/~smcv/telepathy-glib/commit/?h=next-merge-
> logger&id=ae86b52c435aa39b78ae14cb23a75e4e639b0d5a

It it distchecks all good. :)
Comment 48 Simon McVittie 2013-11-04 11:32:35 UTC
Did you get as far as reviewing the 7 "after merge" commits? If not, could someone do those? A couple of them are necessary to make distcheck succeed.

I'd rebase, but then I'd have to do the Giant Merge Of Doom again...
Comment 49 Guillaume Desmottes 2013-11-04 11:40:28 UTC
(In reply to comment #48)
> Did you get as far as reviewing the 7 "after merge" commits? If not, could
> someone do those? A couple of them are necessary to make distcheck succeed.
> 
> I'd rebase, but then I'd have to do the Giant Merge Of Doom again...

I did now, go for it!
Comment 50 Simon McVittie 2013-11-04 12:07:48 UTC
Created attachment 88603 [details] [review]
dist-hook: omit changes before tp-glib 0.22 or tp-logger  0.8

We don't want to generate a 900K ChangeLog containing the logger's
entire history. "Everything since the last stable version" seems
a reasonable rule of thumb.

---

All applied, thanks! Here's one more.
Comment 51 Guillaume Desmottes 2013-11-04 13:18:45 UTC
Comment on attachment 88603 [details] [review]
dist-hook: omit changes before tp-glib 0.22 or tp-logger  0.8

Review of attachment 88603 [details] [review]:
-----------------------------------------------------------------

++
Comment 52 Simon McVittie 2013-11-05 11:43:59 UTC
Fixed in git for 0.99.5

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.