[PATCH] Use stock GMimeSession by default

2011-06-03 Thread Daniel Kahn Gillmor
Our use of GMimeSession was unneeded boilerplate, and we weren't doing
anything with it.  This simplifies and clarifies that assumption.

If we want to do anything fancier later, the examples in the gmime
source are a reasonable source to work from in defining a new
GMimeSession derivative.

Since GMimeSession is going away in GMime 2.6, though, i don't
recommend using it.
---
 Makefile.local  |1 -
 notmuch-client.h|3 --
 notmuch-gmime-session.c |   49 ---
 notmuch-reply.c |2 +-
 notmuch-show.c  |2 +-
 5 files changed, 2 insertions(+), 55 deletions(-)
 delete mode 100644 notmuch-gmime-session.c

diff --git a/Makefile.local b/Makefile.local
index f726f1f..8a8832d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -246,7 +246,6 @@ notmuch_client_srcs =   \
notmuch-show.c  \
notmuch-tag.c   \
notmuch-time.c  \
-   notmuch-gmime-session.c \
query-string.c  \
show-message.c  \
json.c  \
diff --git a/notmuch-client.h b/notmuch-client.h
index 8a27260..63be337 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -237,7 +237,4 @@ notmuch_config_set_maildir_synchronize_flags 
(notmuch_config_t *config,
 notmuch_bool_t
 debugger_is_active (void);

-GType
-notmuch_gmime_session_get_type (void);
-
 #endif
diff --git a/notmuch-gmime-session.c b/notmuch-gmime-session.c
deleted file mode 100644
index d83d9b3..000
--- a/notmuch-gmime-session.c
+++ /dev/null
@@ -1,49 +0,0 @@
-#include "notmuch-client.h"
-
-/* CRUFTY BOILERPLATE for GMimeSession (dkg thinks this will go away once 
GMime 2.6 comes out) */
-typedef struct _NotmuchGmimeSession NotmuchGmimeSession;
-typedef struct _NotmuchGmimeSessionClass NotmuchGmimeSessionClass;
-
-struct _NotmuchGmimeSession {
-GMimeSession parent_object;
-};
-
-struct _NotmuchGmimeSessionClass {
-GMimeSessionClass parent_class;
-};
-
-static void notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass);
-
-static GMimeSessionClass *parent_class = NULL;
-
-GType
-notmuch_gmime_session_get_type (void)
-{
-static GType type = 0;
-
-if (!type) {
-   static const GTypeInfo info = {
-   sizeof (NotmuchGmimeSessionClass),
-   NULL, /* base_class_init */
-   NULL, /* base_class_finalize */
-   (GClassInitFunc) notmuch_gmime_session_class_init,
-   NULL, /* class_finalize */
-   NULL, /* class_data */
-   sizeof (NotmuchGmimeSession),
-   0,/* n_preallocs */
-   NULL, /* object_init */
-   NULL, /* value_table */
-   };
-   type = g_type_register_static (GMIME_TYPE_SESSION, 
"NotmuchGmimeSession", , 0);
-}
-return type;
-}
-
-static void
-notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass)
-{
-GMimeSessionClass *session_class = GMIME_SESSION_CLASS (klass);
-parent_class = g_type_class_ref (GMIME_TYPE_SESSION);
-session_class->request_passwd = NULL;
-}
-/* END CRUFTY BOILERPLATE */
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 5265af6..514bbc6 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -607,7 +607,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
}
} else if ((STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
if (params.cryptoctx == NULL) {
-   GMimeSession* session = 
g_object_new(notmuch_gmime_session_get_type(), NULL);
+   GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
"gpg")))
fprintf (stderr, "Failed to construct gpg context.\n");
else
diff --git a/notmuch-show.c b/notmuch-show.c
index 9267d02..dda83a1 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -899,7 +899,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused 
(char *argv[]))
} else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
   (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
if (params.cryptoctx == NULL) {
-   GMimeSession* session = 
g_object_new(notmuch_gmime_session_get_type(), NULL);
+   GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
"gpg")))
fprintf (stderr, "Failed to construct gpg context.\n");
else
-- 
1.7.4.4



[PATCH] Use stock GMimeSession by default

2011-06-03 Thread Daniel Kahn Gillmor

This patch supercedes my patch from
1307142188-6551-1-git-send-email-dkg at fifthhorseman.net , and provides
what appears to be a much cleaner approach to achieve the same result.

 --dkg


[PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Daniel Kahn Gillmor
On 06/03/2011 07:15 PM, Carl Worth wrote:
> On Fri,  3 Jun 2011 19:03:08 -0400, Daniel Kahn Gillmor  fifthhorseman.net> wrote:
>> The notmuch binary is not in the business of doing interactive
>> prompting with the user.  If credentials are needed for decryption,
>> they should be supplied to the decrypting processes some other way
>> (e.g. gpg-agent).
>>
>> Previously, we returned a NULL function pointer for the
>> request_passwd() function, which may have cause segmentation faults
>> with some versions of gmime.
> 
> Cool. This fixes my segfaults, so thanks!
> 
>> +return g_strdup ("");
> 
> Is the above correct? Or is it a memory leak? (If it's not a leak, then
> GMime really has some bizarre ownership semantics.)

yes, this corner of gmime has some really bizarre ownership semantics;
twisty handoffs and callbacks abound :(

Hm, actually, we should just be returning NULL to indicate a failure; i
think that would be preferable, and apparently is documented to be
acceptable:

 
http://developer.gnome.org/gmime/stable/GMimeSession.html#g-mime-session-request-passwd

Would you mind amending that patch to just return NULL ?

fwiw, i've just filed https://bugzilla.gnome.org/show_bug.cgi?id=651826
to ask gmime for a hook to let us always request the use of gpg-agent if
it is available.

--dkg

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1030 bytes
Desc: OpenPGP digital signature
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/e67b8379/attachment.pgp>


[PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Daniel Kahn Gillmor
The notmuch binary is not in the business of doing interactive
prompting with the user.  If credentials are needed for decryption,
they should be supplied to the decrypting processes some other way
(e.g. gpg-agent).

Previously, we returned a NULL function pointer for the
request_passwd() function, which may have cause segmentation faults
with some versions of gmime.
---
 notmuch-gmime-session.c |   17 -
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/notmuch-gmime-session.c b/notmuch-gmime-session.c
index d83d9b3..33f2817 100644
--- a/notmuch-gmime-session.c
+++ b/notmuch-gmime-session.c
@@ -39,11 +39,26 @@ notmuch_gmime_session_get_type (void)
 return type;
 }

+/* 
+   notmuch never prompts the user for a password.  It always returns
+   an empty string (could it return a NULL pointer instead?)
+
+   If credentials are needed for crypto, they should be supplied via
+   other mechanisms (e.g. gpg-agent, etc)
+ */
+static char *never_request_passwd (unused (GMimeSession *session), unused 
(const char *prompt),
+  unused (gboolean secret), unused (const char 
*item),
+  unused (GError **err)) {
+fprintf (stderr, "Credentials needed for crypto; Please use gpg-agent.\n");
+return g_strdup ("");
+}
+
+
 static void
 notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass)
 {
 GMimeSessionClass *session_class = GMIME_SESSION_CLASS (klass);
 parent_class = g_type_class_ref (GMIME_TYPE_SESSION);
-session_class->request_passwd = NULL;
+session_class->request_passwd = never_request_passwd;
 }
 /* END CRUFTY BOILERPLATE */
-- 
1.7.4.4



release-candidate/0.6 redux

2011-06-03 Thread Carl Worth
On Tue, 31 May 2011 11:43:31 -0700, Jameson Graef Rollins  wrote:
> Hi, folks.  I have pushed a new version of the release-candidate/0.6
> branch to my repo [0].  It is all reworked on top of notmuch/master [1],
> and includes:
...
> cworth: do you want to review this for the new release?  We're very close!
> One last push and we can get this thing out.

I've been going through it, but trying to focus more on the emails I
have queued up (which is currently 58 messages).


[PATCH] Use stock GMimeSession by default

2011-06-03 Thread Carl Worth
On Fri,  3 Jun 2011 19:57:46 -0400, Daniel Kahn Gillmor  wrote:
> Our use of GMimeSession was unneeded boilerplate, and we weren't doing
> anything with it.  This simplifies and clarifies that assumption.

Very nice. This is pushed now.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/323082b2/attachment.pgp>


[PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Carl Worth
On Fri,  3 Jun 2011 19:03:08 -0400, Daniel Kahn Gillmor  wrote:
> The notmuch binary is not in the business of doing interactive
> prompting with the user.  If credentials are needed for decryption,
> they should be supplied to the decrypting processes some other way
> (e.g. gpg-agent).
> 
> Previously, we returned a NULL function pointer for the
> request_passwd() function, which may have cause segmentation faults
> with some versions of gmime.

Cool. This fixes my segfaults, so thanks!

> +return g_strdup ("");

Is the above correct? Or is it a memory leak? (If it's not a leak, then
GMime really has some bizarre ownership semantics.)

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/3aa2afae/attachment.pgp>


[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 15:38:29 -0700, Carl Worth  wrote:
> commit d5b4d950245605b84c56ce991fa3c59a073a70e5
> Author: Jameson Graef Rollins 
> Date:   Sat May 28 14:52:00 2011 -0700
> 
> show: Avoid inadvertently closing stdout
> 
> GMime has a nasty habit of taking ownership by default of any FILE*
> handed to it va g_mime_stream_file_new. Specifically it will close the
> FILE* when the stream is destroyed---even though GMime didn't open the
> file itself.
> 
> To avoid this bad behavior, we have to carefully set_owner(FALSE)
> after calling g_mime_stream_file_new. In the format_part_content_text
> function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
> been calling g_mime_stream_file_new unconditionally, but only calling
> g_mime_stream_file_set_owner(FALSE) conditionally.
> 
> This led to the FILE* being closed early when notmuch show output was
> redirected to a file.
> 
> Fixing this fixes the test-suite cases that broke with the previous
> commit, (which added redirected "notmuch show" calls to the test suite
> to expose this bug).
> 
> Edited-by: Carl Worth  with a new commit message to
> explain the bug and fix.

Now I sound like I know what I'm talking about!  Thanks, Carl.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/ee1f143f/attachment-0001.pgp>


When will we have our next release?

2011-06-03 Thread Carl Worth
On Fri, 03 Jun 2011 14:39:13 -0700, Jameson Graef Rollins  wrote:
> Can we set a target date for 0.6 release?  So we'll all start feeling
> really bad if we miss it?

Frankly, I wouldn't mind doing strict time-based releases with something
like the following:

* We schedule a release period (once per month?)

* We schedule a "safety period" before the release (one week?)

* At the beginning of the safety period, package up the head
  of the notmuch tree and upload to Debian experimental and
  anywhere else similar.

* During the safety period we hopefully get wider testing than
  we normally have from just the git master branch.

  If any bugs are found and fixed during this time we can create
  a branch for them. New feature work can continue to land on
  master.

* At the end of the safety period we package up the same bits,
  or the new bits from the cherry-picked fixes on the branch,
  and upload them to Debian unstable and anywhere else similar.

I'd even be happy for someone else (other than me) to run that process.

That might be beneficial for a couple of reasons:

* It would simply take one thing off my plate.

* I'm inclined to do feature work myself---and when I start
  doing that again, I might get tempted to delay the release
  "just until I finish this next feature...".

I think that's the problematic state we've been in for the past several
months. Right after 0.5 I thought "I should do some database changes to
support indexing/searching of arbitrary headers and to capture complete
email addresses". I've not actually gotten around to doing that work
yet, but I was a bit stuck mentally in "the next release will have those
features" so there was never any threat of a release actually happening.

So switching to a more strictly time-based cycle can definitely help,
(so many other projects use time-based releases very successfully).

Now, in order to hand the release process over to someone else, we need
a really simple "just push this button" mechanism for the release. I
think we've got that pretty-well in place right now, with the large
exception of writing the NEWS file.

So the fix for that is to start rejecting patches that add features or
fix user-visible bugs (other than regressions since the past release)
without also updating the NEWS file. I'll commit myself to doing that
for my own bug fixes and features as well.

I also think it's possible for me to be successful as the release
manager as long as we decide on a schedule as a community and that way
you all keep me to it.

The current state of keep-coding-until-we-have-a-state-good-enough-to-
call-the-next-release does have the potential to keep running on
forever.

I'd be glad to get any feedback or additional ideas from anyone,

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/eccc812f/attachment.pgp>


[PATCH] emacs: User-defined sections in notmuch-hello

2011-06-03 Thread Daniel Schoepe
On Fri, 27 May 2011 20:52:01 +0200, Daniel Schoepe  wrote:
> I'll also work on some tests for this functionality, (if no one
> has big, structural complaints about the code).

I rebased my patch against the current master and wrote some tests.

-- next part --
A non-text attachment was scrubbed...
Name: 0001-emacs-User-defined-sections-in-notmuch-hello.patch
Type: text/x-diff
Size: 20109 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/983b296b/attachment-0002.patch>
-- next part --
A non-text attachment was scrubbed...
Name: 0002-emacs-A-few-tests-for-user-defined-sections.patch
Type: text/x-diff
Size: 7558 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/983b296b/attachment-0003.patch>
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/983b296b/attachment-0001.pgp>


[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Carl Worth
On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins  wrote:
> Well actually it's only meant to sound like the committer doesn't
> understand the problem!

Heh, OK.

> > I'd like to investigate this more. Perhaps with a test case?
> 
> The current tests are how I found the problem!  Without this patch at
> least the multipart tests will fail.  I don't see how another test will
> add anything.

Ah, in my review I'd managed to get this commit detached from the
original previous commit that introduced the test failures. It's funny
that while you were replying I was reviewing *that* commit and thinking,
"why are all these tests failing now just because they changed to
test_expect_equal_file"?

> Carl, if you (or anyone else) understands what the issue is, then please
> go ahead and modify the commit message.

Done.

> I don't understand things
> enough myself to do any better.  Clearly there is some strange
> interaction with things that try to use stdout after
> g_mime_stream_file_new() has already grabbed it.

g_mime_stream_file_new is a bad citizen, API-wise. It closes files that
it didn't open, (by default).

> I really wouldn't block on this, though, since the patch does fix an
> actual bug.

Not blocked. All pushed. New commit message below for reference.

-Carl

commit d5b4d950245605b84c56ce991fa3c59a073a70e5
Author: Jameson Graef Rollins 
Date:   Sat May 28 14:52:00 2011 -0700

show: Avoid inadvertently closing stdout

GMime has a nasty habit of taking ownership by default of any FILE*
handed to it va g_mime_stream_file_new. Specifically it will close the
FILE* when the stream is destroyed---even though GMime didn't open the
file itself.

To avoid this bad behavior, we have to carefully set_owner(FALSE)
after calling g_mime_stream_file_new. In the format_part_content_text
function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
been calling g_mime_stream_file_new unconditionally, but only calling
g_mime_stream_file_set_owner(FALSE) conditionally.

This led to the FILE* being closed early when notmuch show output was
redirected to a file.

Fixing this fixes the test-suite cases that broke with the previous
commit, (which added redirected "notmuch show" calls to the test suite
to expose this bug).

Edited-by: Carl Worth  with a new commit message to
explain the bug and fix.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/e6e00e66/attachment.pgp>


[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth  wrote:
> Otherwise, the patches up to this point in the thread have all either
> been pushed or I've asked for some additional information (perhaps
> that's just this patch and the "old style fcc dirs" patch?).

Great!  That's really great news, Carl.  Thank you very much.  I think
at this point the only thing we need for 0.6 are:

* Austin's atomicity patches
* maybe fix the frc822 handling issue

Can we set a target date for 0.6 release?  So we'll all start feeling
really bad if we miss it?

> I'm actually a bit surprised to see myself preferring patches in
> email. When Linus first wrote git, I couldn't understand why the Linux
> community kept to such a consistent culture of sending patches via
> email. It seemed so backwards to do these awkward machinations (git
> format-patch, git send-email, SMTP, MUA, git am), and risk all the
> problems of email clients corrupting patches, etc.?especially when git
> has such clean mechanisms for reliably moving patches around (git push,
> git pull).

Yeah, I'm with you that I'm surprised by liking this method as well.  I
think it only really works once most of the bulk of things are already
working, but for bug fixes and feature additions it really works well.
It's nice to have comments for patches on list.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/4e3aeb2f/attachment.pgp>


normalizing part numbering across PGP/MIME processing

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:31:08 -0700, Jameson Graef Rollins  wrote:
> On Fri, 27 May 2011 17:53:44 -0700, Carl Worth  wrote:
> >   * Should we set the crypto option to verify/decrypt by default?
...
> I don't really have an opinion on this.  I have it set now, so whether
> or not it's set by default doesn't make much difference to me.

I'm inclined to set it by default. But I'll wait until I get it fully
working, (which looks to be problems in my environment). See below.

> >   * I can't actually get decryption to work for me. :-(
> > 
> > When I run "notmuch show --decrypt" on a message encrypted with
> > my public key I get a segfault within libgmime, specifically in
> > the g_mime_session_request_passwd function.

I'm still getting this. I'll start debugging libgmime next.

> > $ gpg-agent
> > gpg-agent: symbol lookup error: /usr/lib/libassuan.so.0: undefined 
> > symbol: gpg_err_set_errno

That part I at least figured out. This behavior can be replicated (for me) with:

LD_LIBRARY_PATH=/usr/lib gpg-agent

or:

LD_LIBRARY_PATH=/usr/lib:/lib gpg-agent

And it goes away with:

LD_LIBRARY_PATH=/lib:/usr/lib gpg-agent

It's still a mystifying bug in gpg-agent to me. But I've at least got my
environment to no longer trigger it, and I've opened a Debian bug report
for it at least.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/4dd4149a/attachment.pgp>


[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth  wrote:
> On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins  finestructure.net> wrote:
> > The declaration of the GMimeStream pointer to stdout in
> > format_part_content_text was somehow preventing subsequent printf
> > calls from outputting to stdout if the output was redirected to a
> > file.  Scoping the declaration to the actual use of the stream pointer
> > works around this problem.
> 
> This commit message sounds like we don't actually understand the problem
> being fixed here.

Well actually it's only meant to sound like the committer doesn't
understand the problem!

> I'd like to investigate this more. Perhaps with a test case?

The current tests are how I found the problem!  Without this patch at
least the multipart tests will fail.  I don't see how another test will
add anything.

Carl, if you (or anyone else) understands what the issue is, then please
go ahead and modify the commit message.  I don't understand things
enough myself to do any better.  Clearly there is some strange
interaction with things that try to use stdout after
g_mime_stream_file_new() has already grabbed it.

I really wouldn't block on this, though, since the patch does fix an
actual bug.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/c9d3cac7/attachment.pgp>


[PATCH 1/2] Make `notmuch-show-clean-address' parsing-error-proof.

2011-06-03 Thread Carl Worth
On Thu, 12 May 2011 17:56:25 +0400, Dmitry Kurochkin  wrote:
> Mail-header-parse-address may fail for an invalid address.
> Before the change, this would result in empty notmuch-show buffer
> with an error message like: Scan error: "Unbalanced parentheses".
> The patch wraps the function in condition-case and returns
> unchanged address in case of error.

Thanks for the patch, Dmitry. I've just pushed this.

And thanks as well for the test case. I actually pushed the test-case
commit before the bug-fix commit. I like to do things in that order so
that I can:

1. Apply the new test case
2. Run "make test" and see the failure
3. Apply the bug fix
4. Run "make test" and see the test pass

With a sequence like that, it's very easy for me to feel very
comfortable committing the code.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/89821ae3/attachment-0001.pgp>


[PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

2011-06-03 Thread Carl Worth
On Sat, 04 Jun 2011 00:22:04 +0400, Dmitry Kurochkin  wrote:
> "notmuch was incorecctly detecting this as the ..." is not right.

Thanks. I was sure my commit message wasn't right yet. That was my
point---I didn't know what the right message would be! ;-)

> It is a wrong-type-argument lisp error (evaluating (length '(a . b))).  How
> about:
> 
>   Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
>   `notmuch-fcc-dirs' is set to a list.  The error was in the
>   `notmuch-fcc-dirs' format check which was changed in an incompatible
>   way from 0.4 to 0.5.

Perfect! This way, if I want to investigate this patch before committing
it, I now have the information I need to test it.

Thanks,

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/4137e868/attachment.pgp>


[PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

2011-06-03 Thread Carl Worth
On Thu, 02 Jun 2011 10:49:57 +0400, Dmitry Kurochkin  wrote:
> Well, it says that changes are in notmuch 0.5.  So "old" and "previous"
> refer to pre-0.5 (i.e. 0.4) and "new" refers to 0.5.

Sure, but I happen to ahve already forgotten the details of how the
variable could be configured in 0.4 and in 0.5. More importantly, anyone
in the future reading the commit log is much more likely not to
remember.

> Any configuration when `notmuch-fcc-dirs' is a list.  That variable has
> a nice documentation.

Again, I'd like our commit messages to be self-contained. They are much
more useful if the describe the change being made without assuming to
much outside knowledge.

> > It would be easier to understand the code if there were a corresponding
> > test case for it.
...
> I do not think we need a test for this fix.  What we need are tests for
> FCC functionality when notmuch-fcc-dirs is a list.

Yes!

> Old configuration format was changed in 0.5 in an incompatible way.
> There is a check for the unsupported old-style configuration.  But the
> check is broken and results in an error when running with a valid
> new-style configuration.

This is actually what I meant by "corresponding test case". If the bug
here is that a "new-style configuration" doesn't work , (and I still
don't like that wording---don't say "new style"---explain what it
actually *is*), then yes, we need a test case showing that bug.

> I am not sure what you expect from the commit message here.  IMO it is
> enough for this small bugfix and those who interested can always refer
> to documentation for details.

The commit message should provide a self-contained description of the
change. It should be along the lines of:

When fcc-dirs is set to
 notmuch was
incorecctly detecting this as the
 and generating an error
message. Fix the test so that this configuration now works.

Where the  above should be replaced with actual descriptions,
not relative pointers to information like "old style" or "new style".

Does that make sense?

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/f4538682/attachment.pgp>


[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins  wrote:
> The declaration of the GMimeStream pointer to stdout in
> format_part_content_text was somehow preventing subsequent printf
> calls from outputting to stdout if the output was redirected to a
> file.  Scoping the declaration to the actual use of the stream pointer
> works around this problem.

This commit message sounds like we don't actually understand the problem
being fixed here.

I'd like to investigate this more. Perhaps with a test case?

Otherwise, the patches up to this point in the thread have all either
been pushed or I've asked for some additional information (perhaps
that's just this patch and the "old style fcc dirs" patch?).

I'll continue to work through my patch queue as contained in email
messages. I'm finding it easier to do that (and just piping the patches
I like to "git am") as opposed to working through a release-candidate
branch in git, (and having to keep rebasing/postponing it after I reject
some patch in the series).

I'm actually a bit surprised to see myself preferring patches in
email. When Linus first wrote git, I couldn't understand why the Linux
community kept to such a consistent culture of sending patches via
email. It seemed so backwards to do these awkward machinations (git
format-patch, git send-email, SMTP, MUA, git am), and risk all the
problems of email clients corrupting patches, etc.?especially when git
has such clean mechanisms for reliably moving patches around (git push,
git pull).

Call me converted. Email really is the right way to do collaborative
code development.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/48e88dcd/attachment.pgp>


[PATCH 17/25] avoid segfault when calling sanitize_string() on NULL

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:51:52 -0700, Jameson Graef Rollins  wrote:
> +if (NULL == str)
> + return NULL;

I haven't been blocking patches because of this, but can I please ask
everyone to not use the above style?

I understand that the above style is intended to generate a compiler
error in the case of the programmer mistyping '=' where '==' was
intended.

But I just can't stand this style.

It looks so unnatural to me to read "if some_value is some_variable"
instead of the natural "if some_variable is some_value".

Also, gcc is kind enough to warn ("suggest parentheses around assignment
used as truth value") in the case of "if (str = NULL)" anyway, so
there's no actual benefit to the unnatural style.

I really do want our code to be readable, and I think that little things
do make a difference.

Thanks for your attention, (and thanks for your patience if I seem off
my rocker).

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20110603/97f8f7e7/attachment.pgp>


[PATCH] Do not attept to output part raw if part is not GMimePart.

2011-06-03 Thread Jameson Graef Rollins
This was a minor oversite in checking of part type when outputing
content raw.  This was causing gmime was to throw an exception to
stderr.

Unfortunately the gmime exception was not being caught by notmuch, or
the test suite.  I'm not sure if notmuch should have done anything in
this case, but certainly the test suite should be capable of detecting
that something unexpected was output to stderr.
---
This is a cleaner version of this patch.

 notmuch-show.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 9267d02..59f7078 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -644,6 +644,9 @@ format_part_end_json (GMimeObject *part)
 static void
 format_part_content_raw (GMimeObject *part)
 {
+if (! GMIME_IS_PART (part))
+   return;
+
 GMimeStream *stream_stdout;
 GMimeStream *stream_filter = NULL;
 GMimeDataWrapper *wrapper;
-- 
1.7.4.4



Re: [PATCH 17/25] avoid segfault when calling sanitize_string() on NULL

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:51:52 -0700, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 +if (NULL == str)
 + return NULL;

I haven't been blocking patches because of this, but can I please ask
everyone to not use the above style?

I understand that the above style is intended to generate a compiler
error in the case of the programmer mistyping '=' where '==' was
intended.

But I just can't stand this style.

It looks so unnatural to me to read if some_value is some_variable
instead of the natural if some_variable is some_value.

Also, gcc is kind enough to warn (suggest parentheses around assignment
used as truth value) in the case of if (str = NULL) anyway, so
there's no actual benefit to the unnatural style.

I really do want our code to be readable, and I think that little things
do make a difference.

Thanks for your attention, (and thanks for your patience if I seem off
my rocker).

-Carl


pgppLGJuKMVtv.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 The declaration of the GMimeStream pointer to stdout in
 format_part_content_text was somehow preventing subsequent printf
 calls from outputting to stdout if the output was redirected to a
 file.  Scoping the declaration to the actual use of the stream pointer
 works around this problem.

This commit message sounds like we don't actually understand the problem
being fixed here.

I'd like to investigate this more. Perhaps with a test case?

Otherwise, the patches up to this point in the thread have all either
been pushed or I've asked for some additional information (perhaps
that's just this patch and the old style fcc dirs patch?).

I'll continue to work through my patch queue as contained in email
messages. I'm finding it easier to do that (and just piping the patches
I like to git am) as opposed to working through a release-candidate
branch in git, (and having to keep rebasing/postponing it after I reject
some patch in the series).

I'm actually a bit surprised to see myself preferring patches in
email. When Linus first wrote git, I couldn't understand why the Linux
community kept to such a consistent culture of sending patches via
email. It seemed so backwards to do these awkward machinations (git
format-patch, git send-email, SMTP, MUA, git am), and risk all the
problems of email clients corrupting patches, etc.—especially when git
has such clean mechanisms for reliably moving patches around (git push,
git pull).

Call me converted. Email really is the right way to do collaborative
code development.

-Carl


pgpizI5odauir.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

2011-06-03 Thread Dmitry Kurochkin
On Fri, 03 Jun 2011 13:05:00 -0700, Carl Worth cwo...@cworth.org wrote:
Non-text part: multipart/signed
 On Thu, 02 Jun 2011 10:49:57 +0400, Dmitry Kurochkin 
 dmitry.kuroch...@gmail.com wrote:
  Well, it says that changes are in notmuch 0.5.  So old and previous
  refer to pre-0.5 (i.e. 0.4) and new refers to 0.5.
 
 Sure, but I happen to ahve already forgotten the details of how the
 variable could be configured in 0.4 and in 0.5. More importantly, anyone
 in the future reading the commit log is much more likely not to
 remember.
 
  Any configuration when `notmuch-fcc-dirs' is a list.  That variable has
  a nice documentation.
 
 Again, I'd like our commit messages to be self-contained. They are much
 more useful if the describe the change being made without assuming to
 much outside knowledge.
 
   It would be easier to understand the code if there were a corresponding
   test case for it.
 ...
  I do not think we need a test for this fix.  What we need are tests for
  FCC functionality when notmuch-fcc-dirs is a list.
 
 Yes!
 
  Old configuration format was changed in 0.5 in an incompatible way.
  There is a check for the unsupported old-style configuration.  But the
  check is broken and results in an error when running with a valid
  new-style configuration.
 
 This is actually what I meant by corresponding test case. If the bug
 here is that a new-style configuration doesn't work , (and I still
 don't like that wording---don't say new style---explain what it
 actually *is*), then yes, we need a test case showing that bug.
 
  I am not sure what you expect from the commit message here.  IMO it is
  enough for this small bugfix and those who interested can always refer
  to documentation for details.
 
 The commit message should provide a self-contained description of the
 change. It should be along the lines of:
 
   When fcc-dirs is set to
   some-particular-datatype-that-should-work notmuch was
   incorecctly detecting this as the
   old-style-that-is-no-longer-supported and generating an error
   message. Fix the test so that this configuration now works.
 
 Where the phrases above should be replaced with actual descriptions,
 not relative pointers to information like old style or new style.
 
 Does that make sense?
 

notmuch was incorecctly detecting this as the ... is not right.  It is
a wrong-type-argument lisp error (evaluating (length '(a . b))).  How
about:

  Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
  `notmuch-fcc-dirs' is set to a list.  The error was in the
  `notmuch-fcc-dirs' format check which was changed in an incompatible
  way from 0.4 to 0.5.

Regards,
  Dmitry

 -Carl
Non-text part: application/pgp-signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 14/25] Fix old style notmuch-fcc-dirs configuration check.

2011-06-03 Thread Carl Worth
On Sat, 04 Jun 2011 00:22:04 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 notmuch was incorecctly detecting this as the ... is not right.

Thanks. I was sure my commit message wasn't right yet. That was my
point---I didn't know what the right message would be! ;-)

 It is a wrong-type-argument lisp error (evaluating (length '(a . b))).  How
 about:
 
   Fix wrong-type-argument lisp error in `notmuch-fcc-header-setup' when
   `notmuch-fcc-dirs' is set to a list.  The error was in the
   `notmuch-fcc-dirs' format check which was changed in an incompatible
   way from 0.4 to 0.5.

Perfect! This way, if I want to investigate this patch before committing
it, I now have the information I need to test it.

Thanks,

-Carl

-- 
carl.d.wo...@intel.com


pgp7gPKlAp0NC.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth cwo...@cworth.org wrote:
 On Sat, 28 May 2011 14:52:00 -0700, Jameson Graef Rollins 
 jroll...@finestructure.net wrote:
  The declaration of the GMimeStream pointer to stdout in
  format_part_content_text was somehow preventing subsequent printf
  calls from outputting to stdout if the output was redirected to a
  file.  Scoping the declaration to the actual use of the stream pointer
  works around this problem.
 
 This commit message sounds like we don't actually understand the problem
 being fixed here.

Well actually it's only meant to sound like the committer doesn't
understand the problem!

 I'd like to investigate this more. Perhaps with a test case?

The current tests are how I found the problem!  Without this patch at
least the multipart tests will fail.  I don't see how another test will
add anything.

Carl, if you (or anyone else) understands what the issue is, then please
go ahead and modify the commit message.  I don't understand things
enough myself to do any better.  Clearly there is some strange
interaction with things that try to use stdout after
g_mime_stream_file_new() has already grabbed it.

I really wouldn't block on this, though, since the patch does fix an
actual bug.

jamie.


pgpDZan2dZhJr.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: normalizing part numbering across PGP/MIME processing

2011-06-03 Thread Carl Worth
On Sat, 28 May 2011 14:31:08 -0700, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 On Fri, 27 May 2011 17:53:44 -0700, Carl Worth cwo...@cworth.org wrote:
* Should we set the crypto option to verify/decrypt by default?
...
 I don't really have an opinion on this.  I have it set now, so whether
 or not it's set by default doesn't make much difference to me.

I'm inclined to set it by default. But I'll wait until I get it fully
working, (which looks to be problems in my environment). See below.

* I can't actually get decryption to work for me. :-(
  
  When I run notmuch show --decrypt on a message encrypted with
  my public key I get a segfault within libgmime, specifically in
  the g_mime_session_request_passwd function.

I'm still getting this. I'll start debugging libgmime next.

  $ gpg-agent
  gpg-agent: symbol lookup error: /usr/lib/libassuan.so.0: undefined 
  symbol: gpg_err_set_errno

That part I at least figured out. This behavior can be replicated (for me) with:

LD_LIBRARY_PATH=/usr/lib gpg-agent

or:

LD_LIBRARY_PATH=/usr/lib:/lib gpg-agent

And it goes away with:

LD_LIBRARY_PATH=/lib:/usr/lib gpg-agent

It's still a mystifying bug in gpg-agent to me. But I've at least got my
environment to no longer trigger it, and I've opened a Debian bug report
for it at least.

-Carl


pgpJYUZppOY4g.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 12:56:50 -0700, Carl Worth cwo...@cworth.org wrote:
 Otherwise, the patches up to this point in the thread have all either
 been pushed or I've asked for some additional information (perhaps
 that's just this patch and the old style fcc dirs patch?).

Great!  That's really great news, Carl.  Thank you very much.  I think
at this point the only thing we need for 0.6 are:

* Austin's atomicity patches
* maybe fix the frc822 handling issue

Can we set a target date for 0.6 release?  So we'll all start feeling
really bad if we miss it?

 I'm actually a bit surprised to see myself preferring patches in
 email. When Linus first wrote git, I couldn't understand why the Linux
 community kept to such a consistent culture of sending patches via
 email. It seemed so backwards to do these awkward machinations (git
 format-patch, git send-email, SMTP, MUA, git am), and risk all the
 problems of email clients corrupting patches, etc.—especially when git
 has such clean mechanisms for reliably moving patches around (git push,
 git pull).

Yeah, I'm with you that I'm surprised by liking this method as well.  I
think it only really works once most of the bulk of things are already
working, but for bug fixes and feature additions it really works well.
It's nice to have comments for patches on list.

jamie.


pgp5zz3wPc2IA.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Carl Worth
On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 Well actually it's only meant to sound like the committer doesn't
 understand the problem!

Heh, OK.

  I'd like to investigate this more. Perhaps with a test case?
 
 The current tests are how I found the problem!  Without this patch at
 least the multipart tests will fail.  I don't see how another test will
 add anything.

Ah, in my review I'd managed to get this commit detached from the
original previous commit that introduced the test failures. It's funny
that while you were replying I was reviewing *that* commit and thinking,
why are all these tests failing now just because they changed to
test_expect_equal_file?

 Carl, if you (or anyone else) understands what the issue is, then please
 go ahead and modify the commit message.

Done.

 I don't understand things
 enough myself to do any better.  Clearly there is some strange
 interaction with things that try to use stdout after
 g_mime_stream_file_new() has already grabbed it.

g_mime_stream_file_new is a bad citizen, API-wise. It closes files that
it didn't open, (by default).

 I really wouldn't block on this, though, since the patch does fix an
 actual bug.

Not blocked. All pushed. New commit message below for reference.

-Carl

commit d5b4d950245605b84c56ce991fa3c59a073a70e5
Author: Jameson Graef Rollins jroll...@finestructure.net
Date:   Sat May 28 14:52:00 2011 -0700

show: Avoid inadvertently closing stdout

GMime has a nasty habit of taking ownership by default of any FILE*
handed to it va g_mime_stream_file_new. Specifically it will close the
FILE* when the stream is destroyed---even though GMime didn't open the
file itself.

To avoid this bad behavior, we have to carefully set_owner(FALSE)
after calling g_mime_stream_file_new. In the format_part_content_text
function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
been calling g_mime_stream_file_new unconditionally, but only calling
g_mime_stream_file_set_owner(FALSE) conditionally.

This led to the FILE* being closed early when notmuch show output was
redirected to a file.

Fixing this fixes the test-suite cases that broke with the previous
commit, (which added redirected notmuch show calls to the test suite
to expose this bug).

Edited-by: Carl Worth cwo...@cworth.org with a new commit message to
explain the bug and fix.


pgpLSzt2ZnU4W.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text

2011-06-03 Thread Jameson Graef Rollins
On Fri, 03 Jun 2011 15:38:29 -0700, Carl Worth cwo...@cworth.org wrote:
 commit d5b4d950245605b84c56ce991fa3c59a073a70e5
 Author: Jameson Graef Rollins jroll...@finestructure.net
 Date:   Sat May 28 14:52:00 2011 -0700
 
 show: Avoid inadvertently closing stdout
 
 GMime has a nasty habit of taking ownership by default of any FILE*
 handed to it va g_mime_stream_file_new. Specifically it will close the
 FILE* when the stream is destroyed---even though GMime didn't open the
 file itself.
 
 To avoid this bad behavior, we have to carefully set_owner(FALSE)
 after calling g_mime_stream_file_new. In the format_part_content_text
 function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
 been calling g_mime_stream_file_new unconditionally, but only calling
 g_mime_stream_file_set_owner(FALSE) conditionally.
 
 This led to the FILE* being closed early when notmuch show output was
 redirected to a file.
 
 Fixing this fixes the test-suite cases that broke with the previous
 commit, (which added redirected notmuch show calls to the test suite
 to expose this bug).
 
 Edited-by: Carl Worth cwo...@cworth.org with a new commit message to
 explain the bug and fix.

Now I sound like I know what I'm talking about!  Thanks, Carl.

jamie.


pgptPc1mWLDnO.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Daniel Kahn Gillmor
The notmuch binary is not in the business of doing interactive
prompting with the user.  If credentials are needed for decryption,
they should be supplied to the decrypting processes some other way
(e.g. gpg-agent).

Previously, we returned a NULL function pointer for the
request_passwd() function, which may have cause segmentation faults
with some versions of gmime.
---
 notmuch-gmime-session.c |   17 -
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/notmuch-gmime-session.c b/notmuch-gmime-session.c
index d83d9b3..33f2817 100644
--- a/notmuch-gmime-session.c
+++ b/notmuch-gmime-session.c
@@ -39,11 +39,26 @@ notmuch_gmime_session_get_type (void)
 return type;
 }
 
+/* 
+   notmuch never prompts the user for a password.  It always returns
+   an empty string (could it return a NULL pointer instead?)
+
+   If credentials are needed for crypto, they should be supplied via
+   other mechanisms (e.g. gpg-agent, etc)
+ */
+static char *never_request_passwd (unused (GMimeSession *session), unused 
(const char *prompt),
+  unused (gboolean secret), unused (const char 
*item),
+  unused (GError **err)) {
+fprintf (stderr, Credentials needed for crypto; Please use gpg-agent.\n);
+return g_strdup ();
+}
+
+
 static void
 notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass)
 {
 GMimeSessionClass *session_class = GMIME_SESSION_CLASS (klass);
 parent_class = g_type_class_ref (GMIME_TYPE_SESSION);
-session_class-request_passwd = NULL;
+session_class-request_passwd = never_request_passwd;
 }
 /* END CRUFTY BOILERPLATE */
-- 
1.7.4.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Carl Worth
On Fri,  3 Jun 2011 19:03:08 -0400, Daniel Kahn Gillmor 
d...@fifthhorseman.net wrote:
 The notmuch binary is not in the business of doing interactive
 prompting with the user.  If credentials are needed for decryption,
 they should be supplied to the decrypting processes some other way
 (e.g. gpg-agent).
 
 Previously, we returned a NULL function pointer for the
 request_passwd() function, which may have cause segmentation faults
 with some versions of gmime.

Cool. This fixes my segfaults, so thanks!

 +return g_strdup ();

Is the above correct? Or is it a memory leak? (If it's not a leak, then
GMime really has some bizarre ownership semantics.)

-Carl


pgpTQDPA2lLhQ.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Always return the empty string if decryption tries to demand a password

2011-06-03 Thread Daniel Kahn Gillmor
On 06/03/2011 07:15 PM, Carl Worth wrote:
 On Fri,  3 Jun 2011 19:03:08 -0400, Daniel Kahn Gillmor 
 d...@fifthhorseman.net wrote:
 The notmuch binary is not in the business of doing interactive
 prompting with the user.  If credentials are needed for decryption,
 they should be supplied to the decrypting processes some other way
 (e.g. gpg-agent).

 Previously, we returned a NULL function pointer for the
 request_passwd() function, which may have cause segmentation faults
 with some versions of gmime.
 
 Cool. This fixes my segfaults, so thanks!
 
 +return g_strdup ();
 
 Is the above correct? Or is it a memory leak? (If it's not a leak, then
 GMime really has some bizarre ownership semantics.)

yes, this corner of gmime has some really bizarre ownership semantics;
twisty handoffs and callbacks abound :(

Hm, actually, we should just be returning NULL to indicate a failure; i
think that would be preferable, and apparently is documented to be
acceptable:

 
http://developer.gnome.org/gmime/stable/GMimeSession.html#g-mime-session-request-passwd

Would you mind amending that patch to just return NULL ?

fwiw, i've just filed https://bugzilla.gnome.org/show_bug.cgi?id=651826
to ask gmime for a hook to let us always request the use of gpg-agent if
it is available.

--dkg



signature.asc
Description: OpenPGP digital signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Use stock GMimeSession by default

2011-06-03 Thread Daniel Kahn Gillmor
Our use of GMimeSession was unneeded boilerplate, and we weren't doing
anything with it.  This simplifies and clarifies that assumption.

If we want to do anything fancier later, the examples in the gmime
source are a reasonable source to work from in defining a new
GMimeSession derivative.

Since GMimeSession is going away in GMime 2.6, though, i don't
recommend using it.
---
 Makefile.local  |1 -
 notmuch-client.h|3 --
 notmuch-gmime-session.c |   49 ---
 notmuch-reply.c |2 +-
 notmuch-show.c  |2 +-
 5 files changed, 2 insertions(+), 55 deletions(-)
 delete mode 100644 notmuch-gmime-session.c

diff --git a/Makefile.local b/Makefile.local
index f726f1f..8a8832d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -246,7 +246,6 @@ notmuch_client_srcs =   \
notmuch-show.c  \
notmuch-tag.c   \
notmuch-time.c  \
-   notmuch-gmime-session.c \
query-string.c  \
show-message.c  \
json.c  \
diff --git a/notmuch-client.h b/notmuch-client.h
index 8a27260..63be337 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -237,7 +237,4 @@ notmuch_config_set_maildir_synchronize_flags 
(notmuch_config_t *config,
 notmuch_bool_t
 debugger_is_active (void);
 
-GType
-notmuch_gmime_session_get_type (void);
-
 #endif
diff --git a/notmuch-gmime-session.c b/notmuch-gmime-session.c
deleted file mode 100644
index d83d9b3..000
--- a/notmuch-gmime-session.c
+++ /dev/null
@@ -1,49 +0,0 @@
-#include notmuch-client.h
-
-/* CRUFTY BOILERPLATE for GMimeSession (dkg thinks this will go away once 
GMime 2.6 comes out) */
-typedef struct _NotmuchGmimeSession NotmuchGmimeSession;
-typedef struct _NotmuchGmimeSessionClass NotmuchGmimeSessionClass;
-
-struct _NotmuchGmimeSession {
-GMimeSession parent_object;
-};
-
-struct _NotmuchGmimeSessionClass {
-GMimeSessionClass parent_class;
-};
-
-static void notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass);
-
-static GMimeSessionClass *parent_class = NULL;
-
-GType
-notmuch_gmime_session_get_type (void)
-{
-static GType type = 0;
-
-if (!type) {
-   static const GTypeInfo info = {
-   sizeof (NotmuchGmimeSessionClass),
-   NULL, /* base_class_init */
-   NULL, /* base_class_finalize */
-   (GClassInitFunc) notmuch_gmime_session_class_init,
-   NULL, /* class_finalize */
-   NULL, /* class_data */
-   sizeof (NotmuchGmimeSession),
-   0,/* n_preallocs */
-   NULL, /* object_init */
-   NULL, /* value_table */
-   };
-   type = g_type_register_static (GMIME_TYPE_SESSION, 
NotmuchGmimeSession, info, 0);
-}
-return type;
-}
-
-static void
-notmuch_gmime_session_class_init (NotmuchGmimeSessionClass *klass)
-{
-GMimeSessionClass *session_class = GMIME_SESSION_CLASS (klass);
-parent_class = g_type_class_ref (GMIME_TYPE_SESSION);
-session_class-request_passwd = NULL;
-}
-/* END CRUFTY BOILERPLATE */
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 5265af6..514bbc6 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -607,7 +607,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
}
} else if ((STRNCMP_LITERAL (argv[i], --decrypt) == 0)) {
if (params.cryptoctx == NULL) {
-   GMimeSession* session = 
g_object_new(notmuch_gmime_session_get_type(), NULL);
+   GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
gpg)))
fprintf (stderr, Failed to construct gpg context.\n);
else
diff --git a/notmuch-show.c b/notmuch-show.c
index 9267d02..dda83a1 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -899,7 +899,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused 
(char *argv[]))
} else if ((STRNCMP_LITERAL (argv[i], --verify) == 0) ||
   (STRNCMP_LITERAL (argv[i], --decrypt) == 0)) {
if (params.cryptoctx == NULL) {
-   GMimeSession* session = 
g_object_new(notmuch_gmime_session_get_type(), NULL);
+   GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
gpg)))
fprintf (stderr, Failed to construct gpg context.\n);
else
-- 
1.7.4.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Use stock GMimeSession by default

2011-06-03 Thread Daniel Kahn Gillmor

This patch supercedes my patch from
1307142188-6551-1-git-send-email-...@fifthhorseman.net , and provides
what appears to be a much cleaner approach to achieve the same result.

 --dkg
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch