After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 522639 - libcamel dependency
libcamel dependency
Product: totem-pl-parser
Classification: Core
Component: General
Other Linux
: Normal normal
: ---
Assigned To: totem-pl-parser-maint
Depends on:
Reported: 2008-03-15 17:23 UTC by William Jon McCann
Modified: 2009-05-23 21:44 UTC
See Also:
GNOME target: ---
GNOME version: ---

Just throwing some stuff together (7.96 KB, text/plain)
2008-03-18 22:55 UTC, Olliver Schinagl
Optional dependency of libcamel (6.52 KB, patch)
2008-10-27 09:33 UTC, Ivan Frade
none Details | Review
Optional dependency of libcamel (now "yes" by default, and no "auto" option) (6.46 KB, patch)
2008-10-27 10:57 UTC, Ivan Frade
needs-work Details | Review
Optional libcamel with last comments from Bastien (3.96 KB, patch)
2008-10-27 16:38 UTC, Ivan Frade
none Details | Review

Description William Jon McCann 2008-03-15 17:23:09 UTC
Looks like we require libcamel only for the following:
        /* Fall back to RFC 2822 date parsing */
        return camel_header_decode_date (date_str, NULL);

Considering that the RFC probably won't be changing it might be ok to copy this function into totem.  Thoughts?
Comment 1 Bastien Nocera 2008-03-16 13:04:44 UTC
I didn't manage to do it at the time without copying half of libcamel. You're welcome to try...
Comment 2 Olliver Schinagl 2008-03-18 02:18:06 UTC
This one line of code however seems to pull in the entire evolution-data-server which seems a little silly. Even using an entire library for just that one function seems a little whack, but I suppose that could be acceptable. Any chance this will get changed?
Comment 3 Bastien Nocera 2008-03-18 10:13:45 UTC
The function is hard to implement, which is why I'm using a library. Unless someone reimplements the functionality separately, I think you should bring your concerns to your distro's packagers.
Comment 4 Olliver Schinagl 2008-03-18 13:41:20 UTC
I suppose copy pasting it from libcamel isn't an option :p

Well I always thought Gentoo tried to minimize/separate dependencies as much as possible. I haven't found where to get libcamel separately, it seems to be part of eds, something you apparently can't do without. I understand the difficulty in this progress, and your doing a great job so far.

But from a architectural point of few, what does a Playlist manager for a audio/video player have to do with an e-mail application backend? I could understand it in an IM application, but architecturally it doesn't make sense.

From what I saw in the code, it's a fall back for when the ISO version fails e.g. Try iso approach first, then The RFC one. The ISO seems to be dating from 1988 I belive, so the first obvious question is, how often does the fall through case get involved? If it never happens, or only extremely rarely, wouldn't it be an idea to just drop it all together? On that note, I'm unsure as to how this feature is being used, time/date in the playlist?

P.S. I'm not trying to be disrespectful or offensive only helpful.
Comment 5 Bastien Nocera 2008-03-18 13:45:52 UTC
This is used to parse dates in Podcasts. RSS feeds use the "old" standard, Atom feeds and OPML feeds use the new one.
Comment 6 Olliver Schinagl 2008-03-18 13:51:51 UTC
So we can't do without, fair enough. And as mentioned in the original bug report, copying and minimizing that one function isn't an option? No other library implements that rfc either?
Comment 7 Bastien Nocera 2008-03-18 13:56:54 UTC
Not that I found.
Comment 8 Olliver Schinagl 2008-03-18 14:54:05 UTC
So I went hunting, and found many implementations raging from perl, via python, to even haskell and pod. Many scripting languages that I didn't even know off have modules for them. It's hard to imagine there is not C implementation for it, other then eds. I did find that git uses rfc822 date/time parsing, so i'm gonna look at their code now, and see how they do it.
Comment 9 Bastien Nocera 2008-03-18 14:56:33 UTC
It's very likely that git only parses dates it generates itself, whereas libcamel knows how to parse all kind of crap.
Comment 10 Matthew Barnes 2008-03-18 15:07:26 UTC
FWIW, I've been trying to separate Camel from libedataserver at least logically, even if we don't wind up shipping them separately.  Haven't gotten much of a response from the other Evo developers though.  See bug #424744.
Comment 11 Olliver Schinagl 2008-03-18 19:52:47 UTC
I'm looking through the date conversion function, and I think it's only a matter of copying 2 small functions. I'll see if I can work something out.

BTW the function in question has the following header comment:

