Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility

Subject: Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6 incompatibility

Date: Fri, 25 May 2012 12:23:05 -0400

To: Jameson Graef Rollins

Cc: Notmuch Mail

From: Austin Clements

Quoth Jameson Graef Rollins on May 25 at  8:47 am:
> On Fri, May 25 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> >> diff --git a/notmuch-client.h b/notmuch-client.h
> >> index 19b7f01..337409f 100644
> >> --- a/notmuch-client.h
> >> +++ b/notmuch-client.h
> >> @@ -36,6 +36,8 @@
> >>   * these to check the version number. */
> >>  #ifdef GMIME_MAJOR_VERSION
> >>  #define GMIME_ATLEAST_26
> >> +#else
> >> +typedef GMimeCipherContext GMimeCryptoContext;
> >
> > I like the typedef idea, but I don't think we should overload
> > GMimeCryptoContext like this.  If someone is reading through the GMime
> > 2.4 code and sees this, they're going to assume that it's a GMime
> > structure, go looking for it, find that it's only in 2.6 and be
> > baffled.  Instead, how about providing a typedef to abstract *both*
> > cases?  Something like
> >
> > #ifdef GMIME_MAJOR_VERSION
> > #define GMIME_ATLEAST_26
> > typedef notmuch_crypto_context_t GMimeCipherContext;
> > #else
> > typedef notmuch_crypto_context_t GMimeCryptoContext;
> > #endif
> 
> Hey, Austin.  I briefly thought about this, but it seemed kind of heavy
> handed given that I hope these ifdefs will go away in the
> not-too-distant future.  Do we really have a lot of gmime 2.4 readers
> that would not have access to gmime 2.6 documentation?  I'm pretty sure
> that I would personally end up looking at documentation for both
> versions.
> 
> But anyway, if this really is a concern, I guess it's not *that* much
> effort to support a new typedef indefinitely to alleviate any potential
> confusion.
> 
> Any other opinions?

The same thought had crossed my mind, but I figured we could remove
the typedefs once we do drop support for 2.6 and simply replace
notmuch_crypto_context_t with GMimeCipherContext at that point.  I
don't think that would require many changes at that point
(GMimeCryptoContext and GMimeCipherContext together appear only eight
times in the current master, and I think your series reduces that even
further), and most of the places it would change would probably also
require stripping out other compatibility code.  You probably have a
much better sense for the extent of this than I do, though.

Thread: