Conversation
lukefromdc
left a comment
There was a problem hiding this comment.
I do not know how to use vdirsyncer, I normally use text files on the desktop to manage events (primitive but hard to forget) so without a gui option to invoke it from the clock, I was unable to test that functionally. Didn't interfere with Evolution, though I have an empty calendar so obscure problems might not show up.
The line
password = "yourpassword"
in the example config file is a security hazard if users are supposed to store their login password (which gives ROOT access if they have sudo allowed!) in it!
Without it, on running
vdirsyncer discover my_calendar
I get
Discovering collections for pair my_calendar
my_local:
warning: Failed to discover collections for my_remote, use `-vdebug` to see the full traceback.
error: Unknown error occurred: Cannot connect to host your-caldav-server:443 ssl:default [Name or service not known]
error: Use `-vdebug` to see the full traceback.
luke@ubuntu:~$ vdirsyncer discover my_calendar -vdebug
Usage: vdirsyncer discover [OPTIONS] [PAIRS]...
Try 'vdirsyncer discover -h' for help.
vdirsyncer sync asks me to run the previous command, and no change seen anywhere in the calendar.
|
Thanks for testing! Then in the configuration: It replicates vdirsyncher documentation in here, but it's going to spare a few minutes for users of mate desktop and make it more secure. I can add it too. |
|
For test purposes, is there any way to set this up to run without calling the user's
password at all? If not, being able to prompt for it and not retain it should be
enough BUT it remains an unnecessary risk. No guarantee I will be able to
test this for function given the complexity of doing so.
After all, if the calendar is sensitive the whole /home directory
needs to be encrypted.
|
|
Any updates on this? |
39e201e to
5369d48
Compare
|
Assuming this is a simple rebase? |
|
@vkareh would love to have your review here, as this work is based on the EDS backend you backported from gnome. I link to test script in this gist. It will run a calendar using radicale, inject events, setup vdirsync, sync events, sync events, and finally show the calendar pop up with the events. It should be something like this:
The gist: https://gist.github.com/oz123/da9bdb04ce47825be8206b4238e80e0c |
vkareh
left a comment
There was a problem hiding this comment.
The applets/clock/meson.build file still references calendar-sources.c, which breaks meson builds. Also there seem to be some inconsistencies in the if have_eds gating and dependencies between both build systems...
I did a first pass, mostly jumping around between things. I'll review more parts as I go over them.
|
@vkareh thanks for the review! I'll address the issues and push fixes. |
lukefromdc
left a comment
There was a problem hiding this comment.
The vsyncdir documentation is still rather out of my understanding, but like I said earlier I have never used it. I have never seen a password-protected calendar before, note that I have only ever managed anything that sensitive locally never remotely so that may be why,
Have you tried the script in the gist? |
|
First I saw that gist. You have the script set up for an x11 build, meaning I'd have to close all applications, log out, and switch to the x11 session to test that build. Building isn't the problem using it is. |
|
At any rate we have another reviewer and if it works for them I am satisfied. |
|
@vkareh is this ready to go? This one is really out of my understanding |
|
No. It's not really as I haven't addressed all the issues point out by @vkareh. I'm occupied with other things right now, doing my best to address those asap. |
|
No problem, work on in when you get time |
Replace the monolithic EDS-only calendar-sources.c with an extensible provider architecture: - Add CalendarProvider abstract base class with virtual vtable and signals (appointments-changed, tasks-changed) - Add CalendarEDSProvider: absorbs all EDS/calendar-sources logic, guarded by HAVE_EDS - Add CalendarVdirProvider: reads vdir collections (vdirsyncer format), expands recurrences via libical-glib, watches directories with GFileMonitor, guarded by HAVE_LIBICAL - Rewrite CalendarClient as a thin aggregator over a list of providers - Auto-discover vdirsyncer default path (~/.local/share/vdirsyncer/) and allow extra paths via new GSettings key vdir-calendar-paths - Add HAVE_LIBICAL autoconf check for libical-glib >= 3.0 (independent of EDS; EDS implies it) - Update calendar-window.c guards from HAVE_EDS to HAVE_EDS || HAVE_LIBICAL so vdir events show without EDS Remove calendar-sources.c/h (functionality absorbed into calendar-eds-provider.c). Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
- Update #ifdef HAVE_EDS guards in clock.c to #if defined(HAVE_EDS) || defined(HAVE_LIBICAL) so the calendar client is created and passed to the calendar window when only libical is available (no EDS) - Fix calendar_vdir_discover() to recurse one level into subdirectories that are not themselves vdir collections, matching vdirsyncer's layout: <base>/<pair-name>/<collection-name>/ Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
Add applets/clock/README.md describing both calendar backends: - Evolution Data Server integration (appointments and tasks) - vdir/vdirsyncer support: auto-discovery, extra paths via GSettings, setup instructions, recurring events, and collection metadata Add optional dependency entries for EDS and libical-glib to the top-level README. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
- Add libecal2.0-dev, libedataserver1.2-dev, libical-glib-dev to Debian/Ubuntu dependency list - Add evolution-data-server, libical to Arch dependency list - Add -DHAVE_EDS, -DHAVE_LIBICAL, -DLIBICAL_GLIB_UNSTABLE_API to cppcheck defines so the new calendar provider code paths are analysed - Add libecal-2.0 and libical-glib to cppcheck pkg-config packages for correct include paths Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
meeting titles were displayed as <b>event title</b> in the tooltip. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
e_source_get_display_name is the correct method. Previously,
the code showd the protocol type ("caldav", "local", etc.)
Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
i_cal_time_subtract() and i_cal_duration_as_int() were removed in newer
libical-glib (as shipped on Arch Linux). Replace them with:
- direct i_cal_time_as_timet() subtraction for DTEND − DTSTART
- a portable ical_duration_as_secs() helper built from the individual
i_cal_duration_get_{weeks,days,hours,minutes,seconds}() accessors
Property getter functions (i_cal_property_get_dtstart etc.) changed
their parameter from ICalProperty * to const ICalProperty * in newer
releases, making them incompatible as function-pointer arguments on both
old and new versions simultaneously. Add thin prop_get_dt{start,end}/
prop_get_{due,completed} wrappers typed as ICalTime *(*)(ICalProperty *)
so the call sites compile cleanly against either API.
Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
Add guidance for protecting vdirsyncer credentials in the README: chmod 600 / umask on ~/.vdirsyncer/config, and using password.fetch with gnome-keyring as a plaintext-free alternative. Note that this brings vdir credential storage in line with the EDS backend, which already relies on libsecret/gnome-keyring by default. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
calendar-window.h guarded the calendar-client.h include and the calendar_window_set_client prototype with #ifdef HAVE_EDS only, while calendar-window.c already used the HAVE_EDS || HAVE_LIBICAL convention used elsewhere in the applet. Building with --enable-libical --disable-eds left CalendarClient undeclared and the prototype missing, breaking clock.c compilation. Align both guards in the header with HAVE_EDS || HAVE_LIBICAL. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
vdirsync has a none obvious quirk discovered while testing. Document how to bypass it, without needing to fix vdirsync. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
calendar-vdir-provider.c derived the local timezone with
strftime("%Z") and matched it against a builtin timezone by
abbreviation. TZ abbreviations are ambiguous (e.g. "CST" maps to
multiple zones), so the lookup can silently fail and fall back to
UTC, shifting event times.
Use SystemTimezone / system_timezone_get(), as calendar-eds-provider.c
already does, to resolve an unambiguous zone name (e.g.
"Europe/Berlin") from system configuration instead.
Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
The EDS-implies-libical fallback in configure.ac defined HAVE_LIBICAL and set LIBICAL_CFLAGS/LIBICAL_LIBS by aliasing EDS_CFLAGS, without ever running PKG_CHECK_MODULES for libical-glib itself. This borrowed EDS_CFLAGS's transitive inclusion of libical-glib's headers, which only holds if libecal-2.0.pc lists libical-glib under a public Requires rather than Requires.private - a packaging convention, not a guarantee. Run a real PKG_CHECK_MODULES(LIBICAL, ...) whenever EDS is enabled but libical wasn't already detected on its own, so the dependency is verified rather than assumed, with a proper configure-time error if it's missing. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
fc69ddf to
cde5c37
Compare
Replace the g_settings_list_keys() + linear-scan loop with a single g_settings_schema_has_key() call on the applet's schema, as suggested in review. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
cde5c37 to
07b5960
Compare
Previously the 'vdir-calendar-paths' GSettings key was only read once during calendar_client_new(), so changing the value required restarting the applet to take effect. Track the providers created from the key in a separate list and, when the key changes, tear those providers down and re-create them from the new value. If a month is already selected, re-issue the selection so the fresh providers populate their caches immediately. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
The discovery function recurses into any non-collection subdirectory, i.e. to arbitrary depth; say so in the comment. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
i_cal_property_get_rrule() may return NULL for an RRULE property that carries no value, and the resulting iterator may not be usable either. Check both, and when the iterator cannot be created fall back to a single occurrence at DTSTART so such events do not disappear. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>
The clock applet's meson build was broken and out of sync with the autotools build: - it referenced the long-gone calendar-sources.c/h, - it only compiled the calendar client when EDS was enabled and never compiled calendar-provider.c at all, - it had no notion of libical-glib (no dependency, no HAVE_LIBICAL, no LIBICAL_GLIB_UNSTABLE_API define, no vdir provider sources). Mirror the autotools layout: the calendar client/provider sources are always built, the EDS provider is gated on EDS and the vdir provider on libical-glib. Add an enable-libical option (like --enable-libical) and treat libical-glib as a hard requirement when EDS is enabled, as configure.ac does. Signed-off-by: Oz Tiram <oz.tiram@gmail.com>

Building on top of the work for getting events from EDS, I added the option to read events from vdir.
This is useful for people who don't want to install evolution just for getting calendar events shown in the applet. Also, it requires less RAM (no background process, as vdirsyncher can be launched using cron or a systemd user unit files).
This is my 3rd attempt to implement this feature (previous attempts not published), I hope this time I got it right,