/* convert a date to time_t representation */
/* this is an awful mess oh well */
camel_header_decode_date(const char *in, int *saveoffset)
Comment 12 Bastien Nocera 2008-03-18 19:58:59 UTC
Go on, give it a try... You'll end up pulling half of libcamel.

As for the comment, that's a bit expected when you have to handle all the crap coming from broken clients.
Comment 13 Olliver Schinagl 2008-03-18 22:55:43 UTC
Created attachment 107572 [details]
Just throwing some stuff together

I did have to add broken-date-parser.c/h no point in copying those files entirely. The proposal to logically seperate camel from the rest as pointed out above needs to look into e_mktime_utc, the only reference in there I found. It's also the only modification I made to that file, added a simple function for it and removed the header reference to the libedataserver. code:

	tm->tm_isdst = -1;
	tt = mktime (tm);

There are some more define's included int he edataserver part, which I suppose could be included, but for this proof of concept shouldn't be needed.

I also removed all the d() and d2() stuff from the code in cameless.c (I know it would have been easier/smarter to make the define for it)

Compared to the whole camel thing, this is a very small fraction of it. I didn't do extensive testing, but it seems to be working.

Of course this should/could maybe be severely trimmed, since we don't need all the share-ability in a few of these functions.

Also the table at the beginning should be put back into the single file it was in, for the simple reason that it's a generated table, and to keep it 'as is' making it very easy to update when a new one comes out.

It may not be pretty, but it beats pulling a huge dependency that libedataserver is for a mere 891 lines of code. This is less then 1% of the 91497 lines libcamel is, and incomparably less then what libedataserver is.

Just copy broken-date-parser.* over, add the aforementioned change, compile with   gcc -o cameless broken-date-parser.c cameless.c `pkg-config --cflags --libs glib-2.0` and it can be thrown some sample data (which I don't know much about to be fair).
Comment 14 Bastien Nocera 2008-03-19 01:52:21 UTC
Please provide a patch, and something that's updatable from libcamel. I'm not interested in having to cut'n'paste manually from libcamel. One of the points of using libcamel is that I don't have to fix the bugs, or follow the fixes.
Comment 15 Bastien Nocera 2008-03-19 01:53:34 UTC
There's also a number of date parsing self-tests in test-parser.
Comment 16 André Klapper 2008-06-08 15:58:13 UTC
moving libcamel out of e-d-s would reduce the overkill, but that discussion may take a few more years... sigh.
Comment 17 Bastien Nocera 2008-08-20 12:30:29 UTC
It should be possible to use librfc822 from maildrop. rfc822_parsedt() is the function needed. See docs at:

If somebody wants to check the library is able to parse the dates listed in test-parser.c.

I've filed a bug against Fedora to get the library exported (it's not installed by default in the package):
Comment 18 Ivan Frade 2008-10-27 09:30:09 UTC
Attached a patch to do the libcamel dependency optional. My main interest on this topic is to use totem-pl-parser in tracker for maemo (and upstream).

Brief summary of the modifications: Dependency of libtracker optional and added a property to know if the library is compiled with libcamel or not (useful for rhythmbox to know if podcast are supported)
plparse/ Added autotools stuff to check libcamel and the proper flags and libs

plparse/totem-pl-parser.c: ifdefs for the parsing date function depending on libcamel availability.

plparse/test-parser.c: ifdefs to remove the code testing date parsing if libcamel is not available.

Please review.
Comment 19 Ivan Frade 2008-10-27 09:33:00 UTC
Created attachment 121414 [details] [review]
Optional dependency of libcamel
Comment 20 Christian Persch 2008-10-27 09:51:12 UTC
[I'm not a totm-pl-parser maint, but] I just saw this configure snipplet and have a comment on it:

+	      AS_HELP_STRING([--enable-camel=@<:@no/yes/auto@:>@],
+			     [Use libcamel to parse dates (if available).]), ,
+	      [enable_camel=auto])
+if test "x$enable_camel" != "xno" ; then
+		     camel-1.2,
+                     [have_camel=yes], 
+                     [have_camel=no])
+   if test "x$have_camel" = "xyes" ; then
+      AC_SUBST(LIBCAMEL, camel-1.2)
+      AC_DEFINE(HAVE_CAMEL, 1, [Camel available in the system])
+   else

Automagic dependencies are bad, since they create unpredictable results. IMHO this should be enabled by default and error out if camel isn't found, and you'd need to pass --disable-camel if you know what you're doing.
Comment 21 Ivan Frade 2008-10-27 10:57:57 UTC
Created attachment 121419 [details] [review]
Optional dependency of libcamel (now "yes" by default, and no "auto" option)

