When running EDS under valgrindcheck.sh in parallel to running
syncevo-dbus-server under valgrindcheck.sh, then cleaning up after
syncevo-dbus-server must not kill EDS.
To achieve this, look at all log files created for us and extract the
pid from each file.
Also fix sending INT/TERM vs. KILL signals to these processes.
When running test-dbus.py or testpim.py with SYNCEVOLUTION_DEBUG
set, they now skip the redirection of syncevo-dbus-server output
into a file. Useful when doing interactive debugging without running
under a debugger.
When starting a D-Bus session for test-dbus.py and testpim.py, valgrind
gets injected via an env variable and the older check of the program to
be started did not match. Need to check the environment in addition to
the command line, otherwise EDS gets started by the D-Bus daemon and
we never see its output nor does it get checked by valgrind.
The problem has been fixed in the meantime (00f566 "sqlite addressbook: fix
memory corruption in get_revision") and no longer occurs in an release we test
against.
In EDS 3.6.4, opening fails a lot more often with E_CLIENT_ERROR_BUSY.
We must retry until we succeed.
Seen in particular with testpim.py testFilterStartup. Clients are expected to
try the open call again. EDS >= 3.8 does that automatically.
Manually start EDS also when testing with testpim.py, to see the
output of the factory processes and allow running them from places
where the D-Bus daemon would not find them.
The sorting got broken when the number of contacts exceeds the 3
digits reserved via the 03d formatting specifier. Now allow four
digits = up to 9999 contacts.
Binding _done() to self.start reads the *current* value of self.start
when _done() gets called. We need to bind to an inmuted local instance
instead to achieve the desired effect.
The destructor must be virtual, otherwise destructors in dervived
classes are not getting called when destructing via a downcasted
pointer. Should be relevant, but did not show up in valgrind?!
Klocwork complained about not checking the result. Depending on the
situation we now give up or use some kind of fallback (for example,
when logging).
SyncEvolution itself is not multithreaded, but utility libraries
might be, so better avoid the localtime() function and its global
state. May also be relevant when function A gets a tm * pointer,
calls function B which calls localtime() again, then A tries to
use its original value.
When realloc() returns NULL, the already allocated buffer remains
available. Previously we discarded (and leaked!) it. Now we track
whether that memory already contains parts of the next log message
and use that when realloc fails. The dgram will then get discarded
without reading the rest.
When reading from a stream, realloc will only be called once to
get the initial buffer. When that fails, we give up.
Klocwork warned about using a NULL starttime pointer in the case that
the transport is unknown. Not sure whether this can actually happen at the
moment, fix it anyway.
All of the jobs were allocated dynamically without freeing them
explicitly, instead relying on auto-deletion (enabled by default).
This behavior is surprising for SyncEvolution developers and Klocwork,
which expects explicit delete calls for each new. Better use
traditional RAII.
The advantage also is that each job gets deleted right away, instead
of waiting for another round of event processing, which may or may not
happen soon.
Qt has never been happy with the old code, reporting:
QDBusConnection: session D-Bus connection created before QCoreApplication. Application may misbehave.
This error message became visible again after some recent logging
changes and broke tests, so better avoid it by testing for D-Bus
with a private connection.
Klocwork correctly reported that the underlying implementation in
libsynthesis must not be called with NULL pointers. Therefore always
allow it to set the return values, even if our caller was not
interested.
Klocwork failed to detect that the result of getenv() was checked.
Keep Klocwork happy by assigning the result to a temporary variable
and checking that.
The old code triggered a Klocwork warning about the missing self check
in the copy operator. This was not a real problem because of the
temporary instance from which m_error got taken, but that was not
obvious even to a human.
Better use standard coding practices:
- skip the operation when assigning to self
- reuse the set() operation
- return reference instead of copy (probably a bug!)
The code tried to transfer ownership of the GDBusMethodInfo and
GDBusSignalInfo arrays to the refcounted GDBusInterfaceInfo instance,
but that does not work because g_dbus_interface_info_unref() does not
free those arrays.
Let's simplify this so that all memory, including the GDBusInterfaceInfo
instance, is owned by DBusObjectHelper and kept around as long as the
object is active.
The stack-allocated GDBusInterfaceVTable happened to work in practice,
but the API description does not say whether
g_dbus_connection_register_object() makes a copy, so better keep that
around longer, too.
Finally, clean up checking for "is activated" to use only
m_connId. m_activated was redundant and the method resp. signal
pointers are now always non-NULL.
When running syncevo-dbus-server under valgrindcheck.sh,
the following happened occasionally:
- syncevo-dbus-server main thread quits, some threads keep running
=> ps shows the process as <defunct> with ppid = 1 = init
- valgrindcheck.sh notices that the process is done,
reports status and quits
- test-dbus.py fails to wait for the syncevo-dbus-server
process (because it is not the parent) and assumes that
the process is gone
At this point there is a lingering process which occupies the
well-known D-Bus name (= all further tests fail) and which
prevents unmounting the chroot.
It's unknown how the syncevo-dbus-server gets into that state.
Could be valgrind 3.7.0 or the kernel 3.4.28-2.20-xen.
As a workaround, let test-dbus.py collect the pids of all processed that it
couldn't wait for and send them SIGKILLs until that returns with "not
found".
Without the explicit TYPE=INTERNET, email addresses sent to a Nokia
e51 were not shown by the phone and even got lost eventually (when
syncing back?).
This commit ensures that the type is set for all emails sent to any
Nokia phone, because there may be other phones which need it and
phones which don't, shouldn't mind. This was spot-checked with a N97
mini, which works fine with and without the INTERNET type.
This behavior can be disabled again for specific Nokia phones by
adding a remote rule which sets the addInternetEmail session variable
to FALSE again.
Non-Nokia phones can enable the feature in a similar way, by setting
the variable to TRUE.
Compilation takes a lot longer when using -O2 and the result
is less debugger friendly, so filter out the flag. This is
relevant when building a release where the flag is set for the
rest of the compilation.
Complaining about "ERROR SUMMARY" when using valgrind is a false
positive. Ignore that text by making it lower case before searching. Other
attempts based on regex matching somehow failed (UTF-8 encoding error?!).
When starting the server fails, an exception gets thrown when
trying to determine its pid. This used to abort the whole
test script without recording much information. In particular
the server's output was not included.
Now the exception is caught, recorded as error and testing continues
with the next test.
This did not fix the root cause (a stuck process occupied the
D-Bus name) but at least it helped to identify the problem.
With all the interaction between Client::Sync, Client::Source and
CLIENT_TEST_SERVER it can be hard to figure out which test case file
gets used. Log that explicitly.
Because of Google issue with detached recurrences without parent
(http://code.google.com/p/google-caldav-issues/issues/detail?id=58)
and the SyncEvolution workaround (replacing RECURRENCE-ID with
X-SYNCEVOLUTION-RECURRENCE-ID) only one detached recurrence per UID
can be stored.
Removing the second modified recurrence from the test cases for
Google.
Somehow the second modified recurrence made its way back into the
test data while rewriting the generic eds_event.ics.
Some peers have problems with meta data (CtCap, old Nokia phones) and
the sync mode extensions required for advertising the restart
capability (Oracle Beehive).
Because the problem occurs when SyncEvolution contacts the peers
before it gets the device information from the peer, dynamic rules
based on the peer identifiers cannot be used. Instead the local config
must already disable these extra features in advance.
The "SyncMLVersion" property gets extended for this. Instead of just
"SyncMLVersion = 1.0" (as before) it now becomes possible to say
"SyncMLVersion = 1.0, noctcap, norestart".
"noctcap" disables sending CtCap. "norestart" disables the sync mode
extensions and thus doing multiple sync cycles in the same session
(used between SyncEvolution instances in some cases to get client and
server into sync in one session).
Both keywords are case-insensitive. There's no error checking for
typos, so beware!
The "SyncMLVersion" property was chosen because it was already in use
for configuring SyncML compatibility aspects and adding a new property
would have been harder.
In the previous attempt (commit a0375e) setting the <syncmodeextensions>
option was only done on the client side, thus breaking multi-cycle
syncing with SyncEvolution as server. On the server side the option
shouldn't be needed (the server never sends the extensions unless
the client did first), but for symmetry and testing reasons it makes
sense to also offer the options there.
Previously, the database field was interpreted as a Collection ID. This adds
logic to allow the database to be interpreted as a folder path. The logic is:
1) If the database is an empty string, pass it through (this is the most
common case as it is interpreted as "use the default folder for the
source type").
2) If the database matches a Collection ID, use the ID (this is the same as
the previous behaviour).
3) If the database matches a folder path name, with an optional leading "/",
use the Collection ID for the matching folder.
4) Otherwise, force a FolderSync to get the latest folder changes from the
server and repeat steps 2 and 3
5) If still no match, throw an error.
Steps 2 and 3 are in the new function lookupFolder. The remaining logic has
been added to the open function. Note that the result is that m_folder (and
hence getFolder()) are always either empty or a Collection ID -- that is as
before so the sync logic itself is unchanged.
The command line swallowed errors thrown by the backend while listing
databases. Instead it just showed "<backend name>: backend failed". The goal
was to not distract users who accidentally access a non-functional backend.
But the result is that operations like --configure or --print-databases could
fail without giving the user any hint about the root cause of the issue.
Now the error explanation in all its gory details is included.
For example, not having activesyncd running leads to:
INFO] eas_contact: backend failed: fetching folder list:
GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name
org.meego.activesyncd was not provided by any .service files
And running activesyncd without the necessary gconf keys shows up as:
[INFO] eas_contact: backend failed: fetching folder list:
GDBus.Error:org.meego.activesyncd.Error.AccountNotFound: Failed to find
account [syncevolution@lists.intel.com]
Both of these happened when trying to use the "list database" ActiveSync
patches in the nightly test setup...
