Bug 93194 - continuous-integration glue for dbus
Summary: continuous-integration glue for dbus
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other All
: medium enhancement
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-12-01 17:59 UTC by Simon McVittie
Modified: 2016-07-25 11:00 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Avoid -Wunused-label when compiling with libselinux but no libaudit (584 bytes, patch)
2015-12-01 18:00 UTC, Simon McVittie
Details | Splinter Review
[2/10] cmake: manual-tcp is not an automated test (1.22 KB, patch)
2015-12-01 18:00 UTC, Simon McVittie
Details | Splinter Review
[3/10] cmake: copy the systemd-activation directory too (768 bytes, patch)
2015-12-01 18:01 UTC, Simon McVittie
Details | Splinter Review
embedded tests: accept and ignore --tap argument (2.22 KB, patch)
2015-12-01 18:01 UTC, Simon McVittie
Details | Splinter Review
[05/10] cmake: run all automated tests with --tap for better diagnostics (1.34 KB, patch)
2015-12-01 18:02 UTC, Simon McVittie
Details | Splinter Review
[06/10] _dbus_test_oom_handling: allow disabling it as documented (908 bytes, patch)
2015-12-01 18:02 UTC, Simon McVittie
Details | Splinter Review
[07/10] Do not require systemd to have a service file if using it for activation (4.29 KB, patch)
2015-12-01 18:02 UTC, Simon McVittie
Details | Splinter Review
[08/10] Fix shell syntax for installcheck-local with no DESTDIR (869 bytes, patch)
2015-12-01 18:03 UTC, Simon McVittie
Details | Splinter Review
[09/10] Don't try to run manual tests in installcheck (764 bytes, patch)
2015-12-01 18:03 UTC, Simon McVittie
Details | Splinter Review
[10/10] add travis-ci.org build machinery (8.67 KB, patch)
2015-12-01 18:04 UTC, Simon McVittie
Details | Splinter Review
add travis-ci.org build machinery (8.72 KB, patch)
2015-12-01 22:27 UTC, Ralf Habacker
Details | Splinter Review
add travis-ci.org build machinery (8.77 KB, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
Don't try to run manual tests in installcheck (869 bytes, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
Fix shell syntax for installcheck-local with no DESTDIR (974 bytes, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
Do not require systemd to have a service file if using it for activation (4.39 KB, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
_dbus_test_oom_handling: allow disabling it as documented (1012 bytes, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
cmake: run all automated tests with --tap for better diagnostics (1.44 KB, patch)
2015-12-02 00:07 UTC, Ralf Habacker
Details | Splinter Review
embedded tests: accept and ignore --tap argument (2.33 KB, patch)
2015-12-02 00:08 UTC, Ralf Habacker
Details | Splinter Review
cmake: copy the systemd-activation directory too (873 bytes, patch)
2015-12-02 00:08 UTC, Ralf Habacker
Details | Splinter Review
cmake: manual-tcp is not an automated test (1.32 KB, patch)
2015-12-02 00:08 UTC, Ralf Habacker
Details | Splinter Review
Avoid -Wunused-label when compiling with libselinux but no libaudit (688 bytes, patch)
2015-12-02 00:08 UTC, Ralf Habacker
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2015-12-01 17:59:43 UTC
dbus sometimes fails to build in unusual configurations (for example when cross-compiling, or if you have libapparmor but no libaudit). I have an ad-hoc multiple-build setup on my laptop, (ab)using <https://myrepos.branchable.com/>, which sometimes catches this sort of thing, but we should be able to do this better and more automatically.

We also don't routinely run the integration tests as root, which is needed for full coverage (although Ubuntu do), which led to Bug #93036 going unnoticed.

The specific CI system that I've been experimenting with as a solution to this is travis-ci.org, which uses disposable Ubuntu 14.04 LTS virtual machines for testing, and is free for open source projects on GitHub. I've uploaded an unofficial GitHub mirror of dbus <https://github.com/smcv/dbus-mirror> which gets tested at <https://travis-ci.org/smcv/dbus-mirror/branches>. The tests have full sudo access to the VM, so we can (and do) run tests that require root.

If we merge this upstream, anyone who wants to use travis-ci can put a fork on GitHub, set it up on travis-ci in a few clicks (the only special setting via the web-UI is "only build branches with a .travis.yml"), and push their branches there to see whether they compile.

I've separated the test scripts into a script that does not assume travis-ci (and so could be recycled for any other CI environments we want to try), and a simple .travis.yml that is Travis-specific.

To make this work I also had to fix some build issues. None of them really matter for anything except testing, so at least initially, my plan is to push this to master only; however, I might start a dbus-1.10-ci branch with backports of these commits.
Comment 1 Simon McVittie 2015-12-01 18:00:24 UTC
Created attachment 120226 [details] [review]
Avoid -Wunused-label when compiling with libselinux but  no libaudit
Comment 2 Simon McVittie 2015-12-01 18:00:43 UTC
Created attachment 120227 [details] [review]
[2/10] cmake: manual-tcp is not an automated test

Don't run it when we run automated tests.
Comment 3 Simon McVittie 2015-12-01 18:01:27 UTC
Created attachment 120228 [details] [review]
[3/10] cmake: copy the systemd-activation directory too

It is needed for a couple of test-cases.
Comment 4 Simon McVittie 2015-12-01 18:01:48 UTC
Created attachment 120229 [details] [review]
embedded tests: accept and ignore --tap argument

This makes them semi-command-line-compatible with a way we can
invoke the GLib-based tests to get more useful debug logs.
 
These tests still do not actually produce TAP output yet; I tried
implementing that, but it requires changing a lot of noise on stdout
to come out of stderr, and there was something weird going on with
subprocesses restarting the test numbering which will need further
investigation before making that change.
Comment 5 Simon McVittie 2015-12-01 18:02:03 UTC
Created attachment 120230 [details] [review]
[05/10] cmake: run all automated tests with --tap for better  diagnostics
Comment 6 Simon McVittie 2015-12-01 18:02:23 UTC
Created attachment 120231 [details] [review]
[06/10] _dbus_test_oom_handling: allow disabling it as  documented

We document DBUS_TEST_MALLOC_FAILURES=0 in HACKING, but it didn't
actually work: we'd iterate from i=-1 to i=0.
Comment 7 Simon McVittie 2015-12-01 18:02:59 UTC
Created attachment 120232 [details] [review]
[07/10] Do not require systemd to have a service file if using  it for activation

With --systemd-activation we special-case the name
org.freedesktop.systemd1 by assuming that it will eventually connect
to the bus. With that in mind, we can ignore whether it has a
.service file, and let it be "activated" regardless.
 
This fixes a regression test failure on non-systemd systems such
as the Ubuntu 14.04 OS on travis-ci.org: UpdateActivationEnvironment
failed, because it tried to update the (fake) systemd environment,
but because systemd was not actually installed, there was no
service file for it in the system's search paths. We could address this
by placing a dummy service file with Exec=/bin/false in our search path
like the real systemd does, but it seems cleaner to not require this;
this would eventually enable the real systemd to stop installing
that dummy service file.

This would not happen outside the regression tests, because there is
no sense in using --systemd-activation without systemd installed.
Comment 8 Simon McVittie 2015-12-01 18:03:19 UTC
Created attachment 120233 [details] [review]
[08/10] Fix shell syntax for installcheck-local with no DESTDIR

A closing brace must be preceded by a semicolon. The CI integration
added later in this branch actually runs "make installcheck"
with no DESTDIR; apparently nobody else has ever tried that.
Comment 9 Simon McVittie 2015-12-01 18:03:47 UTC
Created attachment 120234 [details] [review]
[09/10] Don't try to run manual tests in installcheck

We were mistakenly running all installed executables, even manual tests
that never terminate.
Comment 10 Simon McVittie 2015-12-01 18:04:10 UTC
Created attachment 120235 [details] [review]
[10/10] add travis-ci.org build machinery

The idea is that .travis.yml is specific to Travis-CI, but most of the
actual work is done in tools/ci-build.sh, which should be reasonably
CI-platform-agnostic (it currently assumes that build-dependendencies are
preinstalled, that the "native" platform we're building on is GNU/Linux
or something very close, and that "mingw" means mingw-w64 as packaged
in Debian and Ubuntu).
Comment 11 Ralf Habacker 2015-12-01 20:06:41 UTC
Comment on attachment 120226 [details] [review]
Avoid -Wunused-label when compiling with libselinux but  no libaudit

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

looks good
Comment 12 Ralf Habacker 2015-12-01 20:06:57 UTC
Comment on attachment 120227 [details] [review]
[2/10] cmake: manual-tcp is not an automated test

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

looks good
Comment 13 Ralf Habacker 2015-12-01 20:10:04 UTC
Comment on attachment 120228 [details] [review]
[3/10] cmake: copy the systemd-activation directory too

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

looks good
Comment 14 Ralf Habacker 2015-12-01 20:14:19 UTC
Comment on attachment 120230 [details] [review]
[05/10] cmake: run all automated tests with --tap for better  diagnostics

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

looks good
Comment 15 Ralf Habacker 2015-12-01 20:21:10 UTC
Comment on attachment 120233 [details] [review]
[08/10] Fix shell syntax for installcheck-local with no DESTDIR

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

looks good
Comment 16 Ralf Habacker 2015-12-01 20:23:46 UTC
Comment on attachment 120234 [details] [review]
[09/10] Don't try to run manual tests in installcheck

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

looks good
Comment 17 Ralf Habacker 2015-12-01 20:33:12 UTC
Comment on attachment 120229 [details] [review]
embedded tests: accept and ignore --tap argument

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

looks good
Comment 18 Ralf Habacker 2015-12-01 20:47:48 UTC
Comment on attachment 120235 [details] [review]
[10/10] add travis-ci.org build machinery

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

Looks good 

Note: x86_64 mingw target is missing

Question: How are these builds triggered, on git HEAD commit hash change ?

::: .travis.yml
@@ +43,5 @@
> +  - dbus_ci_buildsys=cmake
> +  - dbus_ci_host=mingw
> +  - dbus_ci_host=mingw dbus_ci_buildsys=cmake
> +
> +# vim:set sw=2 sts=2 et:

I do not know exactly this syntax, but it looks reasonally.

::: cmake/i686-w64-mingw32.cmake
@@ +16,5 @@
> +set(CMAKE_RC_COMPILER ${COMPILER_PREFIX}windres)
> +
> +set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
> +set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
> +set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

Looks like a minimal setting, obs uses more variables (search %_mingw32_cmake at https://build.opensuse.org/package/view_file/windows:mingw:win32/mingw32-filesystem/macros.mingw32?expand=1)

::: tools/ci-build.sh
@@ +20,5 @@
> +fi
> +
> +dbus_test=yes
> +dbus_test_fatal=yes
> +

if test "$dbus_ci_buildsys" == "autotools"; then

@@ +21,5 @@
> +
> +dbus_test=yes
> +dbus_test_fatal=yes
> +
> +NOCONFIGURE=1 ./autogen.sh

fi

@@ +127,5 @@
> +				set "$@" LDFLAGS=-L"${mingw}/lib"
> +				set "$@" CPPFLAGS=-I"${mingw}/include"
> +				set "$@" CFLAGS=-static-libgcc
> +				set "$@" CXXFLAGS=-static-libgcc
> +				# don't run tests yet, Wine needs Xvfb and

You also need a wine prefix. on obs I use this https://build.opensuse.org/package/show/home:rhabacker:branches:windows:mingw:win32/mingw32-cross-wine

@@ +182,5 @@
> +				set "$@" -D CMAKE_LIBRARY_PATH="${mingw}/lib"
> +				set "$@" -D EXPAT_LIBRARY="${mingw}/lib/libexpat.dll.a"
> +				set "$@" -D GLIB2_LIBRARIES="${mingw}/lib/libglib-2.0.dll.a"
> +				set "$@" -D GOBJECT_LIBRARIES="${mingw}/lib/libgobject-2.0.dll.a"
> +				set "$@" -D GIO_LIBRARIES="${mingw}/lib/libgio-2.0.dll.a"

obs _mingw32_cmake macro defines some more variables, see https://build.opensuse.org/package/view_file/windows:mingw:win32/mingw32-filesystem/macros.mingw32?expand=1 which let cmake be able to find libraries by itself if present.
Comment 19 Ralf Habacker 2015-12-01 20:54:47 UTC
Comment on attachment 120231 [details] [review]
[06/10] _dbus_test_oom_handling: allow disabling it as  documented

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

looks good
Comment 20 Ralf Habacker 2015-12-01 21:02:13 UTC
Comment on attachment 120232 [details] [review]
[07/10] Do not require systemd to have a service file if using  it for activation

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

Looks good as far as I can say (I am not a systemd expert)

::: bus/activation.c
@@ +1429,5 @@
>              {
>                BusPendingActivation *p = _dbus_hash_iter_get_value (&iter);
>  
> +              if (p != pending_activation && p->exec != NULL &&
> +                  strcmp (p->exec, pending_activation->exec) == 0)

make sense to avoid crashes on strcmp.

@@ +1879,2 @@
>          {
> +          pending_activation->exec = _dbus_strdup (entry->exec);

no need to check entry->exec != 0 similar to line 1432 ?
Comment 21 Ralf Habacker 2015-12-01 22:27:05 UTC
Created attachment 120245 [details] [review]
add travis-ci.org build machinery

The idea is that .travis.yml is specific to Travis-CI, but most of the
actual work is done in tools/ci-build.sh, which should be reasonably
CI-platform-agnostic (it currently assumes that build-dependendencies are
preinstalled, that the "native" platform we're building on is GNU/Linux
or something very close, and that "mingw" means mingw-w64 as packaged
in Debian and Ubuntu).

Bug:
Comment 22 Ralf Habacker 2015-12-02 00:06:46 UTC
The following fixes have been pushed:
072d3a6 Don't try to run manual tests in installcheck
81f48a9 Fix shell syntax for installcheck-local with no DESTDIR
1b1d251 _dbus_test_oom_handling: allow disabling it as documented
9c3034f cmake: run all automated tests with --tap for better diagnostics
b210934 embedded tests: accept and ignore --tap argument
4ddf4be cmake: copy the systemd-activation directory too
90a5da3 cmake: manual-tcp is not an automated test
3294b7a Avoid -Wunused-label when compiling with libselinux but no libaudit
Comment 23 Ralf Habacker 2015-12-02 00:07:26 UTC
Created attachment 120251 [details] [review]
add travis-ci.org build machinery

The idea is that .travis.yml is specific to Travis-CI, but most of the
actual work is done in tools/ci-build.sh, which should be reasonably
CI-platform-agnostic (it currently assumes that build-dependendencies are
preinstalled, that the "native" platform we're building on is GNU/Linux
or something very close, and that "mingw" means mingw-w64 as packaged
in Debian and Ubuntu).

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 24 Ralf Habacker 2015-12-02 00:07:31 UTC
Created attachment 120252 [details] [review]
Don't try to run manual tests in installcheck

We were mistakenly running all installed executables, even manual tests
that never terminate.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 25 Ralf Habacker 2015-12-02 00:07:37 UTC
Created attachment 120253 [details] [review]
Fix shell syntax for installcheck-local with no DESTDIR

A closing brace must be preceded by a semicolon. The CI integration
added later in this branch actually runs "make installcheck"
with no DESTDIR; apparently nobody else has ever tried that.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 26 Ralf Habacker 2015-12-02 00:07:43 UTC
Created attachment 120254 [details] [review]
Do not require systemd to have a service file if using it for activation

With --systemd-activation we special-case the name
org.freedesktop.systemd1 by assuming that it will eventually connect
to the bus. With that in mind, we can ignore whether it has a
.service file, and let it be "activated" regardless.

This fixes a regression test failure on non-systemd systems such
as the Ubuntu 14.04 OS on travis-ci.org: UpdateActivationEnvironment
failed, because it tried to update the (fake) systemd environment,
but because systemd was not actually installed, there was no
service file for it in the system's search paths. We could address this
by placing a dummy service file with Exec=/bin/false in our search path
like the real systemd does, but it seems cleaner to not require this;
this would eventually enable the real systemd to stop installing
that dummy service file.

This would not happen outside the regression tests, because there is
no sense in using --systemd-activation without systemd installed.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 27 Ralf Habacker 2015-12-02 00:07:49 UTC
Created attachment 120255 [details] [review]
_dbus_test_oom_handling: allow disabling it as documented

We documented DBUS_TEST_MALLOC_FAILURES=0 in HACKING, but it didn't
actually work: we'd iterate from i=-1 to i=0.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 28 Ralf Habacker 2015-12-02 00:07:55 UTC
Created attachment 120256 [details] [review]
cmake: run all automated tests with --tap for better diagnostics

For GLib-based tests it's useful, because it means g_test_message()
gets logged. For the embedded tests it's now accepted and ignored.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 29 Ralf Habacker 2015-12-02 00:08:01 UTC
Created attachment 120257 [details] [review]
embedded tests: accept and ignore --tap argument

This makes them semi-command-line-compatible with a way we can
invoke the GLib-based tests to get more useful debug logs.

These tests still do not actually produce TAP output yet; I tried
implementing that, but it requires changing a lot of noise on stdout
to come out of stderr, and there was something weird going on with
subprocesses restarting the test numbering which will need further
investigation before making that change.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 30 Ralf Habacker 2015-12-02 00:08:08 UTC
Created attachment 120258 [details] [review]
cmake: copy the systemd-activation directory too

It is needed for a couple of test-cases.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 31 Ralf Habacker 2015-12-02 00:08:13 UTC
Created attachment 120259 [details] [review]
cmake: manual-tcp is not an automated test

Don't run it when we run automated tests.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 32 Ralf Habacker 2015-12-02 00:08:20 UTC
Created attachment 120260 [details] [review]
Avoid -Wunused-label when compiling with libselinux but no libaudit

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 33 Ralf Habacker 2015-12-02 00:22:01 UTC
(In reply to Ralf Habacker from comment #22)
> The following fixes have been pushed:

This came from git bz push, which is good. Also all related patches are made obsolate, which is also good. Unfortunally git bz push also reattaches all patches to this bug, which is undocumented and bad :-(  

From http://git.fishsoup.net/man/git-bz.html:

"...
git bz push
This does git push, adds a comment that the commits were pushed and marks the patches committed. The changes it is making to the bug will be shown in your editor to give you a chance to confirm them and add extra comments if desired.
..."

It is also good, that I can make single patches obsolate by uncommenting the related line. Unfortunally the same does not work on pushing. I tried to exclude patch 7 and 10 because of issues.

That means additional fixes needs to be committed as followup.
Comment 34 Ralf Habacker 2015-12-02 00:35:21 UTC
(In reply to Ralf Habacker from comment #33)

> Unfortunally git bz push also reattaches all patches to this bug,

This "feature" has been added later http://blog.fishsoup.net/2009/09/05/git-bz-push/

" git bz push --fix 43215

This combines ‘git bz attach’ and ‘git bz push’ into one. So, to review, what it does is:

    Rewrites the commit to include the bug URL
    Attaches the commit to the bug, and marks it ‘committed’
    Pushes the commit
    Adds a comment to the bug and mark it resolved
"
Comment 35 Simon McVittie 2015-12-02 10:47:22 UTC
(In reply to Ralf Habacker from comment #18)
> Note: x86_64 mingw target is missing

Yes. I don't want to introduce too many parallel builds right now, and a 32-bit Windows build seems higher priority to test than 64-bit.

I'll consider 

> Question: How are these builds triggered, on git HEAD commit hash change ?

On any push (or pull request, but please don't send me pull requests) to smcv/dbus-mirror, or to any other clone of dbus.git on GitHub that has opted in to travis-ci integration.

> Looks like a minimal setting, obs uses more variables

Right, what I'm interested in here is the minimum to get a working build.

> > +				# don't run tests yet, Wine needs Xvfb
> 
> You also need a wine prefix. on obs I use this
> https://build.opensuse.org/package/show/home:rhabacker:branches:windows:
> mingw:win32/mingw32-cross-wine

I'm not adding tests under Wine right now.

(In reply to Ralf Habacker from comment #20)
> > +          pending_activation->exec = _dbus_strdup (entry->exec);
> 
> no need to check entry->exec != 0 similar to line 1432 ?

No. A valid BusActivationEntry (essentially a struct representing a .service file) always has exec != NULL.

The change made by this commit is to BusPendingActivation, the struct representing an activatable service that we are about to start, or that we have started but are still waiting for it to claim its bus name. The change is:

* previously, a BusPendingActivation was always created for a non-NULL
  BusActivationEntry, therefore it had a non-NULL exec
* now, the BusPendingActivation for "org.freedesktop.systemd1" is created
  without a BusActivationEntry, so it has a NULL exec
Comment 36 Simon McVittie 2015-12-02 10:51:57 UTC
(In reply to Ralf Habacker from comment #22)
> The following fixes have been pushed:
> 072d3a6 Don't try to run manual tests in installcheck
> 81f48a9 Fix shell syntax for installcheck-local with no DESTDIR
> 1b1d251 _dbus_test_oom_handling: allow disabling it as documented
> 9c3034f cmake: run all automated tests with --tap for better diagnostics
> b210934 embedded tests: accept and ignore --tap argument
> 4ddf4be cmake: copy the systemd-activation directory too
> 90a5da3 cmake: manual-tcp is not an automated test
> 3294b7a Avoid -Wunused-label when compiling with libselinux but no libaudit

In general I would prefer to push my own changes after review, particularly if you queried things during review and I have not yet responded.

If you review a self-contained bug fix that I've written, and you have no queries or objections, it's OK to just push it, particularly if it's fixing something that was breaking the build; but for "features" originating from a maintainer, I'd really prefer for the author of the branch to be the one to confirm that it's ready and push it.
Comment 37 Philip Withnall 2015-12-02 11:38:56 UTC
(In reply to Simon McVittie from comment #35)
> > Question: How are these builds triggered, on git HEAD commit hash change ?
> 
> On any push (or pull request, but please don't send me pull requests) to
> smcv/dbus-mirror, or to any other clone of dbus.git on GitHub that has opted
> in to travis-ci integration.

How are you updating the GitHub mirror? A cronjob somewhere which pushes to it?

You might want to consider using phabricator.freedesktop.org for this — it has an automatic git repository mirroring function which pushes to remote mirrors of any repository which you’ve added to Phabricator (if you ask it to).
Comment 38 Simon McVittie 2015-12-02 12:06:31 UTC
(In reply to Philip Withnall from comment #37)
> How are you updating the GitHub mirror? A cronjob somewhere which pushes to
> it?

At the moment, by hand, which is better than nothing.

> You might want to consider using phabricator.freedesktop.org for this — it
> has an automatic git repository mirroring function which pushes to remote
> mirrors of any repository which you’ve added to Phabricator (if you ask it
> to).

If I understand correctly, that would require me to give phabricator.freedesktop.org control over my GitHub account, which I certainly don't want. Or does Phab already have its own GitHub account which I can add as a collaborator?
Comment 39 Philip Withnall 2015-12-02 12:30:39 UTC
(In reply to Simon McVittie from comment #38)
> (In reply to Philip Withnall from comment #37)
> > How are you updating the GitHub mirror? A cronjob somewhere which pushes to
> > it?
> 
> At the moment, by hand, which is better than nothing.
> 
> > You might want to consider using phabricator.freedesktop.org for this — it
> > has an automatic git repository mirroring function which pushes to remote
> > mirrors of any repository which you’ve added to Phabricator (if you ask it
> > to).
> 
> If I understand correctly, that would require me to give
> phabricator.freedesktop.org control over my GitHub account, which I
> certainly don't want. Or does Phab already have its own GitHub account which
> I can add as a collaborator?

You can add a ‘deploy key’ to your project in GitHub, which is a separate SSH key for pushing to the repository. Give that SSH key to Phabricator for mirroring.
Comment 40 Ralf Habacker 2015-12-04 10:52:43 UTC
(In reply to Philip Withnall from comment #39)
> (In reply to Simon McVittie from comment #38)

> to Phabricator for mirroring.

So it's time to open dbus project on phabricator.freedesktop.org ?
Comment 41 Philip Withnall 2015-12-04 12:09:45 UTC
(In reply to Ralf Habacker from comment #40)
> (In reply to Philip Withnall from comment #39)
> > (In reply to Simon McVittie from comment #38)
> 
> > to Phabricator for mirroring.
> 
> So it's time to open dbus project on phabricator.freedesktop.org ?

Iff you want to use the mirroring feature of Phabricator.
Comment 42 Ralf Habacker 2015-12-04 12:56:51 UTC
(In reply to Philip Withnall from comment #41)
> (In reply to Ralf Habacker from comment #40)
> > (In reply to Philip Withnall from comment #39)
> > > (In reply to Simon McVittie from comment #38)
> > 
> > > to Phabricator for mirroring.
> > 
> > So it's time to open dbus project on phabricator.freedesktop.org ?
> 
> Iff you want to use the mirroring feature of Phabricator.

Yes, I created a related project https://phabricator.freedesktop.org/project/profile/67/
Unfortunally git repo setup is limited to site admins.:-(

Creating repo at https://phabricator.freedesktop.org/diffusion/ returns: 

You do not have permission to create new repositories.
Comment 43 Simon McVittie 2015-12-04 13:08:03 UTC
(In reply to Ralf Habacker from comment #42)
> Creating repo at https://phabricator.freedesktop.org/diffusion/ returns: 
> 
> You do not have permission to create new repositories.

Right. We would have to discuss this with the freedesktop.org sysadmins.

Please don't create any more "official-looking" infrastructure without some discussion on the mailing list first: if we do this, I want to do it right. The stuff on GitHub and travis-ci is deliberately under my personal namespace, and marked as unofficial and subject to change/breakage; if we create things on fd.o infrastructure in particular, they're going to look "official" and people will start using them.

(In particular, I really don't want to get into a situation where some people open D-Bus bugs as Bugzilla bugs and some people open them as Phabricator tasks!)
Comment 44 Philip Withnall 2015-12-04 14:28:40 UTC
(In reply to Simon McVittie from comment #43)
> (In reply to Ralf Habacker from comment #42)
> > Creating repo at https://phabricator.freedesktop.org/diffusion/ returns: 
> > 
> > You do not have permission to create new repositories.
> 
> Right. We would have to discuss this with the freedesktop.org sysadmins.

.o/  I have those powers on phab.fdo.

> Please don't create any more "official-looking" infrastructure without some
> discussion on the mailing list first: if we do this, I want to do it right.
> The stuff on GitHub and travis-ci is deliberately under my personal
> namespace, and marked as unofficial and subject to change/breakage; if we
> create things on fd.o infrastructure in particular, they're going to look
> "official" and people will start using them.
> 
> (In particular, I really don't want to get into a situation where some
> people open D-Bus bugs as Bugzilla bugs and some people open them as
> Phabricator tasks!)

Definitely. For libnice, we have the problem at the moment that someone will report a bug via the mailing list. Then they’ll find the GitHub and fork that and send a pull request. Then they’ll find Phabricator and create a task and diff there too. It’s a pain.
Comment 45 Ralf Habacker 2015-12-04 18:04:08 UTC
(In reply to Simon McVittie from comment #43)
> (In reply to Ralf Habacker from comment #42)
> > Creating repo at https://phabricator.freedesktop.org/diffusion/ returns: 
> > 
> > You do not have permission to create new repositories.
> 
> Right. We would have to discuss this with the freedesktop.org sysadmins.
> 
> Please don't create any more "official-looking" infrastructure without some
> discussion on the mailing list first: if we do this, I want to do it right.

You did take a look at the dbus project on phabricator.freedesktop.org  ? It is not public, noone can join and noone except the current members  (which are  you, Philip and I) can create tasks assigned to dbus  :-)
Comment 46 Ralf Habacker 2015-12-07 13:42:52 UTC
(In reply to Simon McVittie from comment #43)
> 
> Right. We would have to discuss 
I filed a related bug at 93283.
Comment 47 Ralf Habacker 2015-12-07 13:44:22 UTC
(In reply to Ralf Habacker from comment #46)
> (In reply to Simon McVittie from comment #43)
> > 
> > Right. We would have to discuss 
> I filed a related bug at 93283.
bug 93283
Comment 48 Simon McVittie 2016-06-30 15:59:47 UTC
This landed a while ago. For dbus 1.10.x, the CI glue is on a separate branch (dbus-1.10-ci), although I'm increasingly tempted to just merge it.
Comment 49 Simon McVittie 2016-07-25 11:00:15 UTC
(In reply to Simon McVittie from comment #48)
> For dbus 1.10.x, the CI glue is on a separate
> branch (dbus-1.10-ci), although I'm increasingly tempted to just merge it.

This CI glue is sufficiently useful that I think we should bend the stable-branch policy enough to include it, so I've merged dbus-1.10-ci into dbus-1.10 (currently only on Github due to Bug #97014).

The only "real" code change (not in build or test code) is Attachment #120232 [details]. I think that's minimal enough that we can just ship it.


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.