New version of the patch, removing the "auto" option in and using by default "yes" (as suggested by Christian).
Comment 22 Bastien Nocera 2008-10-27 15:22:19 UTC
+ This can affect seriously the podcast handling.

It doesn't seriously affect it, it breaks it.

+#ifdef HAVE_CAMEL
 guint64 totem_pl_parser_parse_date     (const char *date_str, gboolean debug);

You can't do that in a public header.


Please remove the ifdef HAVE_CAMEL in test-parser.c

Also, for the change in, check the libcamel option first, then simply add/remove libcamel to the PKG_CHECK_MODULES call for TOTEM_PLPARSER. No need to modify then.
Comment 23 Ivan Frade 2008-10-27 16:38:30 UTC
Created attachment 121439 [details] [review]
Optional libcamel with last comments from Bastien

Applied the suggested changes on the patch.

Now it only modifies:

* Adding the check for libcamel
* plparser/totem-pl-parser.c (totem_pl_parser_parse_date)
  g_warning indicating a call to the function without libcamel support.
* Added variable to know if the library is compiled with  libcamel support and to set dynamically the library dependencies.
Comment 24 Bastien Nocera 2008-11-05 17:15:11 UTC
Applied with slight changes. The configure option is now:
                          Disable libcamel (Unsupported, breaks Podcast

And every Podcast parsing function will fail with a big fat warning, and an error.

Could you please file a separate bug for the Rhythmbox check as well?

Any further hacking should be done on reducing libcamel dependencies.

2008-11-05  Bastien Nocera  <>

        * plparse/totem-pl-parser-podcast.c (totem_pl_parser_add_rss),
        (totem_pl_parser_add_itpc), (totem_pl_parser_add_zune),
        (totem_pl_parser_add_atom), (totem_pl_parser_add_xml_feed),
        (totem_pl_parser_add_itms), (totem_pl_parser_add_opml):
        * plparse/totem-pl-parser-podcast.h:
        * plparse/totem-pl-parser.c (totem_pl_parser_parse_date):
        * Adapt patch from Ivan Frade
        <> to make libcamel optional for some
        "embedded" platforms. This is absolutely unsupported and 
        any general purpose distribution shipping with libcamel
        disabled will be kicked in the knackers
        forcefully (Closes: #522639)
Comment 25 Peter Robinson 2008-11-18 10:45:27 UTC
Just an idea, not sure if its workable or not. Would it be possible to move the date parsing function that camel uses into libical (they are both date related although I'm not sure its an exact match) when evolution moves to using the upstream libical. That way totem would only need to depend on the much smaller libical, and e-d-s already would be depending on it anyway. That would split it out into a single upstream location that's a smaller dep and still a single point for maintenance?
Comment 26 Bastien Nocera 2008-11-18 11:12:40 UTC
(In reply to comment #25)
> Just an idea, not sure if its workable or not. Would it be possible to move the
> date parsing function that camel uses into libical (they are both date related
> although I'm not sure its an exact match) when evolution moves to using the
> upstream libical. That way totem would only need to depend on the much smaller
> libical, and e-d-s already would be depending on it anyway. That would split it
> out into a single upstream location that's a smaller dep and still a single
> point for maintenance?

Where's that elusive upstream libical, and are you sure it provides parsing for RFC822 dates?
Comment 27 Peter Robinson 2008-11-18 11:44:40 UTC
The upstream libical is according to the Fedora package and I don't believe it currently provides it (although it might provide an equivalent). Evolution is moving from its included copy and pushing its changes upstream. I was wondering whether this date function could also be pushed upstream to that library allowing the dependency to move as well. Matthew Barnes would be the best person to answer that question though.
Comment 28 Matthew Barnes 2008-11-18 17:10:01 UTC
(In reply to comment #27)
> I was wondering whether this date function could also be pushed upstream
> to that library allowing the dependency to move as well.

Well, if Bastien couldn't pull out that Camel function without dragging in half the library I'm not sure it'd be any easier for the libical guys.

I've lost track of how the E-D-S merge to upstream libical is going.  Bug #541209 is tracking the effort but hasn't been updated in awhile.  Maybe post an inquiry there or at <>.

Personally, I think it's toss up which will happen first: E-D-S ditches its libical fork or Camel breaks free of E-D-S.  We do have more control over the Camel situation...