| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This implements a different way of making blocking method calls.
This approach is older than the call operator approach and
archived here in case here just in case.
|
|
Always try to create address book or calendar database, because even
if there is a source there's no guarantee that the actual database was
created already; the original logic for only setting this when
explicitly requesting a new database therefore failed in some cases.
|
|
The implementation was broken when using GDBus GIO (mismatch between
string and D-Bus object type) and probably also with GDBus libdbus
(missing boolean for GetConfig()).
Rewritten using blocking D-Bus calls, which makes the implementation
also a lot shorter and easier to understand.
It would be good to have automated tests for these features. Not done
yet.
|
|
The call operator on DBusClientCall[0123] is now blocking. It will
return result values as single value, std::pair or boost::tuple,
respectively. If an error is encountered either locally or in the
peer, a runtime_error is thrown.
This is an API change. The traditional implementation was to start an
asynchronous call. That is still possible, but has to be requested
explicitly with the new start() method.
This distinction is necessary because C++ cannot guess (easily)
whether the callback parameter is a callback type or another parameter
for the D-Bus peer. It's also a bit cleaner in the source code because
now the call operator really acts like a normal, synchronous call.
The downside of the synchronous calls is that complex return values
have to be copied more often. If that is too expensive, then use the
asynchronous start() + callback approach.
|
|
API adapted to the one used by GDBus libdbus. Actual implementation
didn't have to be changed.
|
|
When attempting to activate a SignalWatch for one-to-one connections
an error occured because activation attempted to add a match rule
using the AddMatch member of the org.freedsktop.DBus interface.
This patch adds an is_bus_conn argument to the SignalWatch constructor
which is used to forgo adding a match rule to non-bus
connections. is_bus_conn defaults to true so no existing code needs
modification.
|
|
Moved the get() call from all users of AppendRetvals into
the constructor of AppendRetvals. Shorter and makes it
easier to copy-and-paste from the libdbus bindings, where
AppendRetvals also works like that.
|
|
Assigning to GDBusMessagePtr increments the ref count. Only the
constructor steals the reference. Therefore the result of
g_dbus_message_new_method_call() must be stored in a newly created
GDBusMessagePtr to avoid leaking the message.
|
|
When the chosen helper already had an absolute path, the code
accidentally continued with an empty helper file name instead of using
the full path.
|
|
Some more tests failed due to timing issues when running under
valgrind. Increased timeouts + delays.
|
|
Somehow the auto sync code triggers a valgrind warning when
running the TestFileNotify.testRestart code. That code is
going to be rewritten, so ignore the warning for now.
|
|
Noticed that KApplication messes with signal handlers (although
not necessarily SIGINT/SIGTERM). Either way, restore the state
of those two after creating the KApplication instance.
|
|
Added debug logging to valgrindcheck.sh, suspend flags, fork/exec
and main() functions. Helps with tracing where signal handlers
are installed and which processes run.
|
|
syncevo-local-sync dies with a segfault in libdl after the
TestLocalSync.testConcurrency D-Bus tests completes, but only when
running under valgrind. Ignore that problem via a suppression.
|
|
Local syncing under valgrind can take a while. Increased timeouts.
|
|
|
|
libsyncevolution depends on GDBus, which therefore must be dealt
with first.
Mostly the dependencies take care of the right compilation order.
Installing the results is different, items of the same kind are
installed in the order in which they are added to their variables,
which in turn is defined by the order in which .am files are
included.
That broke when making libgdbussyncevo a normal lib like
libsyncevolution (see next commit).
|
|
Some backends react to the "backend" aliases
("calendar/todo/memo/addressbook"). If more than one such backend
is enabled and installed, then these aliases become ambiguous.
Used to be the case when Akonadi and EDS were both enabled.
This commit introduces an error message for that case.
In the case of Akonadi and EDS it is (temporarily) avoided
by not choosing Akonadi when only an alias is used.
|
|
When using the "backend = addressbook/calendar/todo/memo" aliases, then
the EDS backend must be the only backend that accepts that, to avoid
ambiguities and regressions for traditional usage of SyncEvolution.
These aliases are used when following the current setup instructions
or the GTK sync UI.
Even if SyncEvolution somehow figured out that "addressbook" is meant to
be the KDE addressbook (for example by looking at KDE_FULL_SESSION once
more), fully automatic configuration for KDE still wouldn't work because
the "database" property has to be set explicitly to a URI of an Akonadi
resource. There is no code in the backend or Akonadi to identify the
default resource.
|
|
The testDelete404 fails because AkonadiSyncSource::removeItem() doesn't
detect attempts to delete a non-existent item. That is only relevant for
SyncEvolution's virtual calendar+todo storage (used when syncing with
Nokia phones).
All tests involving more advanced test data fail because the test data
was written for EDS. Akonadi uses a different vCard flavor (different
extensions, for example), so different test data has to be created.
|
|
When building modules, the registation code must be part of
the module. Integration testing should always be offered by
the backend. The ifdef is only for the test driver.
|
|
There's no time for a detailed analysis of the issues
reported by valgrind in KDE/Qt libraries. Suppress
them so that nightly testing can pass again.
|
|
--trace-children was still set to "no" in the nightly testing.
Setting it to "yes" showed some deficits in valgrindcheck.sh:
- having multiple processes write into the same log file
garbles the file; now valgrindcheck.sh uses %p and checks
all resulting files one after the other
- a child process might have to be checked (for example,
to check also its child processes) without us actually
caring for the result; valgrindcheck.sh now can filter
out the common potential memory leaks if VALGRIND_LEAK_CHECK_SKIP
containts a *regex* matching the comman name
More control over which errors are ignored might be useful.
Perhaps suppression rules are a better solution (match by
executable with wild cards for stack and repeat for all errors).
|
|
readItem() might be asked to retrieve a non-existent item.
The fetch job succeeds in that case, but without returning
any item. Throw the 404 status error in this case.
|
|
Due to bitrot the Akonadi backend and KWallet support code no longer
worked. Moved the common code for KApplication initialization into
libsyncevolution's SyncContext::initMain() and fixed autotools rules.
The old code always tried to contact an X server (default constructor
of KApplication). That doesn't seem to be necessary and is avoided now.
Even better might be to skip KApplication entirely and instead use
QCoreApplication and KComponentData, as suggested by
http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKApplication.html
KAboutData was incorrectly passed the address of a string pointer, not
the pointer itself.
Testing the Akonadi backend in client-test failed because client-test
always overwrites the "backend" value with
"Test_kde_[contact/event/..]._[1/2]". Now this special case is
detected. The backend then uses the first resp. second resource that
it finds.
|
|
Instead of adding every single entry in a DBusArray separately,
use a single g_variant_new_from_data(). The advantage is
better performance (no need to realloc array multiple times,
a lot less function calls).
Also avoids some valgrind warnings about "potentially lost"
memory. Those warnings drew attention to the DBusArray append()
call, now they are gone.
|
|
Made the previous suppression for dbus_server_listen() more generic,
added suppressions for dbus_setup_server(). Covers some small
leaks in code which is obsolete and hard to fix.
|
|
libsyncevolution depends on libgdbussyncevo. On installation the
former needs to be relinked, but the latter is still not installed
and that causes linking to fail with following message:
/usr/bin/ld: cannot find -lgdbussyncevo
That is so, because the order of installing binaries is not
dependency-based but more like target directory based:
install-exec-am: install-binPROGRAMS install-binSCRIPTS \
install-libLTLIBRARIES install-libexecPROGRAMS \
install-libexecSCRIPTS install-nodist_binSCRIPTS \
install-pkglibLTLIBRARIES
libsyncevolution is installed during install-libLTLIBRARIES, while
its dependency is installed during later install-pkglibLTLIBRARIES.
|
|
The errors are minor and don't seem to be caused by SyncEvolution
(dbus_server_disconnect() is called). Will go away once we move away
from libdbus and/or when running one sync session per server (and thus
not creating multiple servers?).
|
|
Several tests (linked items, remove properties) were failing with
Apple Calendar Server because SyncEvolution didn't send a DTSTAMP
property. The server expects that and (probably correctly) rejects
VEVENTs without it. Sending it now...
|
|
Memotoo supports PERCENT-COMPLETE now. However, it sets the value to
100% if the status is completed. Therefore we can't have the 99%
complete value in the eds_task test, as for other servers.
|
|
The D-Bus server implemented suspend/abort of a session only in its
checkForSuspend/Abort() implementation my looking at the sessions
current state. The downside is that polling is necessary to detect
that state change.
This commit introduces the possibility to temporarily change the
global SuspendFlags state to abort resp. suspend. The duration of that
change is determined by the caller, which needs to keep a shared
pointer alive as long as it wants to remain in that
state. SuspendFlags itself doesn't keep the shared pointer; it merely
checks via weak pointers whether the shared ones still exist.
This approach ensures that if the caller gets destructed, the global
state automatically returns what it was before. This is necessary in
the D-Bus server because it runs one session after the other and
aborting one session should never affect another.
However, sessions stay around longer than they are active (= own the
global state). Therefore it is necessary to never modify the global
state before the session is active and restore it as soon as the
session becomes inactive again. The redundancy between session state
and global state is encapsulated in the new SyncStateOwner class which
replaces a mere enum. Now the state assignment triggers additional
code which mirrors the session's state in SuspendFlags.
It can also do additional sanity checks, like preventing a transition
from "abort" to "running". Previously, the abort or suspend request on
a not yet running session most likely was ignored by overwriting it
with SYNC_RUNNING in run().
|
|
The parent always reported a "remote transport error" when the child
didn't finish as expected. In cases where we know what the problem
was, because the child sent a report, using that status also on the
server side gives more useful information to the user.
For example, the overall sync status is now "unexpected slow sync
(local, status 22000)" after an unexpected slow sync instead of
logging a transport error there.
|
|
Shutting down syncevo-local-sync in a timely manner when
aborting is hard: the process might be stuck in a blocking
call which cannot be made to check the abort request (blocking
libneon, activesyncd client library, ...).
The best that can be done is to let the process be killed by the
SIGTERM. To have some trace of that, catch the signal and log the
signal; there's a slight risk that the logging system is in an
inconsistent state, but overall that risk is minor.
Because syncevo-local-sync catches SIGINT, ForkExec::stop() must send
SIGTERM in addition to SIGINT. To suppress redundant and misleading
ERROR messages when the bad child status is handled, the
ForkExecParent remembers that itself asked the child to stop and only
treats unexpected "killed by signal" results as error.
The local transport must call that stop() in its cancel(). It enters
the "canceled" state which prevents all further communication with the
child, in particular waiting for the child sync report; doing that
would produce another redundant error message about "child exited
without sending report".
Calling stop() in the local transport's shutdown() is no longer
possible, because it would kill the child right away. Before it simply
had no effect, because SIGINT was ignored. This points towards an
unsolved problem: how long should the parent wait for the child after
the sync is done? If the child gets stuck hard after sending its last
message, the parent currently waits forever until the user aborts.
In the sync event loop the caller of the transport must recognize
CANCELED as something which might be desired and thus should not be
logged as ERROR. That way the Synthesis engine is called one more time
with STEPCMD_ABORT also in those cases where the transport itself
detected the abort request first.
|
|
Having the signal handling code in SyncContext created an unnecessary
dependency of some classes (in particular the transports) on
SyncContext.h. Now the code is in its own SuspendFlags.cpp/h files.
Cleaning up when the caller is done with signal handling is now part
of the utility class (removed automatically when guard instance is
freed).
The signal handlers now push one byte for each caught signal into a
pipe. That byte tells the rest of the code which message it needs to
print, which cannot be done in the signal handlers (because the
logging code is not reentrant and thus not safe to call from a signal
handler).
Compared to the previous solution, this solves several problems:
- no more race condition between setting and printing the message
- the pipe can be watched in a glib event loop, thus removing
the need to poll at regular intervals; polling is still possible
(and necessary) in those transports which do not integrate with
the event loop (CurlTransport) while it can be removed from
others (SoupTransport, OBEXTransport)
A boost::signal is emitted when the global SuspendFlags change.
Automatic connection management is used to disconnect instances which
are managed by boost::shared_ptr. For example, the current transport's
cancel() method is called when the state changes to "aborted".
The early connection phase of the OBEX transport now also can be
aborted (required cleaning up that transport!).
Currently watching for aborts via the event loop only works for real
Unix signals, but not for "abort" flags set in derived SyncContext
instances. The plan is to change that by allowing a "set abort" on
SuspendFlags and thus making
SyncContext::checkForSuspend/checkForAbort() redundant.
The new class is used as follows:
- syncevolution command line without daemon uses it to control
suspend/abort directly
- syncevolution command line as client of syncevo-dbus-server
connects to the state change signal and relays it to the
syncevo-dbus-server session via D-Bus; now all operations
are protected like that, not just syncing
- syncevo-dbus-server installs its own handlers for SIGINT
and SIGTERM and tries to shut down when either of them
is received. SuspendFlags then doesn't activate its own
handler. Instead that handler is invoked by the
syncevo-dbus-server niam() handler, to suspend or abort
a running sync. Once syncs run in a separate process, the
syncevo-dbus-server should request that these processes
suspend or abort before shutting down itself.
- The syncevo-local-sync helper ignores SIGINT after a sync
has started. It would receive that signal when forked by
syncevolution in non-daemon mode and the user presses
CTRL-C. Now the signal is only handled in the parent
process, which suspends as part of its own side of
the SyncML session and aborts by sending a SIGTERM+SIGINT
to syncevo-local-sync. SIGTERM in syncevo-local-sync is
handled by SuspendFlags and is meant to abort whatever
is going on there at the moment (see below).
Aborting long-running operations like import/export or communication
via CardDAV or ActiveSync still needs further work. The backends need
to check the abort state and return early instead of continuing.
|
|
Resource tracking used custom C++ classes which owned various lower
level C objects. Can be done in a simpler way with eptr and a custom
unref operation.
The sdp_session_t in m_sdp was not tracked in a smart pointer and
closed a few times without setting it to NULL. The corresponding glib
event was also not properly tracked and never (?) freed, leaving a
dangling reference to the ObexTransportAgent in the glib context. Both
might have caused random crashes. Fixed by tracking both in smart
pointers and thus deleting them before ObexTransportAgent gets
deleted.
Removed code duplication when catching exceptions in C callbacks.
Reuse generic exception logging. The logic was improved: an error is
only logged when the transport has not encountered a problem already.
That removes some noise from the console output.
Cancelling the transport now properly reports a "cancelled" status to
the upper layers. Previously, shutting down the transport caused a
transport error and that status got reported back.
ObexTransportAgent::cancel() crashed when called before the connection
could be established. Now variables are checked before using them.
|
|
Instead of forking and continuing to sync in the forked process
without an explicit exec(), exec() the 'syncevo-local-sync' helper in
the forked process. The syncevo-local-sync helper binary gets
installed into libexec. SYNCEVOLUTION_LIBEXEC_DIR must be set if that
helper is not installed yet, not in the PATH, or an old version would
be found without that env variable ("make" without "make install"
during development!).
Main advantage is the cleaner environment for running the child side
of local sync. Required for getting ActiveSync to work again (GDBus
GIO as used by recent activesyncd client libraries did not work in the
forked process without the exec()).
Full D-Bus communication gets established between parent and child.
The downside is the hard dependency of local sync on the D-Bus
libraries (not the daemon!).
D-Bus communication allowed implementing interactive password requests
from the child side through the parent to the UI using the parent,
something that wasn't implemented before.
The child asks its parent for the password, which in turn
passes the request to its SyncContext. This happens to work
when that SyncContext is a normal instance (reads from stdin,
the "syncevolution --daemon" case) and the syncevo-dbus-server
(sends out an Info Request signal and waits for a response).
The info request and response are handled in the blocking
askPassword() by polling the running mail loop, so the parent should
remain responsive. Overall it is still a pretty difficult setup; it
would be better if askPassword() was asynchronous.
Describing the required password also is sub-optimal: the sync-ui just
asks for a password in its current config (even though that might not
be the config which currently gets synced) and crashes if no config is
currently selected. The command line uses the description derived from
the property and config name, which is a bit technical, but at least
correct.
Syncing uses the child's error string as "first error" in the parent,
too, by logging it anew on the parent side. That puts it into the
parent's sync report ahead of any additional error that it might
encounter while the child shuts down. Also use the child's status when
available instead of a misleading TransportError.
In addition, suppress as many of these errors as possible when we know
already that the child reported an error in its sync report. Not all
"transport errors" are currently avoided that way, but this is at
least a first step.
|
|
The LocalTransportAgent needs to call the askPassword()
implementation of its SyncContext, so keep the methods
public as in the ConfigUserInterface base class.
|
|
Only the parent process is receiving a sync report. Don't try that in
the child. Relevant for local sync with fork/exec, because there the
child properly deconstructs all classes including its SyncContext.
|
|
Necessary for transmitting via D-Bus (easier that way compared
to writing D-Bus traits).
|
|
libgdbussyncevo is a private library only needed when linking
statically. In the dynamic case it must be in the LD_LIBRARY_PATH
or rpath (affects install check when using non-empty DESTDIR).
Probably a lot of private libraries are missing from the .pc file.
This is at least a start.
|
|
The class depends on glib, and glib alone.
|
|
libgdbussyncevo always has to be built (needed by fork/exec and
local sync), even if the D-Bus server is not enabled.
|
|
src/src.am still used its local variables instead of the top-level
ones. In addition, it got the paths slightly wrong, causing build
errors in the nighty testing.
|
|
Not calling dbus_server_disconnect() kept the socket open. The error
handling for trying different bus unix domain paths was also broke
(didn't reset the DBusError).
|
|
test/dbus-client-server failed to link in shared mode because
some of the unit testing flags were missing.
|
|
Local sync needs D-Bus and glib. Either libdbus or gio-dbus (in gio >=
2.26) are good enough.
|
|
Added code which checks for existence of the helper binary in the
libexec dir when it is given without any path component. Searching
inside the PATH env variable is still used as fallback when not
found in libexec (useful for testing without actually installing
in the final destination).
|
|
Dead code, doesn't work. Merely committing as reminder. As before,
anything written by the child's logging code to stdout is printed
directly to the parent's stdout (no redirection anywhere).
The problems with this patch were:
- when SYNCEVOLUTION_DEBUG was set, the code still redirected
the child's stdout to stderr
- the child prints INFO messages which are relevant for the user;
with redirection, those do not reach the user
- the child's ERROR messages end up being printed as
[ERROR] stderr output: [ERROR] ....
|
|
When calling a method, give the peer unlimited time to reply. Relevant
in particular for local sync, where the time for processing a message
is basically unpredictable.
Communication with the D-Bus daemon still uses the default timeout.
|
