unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/2] cli: notmuch dump abstractions
@ 2014-03-25 17:07 Jani Nikula
  2014-03-25 17:07 ` [PATCH 1/2] cli: abstract database dumping from the dump command Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Jani Nikula @ 2014-03-25 17:07 UTC (permalink / raw)
  To: notmuch

These are non-functional prep work for letting us do database dumps from
elsewhere in notmuch, e.g. automatically before a database upgrade.

BR,
Jani.


Jani Nikula (2):
  cli: abstract database dumping from the dump command
  cli: abstract dump file open from the dump command

 notmuch-dump.c | 120 +++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 74 insertions(+), 46 deletions(-)

-- 
1.9.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/2] cli: abstract database dumping from the dump command
  2014-03-25 17:07 [PATCH 0/2] cli: notmuch dump abstractions Jani Nikula
@ 2014-03-25 17:07 ` Jani Nikula
  2014-03-25 17:07 ` [PATCH 2/2] cli: abstract dump file open " Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2014-03-25 17:07 UTC (permalink / raw)
  To: notmuch

No functional changes, except for slight improvement in error
handling.
---
 notmuch-dump.c | 107 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 61 insertions(+), 46 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 158142f55b49..179e2e97bf61 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -22,56 +22,17 @@
 #include "dump-restore-private.h"
 #include "string-util.h"
 
-int
-notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
+static int
+database_dump_file (notmuch_database_t *notmuch, FILE *output,
+		    const char *query_str, int output_format)
 {
-    notmuch_database_t *notmuch;
     notmuch_query_t *query;
-    FILE *output = stdout;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     notmuch_tags_t *tags;
-    const char *query_str = "";
-
-    if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
-	return EXIT_FAILURE;
-
-    char *output_file_name = NULL;
-    int opt_index;
-
-    int output_format = DUMP_FORMAT_BATCH_TAG;
 
-    notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
-	  (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
-				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
-				  { 0, 0 } } },
-	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
-	{ 0, 0, 0, 0, 0 }
-    };
-
-    opt_index = parse_arguments (argc, argv, options, 1);
-    if (opt_index < 0)
-	return EXIT_FAILURE;
-
-    if (output_file_name) {
-	output = fopen (output_file_name, "w");
-	if (output == NULL) {
-	    fprintf (stderr, "Error opening %s for writing: %s\n",
-		     output_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
-    }
-
-
-    if (opt_index < argc) {
-	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
-	if (query_str == NULL) {
-	    fprintf (stderr, "Out of memory.\n");
-	    return EXIT_FAILURE;
-	}
-    }
+    if (! query_str)
+	query_str = "";
 
     query = notmuch_query_create (notmuch, query_str);
     if (query == NULL) {
@@ -149,11 +110,65 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 	notmuch_message_destroy (message);
     }
 
+    notmuch_query_destroy (query);
+
+    return EXIT_SUCCESS;
+}
+
+int
+notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
+{
+    notmuch_database_t *notmuch;
+    FILE *output = stdout;
+    const char *query_str = NULL;
+    int ret;
+
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
+	return EXIT_FAILURE;
+
+    char *output_file_name = NULL;
+    int opt_index;
+
+    int output_format = DUMP_FORMAT_BATCH_TAG;
+
+    notmuch_opt_desc_t options[] = {
+	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
+	  (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
+				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
+				  { 0, 0 } } },
+	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
+	{ 0, 0, 0, 0, 0 }
+    };
+
+    opt_index = parse_arguments (argc, argv, options, 1);
+    if (opt_index < 0)
+	return EXIT_FAILURE;
+
+    if (output_file_name) {
+	output = fopen (output_file_name, "w");
+	if (output == NULL) {
+	    fprintf (stderr, "Error opening %s for writing: %s\n",
+		     output_file_name, strerror (errno));
+	    return EXIT_FAILURE;
+	}
+    }
+
+
+    if (opt_index < argc) {
+	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
+	if (query_str == NULL) {
+	    fprintf (stderr, "Out of memory.\n");
+	    return EXIT_FAILURE;
+	}
+    }
+
+    ret = database_dump_file (notmuch, output, query_str, output_format);
+
     if (output != stdout)
 	fclose (output);
 
-    notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
-    return EXIT_SUCCESS;
+    return ret;
 }
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/2] cli: abstract dump file open from the dump command
  2014-03-25 17:07 [PATCH 0/2] cli: notmuch dump abstractions Jani Nikula
  2014-03-25 17:07 ` [PATCH 1/2] cli: abstract database dumping from the dump command Jani Nikula
@ 2014-03-25 17:07 ` Jani Nikula
  2014-03-25 17:48   ` [PATCH v2] " Jani Nikula
  2014-03-26 22:01 ` [PATCH 0/2] cli: notmuch dump abstractions Mark Walters
  2014-03-30 22:33 ` [PATCH 0/2] cli: notmuch dump abstractions David Bremner
  3 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2014-03-25 17:07 UTC (permalink / raw)
  To: notmuch

No functional changes, except for slight improvement in error
handling.
---
 notmuch-dump.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 179e2e97bf61..88bef3f77e82 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -115,11 +115,37 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
     return EXIT_SUCCESS;
 }
 
+/* Dump database into output_file_name if it's non-NULL, stdout
+ * otherwise.
+ */
+static int
+database_dump (notmuch_database_t *notmuch, const char *output_file_name,
+	       const char *query_str, int output_format)
+{
+    FILE *output = stdout;
+    int ret;
+
+    if (output_file_name) {
+	output = fopen (output_file_name, "w");
+	if (output == NULL) {
+	    fprintf (stderr, "Error opening %s for writing: %s\n",
+		     output_file_name, strerror (errno));
+	    return EXIT_FAILURE;
+	}
+    }
+
+    ret = database_dump_file (notmuch, output, query_str, output_format);
+
+    if (output != stdout)
+	fclose (output);
+
+    return ret;
+}
+
 int
 notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    FILE *output = stdout;
     const char *query_str = NULL;
     int ret;
 
@@ -145,16 +171,6 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
-    if (output_file_name) {
-	output = fopen (output_file_name, "w");
-	if (output == NULL) {
-	    fprintf (stderr, "Error opening %s for writing: %s\n",
-		     output_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
-    }
-
-
     if (opt_index < argc) {
 	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
 	if (query_str == NULL) {
@@ -163,10 +179,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 	}
     }
 
-    ret = database_dump_file (notmuch, output, query_str, output_format);
-
-    if (output != stdout)
-	fclose (output);
+    ret = database_dump (notmuch, output_file_name, query_str, output_format);
 
     notmuch_database_destroy (notmuch);
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2] cli: abstract dump file open from the dump command
  2014-03-25 17:07 ` [PATCH 2/2] cli: abstract dump file open " Jani Nikula
@ 2014-03-25 17:48   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2014-03-25 17:48 UTC (permalink / raw)
  To: notmuch

Also expose the dump function to the rest of notmuch. No functional
changes, except for slight improvement in error handling.
---
 dump-restore-private.h | 13 -------------
 notmuch-client.h       | 11 +++++++++++
 notmuch-dump.c         | 47 +++++++++++++++++++++++++++++++----------------
 notmuch-restore.c      |  2 +-
 4 files changed, 43 insertions(+), 30 deletions(-)
 delete mode 100644 dump-restore-private.h

diff --git a/dump-restore-private.h b/dump-restore-private.h
deleted file mode 100644
index 896a00430986..000000000000
--- a/dump-restore-private.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef DUMP_RESTORE_PRIVATE_H
-#define DUMP_RESTORE_PRIVATE_H
-
-#include "hex-escape.h"
-#include "command-line-arguments.h"
-
-typedef enum dump_formats {
-    DUMP_FORMAT_AUTO,
-    DUMP_FORMAT_BATCH_TAG,
-    DUMP_FORMAT_SUP
-} dump_format_t;
-
-#endif
diff --git a/notmuch-client.h b/notmuch-client.h
index 278b498a246a..d11064824ddf 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -441,5 +441,16 @@ mime_node_child (mime_node_t *parent, int child);
 mime_node_t *
 mime_node_seek_dfs (mime_node_t *node, int n);
 
+typedef enum dump_formats {
+    DUMP_FORMAT_AUTO,
+    DUMP_FORMAT_BATCH_TAG,
+    DUMP_FORMAT_SUP
+} dump_format_t;
+
+int
+notmuch_database_dump (notmuch_database_t *notmuch,
+		       const char *output_file_name,
+		       const char *query_str, dump_format_t output_format);
+
 #include "command-line-arguments.h"
 #endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 179e2e97bf61..21702d793bcf 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -19,7 +19,7 @@
  */
 
 #include "notmuch-client.h"
-#include "dump-restore-private.h"
+#include "hex-escape.h"
 #include "string-util.h"
 
 static int
@@ -115,11 +115,38 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
     return EXIT_SUCCESS;
 }
 
+/* Dump database into output_file_name if it's non-NULL, stdout
+ * otherwise.
+ */
+int
+notmuch_database_dump (notmuch_database_t *notmuch,
+		       const char *output_file_name,
+		       const char *query_str, dump_format_t output_format)
+{
+    FILE *output = stdout;
+    int ret;
+
+    if (output_file_name) {
+	output = fopen (output_file_name, "w");
+	if (output == NULL) {
+	    fprintf (stderr, "Error opening %s for writing: %s\n",
+		     output_file_name, strerror (errno));
+	    return EXIT_FAILURE;
+	}
+    }
+
+    ret = database_dump_file (notmuch, output, query_str, output_format);
+
+    if (output != stdout)
+	fclose (output);
+
+    return ret;
+}
+
 int
 notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    FILE *output = stdout;
     const char *query_str = NULL;
     int ret;
 
@@ -145,16 +172,6 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
-    if (output_file_name) {
-	output = fopen (output_file_name, "w");
-	if (output == NULL) {
-	    fprintf (stderr, "Error opening %s for writing: %s\n",
-		     output_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
-    }
-
-
     if (opt_index < argc) {
 	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
 	if (query_str == NULL) {
@@ -163,10 +180,8 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 	}
     }
 
-    ret = database_dump_file (notmuch, output, query_str, output_format);
-
-    if (output != stdout)
-	fclose (output);
+    ret = notmuch_database_dump (notmuch, output_file_name, query_str,
+				 output_format);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/notmuch-restore.c b/notmuch-restore.c
index f23ab983a93d..c54d51328b51 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -19,7 +19,7 @@
  */
 
 #include "notmuch-client.h"
-#include "dump-restore-private.h"
+#include "hex-escape.h"
 #include "tag-util.h"
 #include "string-util.h"
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/2] cli: notmuch dump abstractions
  2014-03-25 17:07 [PATCH 0/2] cli: notmuch dump abstractions Jani Nikula
  2014-03-25 17:07 ` [PATCH 1/2] cli: abstract database dumping from the dump command Jani Nikula
  2014-03-25 17:07 ` [PATCH 2/2] cli: abstract dump file open " Jani Nikula
@ 2014-03-26 22:01 ` Mark Walters
  2014-03-29  1:20   ` [PATCH] RFC: impliment gzipped output for notmuch dump David Bremner
  2014-03-30 22:33 ` [PATCH 0/2] cli: notmuch dump abstractions David Bremner
  3 siblings, 1 reply; 23+ messages in thread
From: Mark Walters @ 2014-03-26 22:01 UTC (permalink / raw)
  To: Jani Nikula, notmuch


This series (with v2 of patch 2/2) LGTM +1

Best wishes

Mark



On Tue, 25 Mar 2014, Jani Nikula <jani@nikula.org> wrote:
> These are non-functional prep work for letting us do database dumps from
> elsewhere in notmuch, e.g. automatically before a database upgrade.
>
> BR,
> Jani.
>
>
> Jani Nikula (2):
>   cli: abstract database dumping from the dump command
>   cli: abstract dump file open from the dump command
>
>  notmuch-dump.c | 120 +++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 74 insertions(+), 46 deletions(-)
>
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-26 22:01 ` [PATCH 0/2] cli: notmuch dump abstractions Mark Walters
@ 2014-03-29  1:20   ` David Bremner
  2014-03-29  7:16     ` Tomi Ollila
                       ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29  1:20 UTC (permalink / raw)
  To: notmuch

---

This obviously needs at least one piece of tidying, and some tests.

My motivation here is the ability to make gzipped backups from within
notmuch, e.g. as part of an upgrade procedure.


 Makefile.local   |  2 +-
 configure        | 19 ++++++++++++++++++-
 notmuch-client.h |  4 +++-
 notmuch-dump.c   | 41 +++++++++++++++++++++++++++--------------
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index cb7b106..e5a20a7 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
 # Smash together user's values with our extra values
 FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CPPFLAGS) $(CFLAGS) $(WARN_CFLAGS) $(extra_cflags) $(CONFIGURE_CFLAGS)
 FINAL_CXXFLAGS = $(CPPFLAGS) $(CXXFLAGS) $(WARN_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) $(CONFIGURE_CXXFLAGS)
-FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
+FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(ZLIB_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
 FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
diff --git a/configure b/configure
index 1d430b9..02ef785 100755
--- a/configure
+++ b/configure
@@ -340,6 +340,18 @@ else
     errors=$((errors + 1))
 fi
 
+printf "Checking for zlib development files... "
+have_zlib=0
+if pkg-config --exists zlib; then
+    printf "Yes.\n"
+    have_zlib=1
+    zlib_cflags=$(pkg-config --cflags zlib)
+    zlib_ldflags=$(pkg-config --libs zlib)
+else
+    printf "No.\n"
+    errors=$((errors + 1))
+fi
+
 printf "Checking for talloc development files... "
 if pkg-config --exists talloc; then
     printf "Yes.\n"
@@ -844,6 +856,10 @@ XAPIAN_LDFLAGS = ${xapian_ldflags}
 GMIME_CFLAGS = ${gmime_cflags}
 GMIME_LDFLAGS = ${gmime_ldflags}
 
+# Flags needed to compile and link against zlib
+ZLIB_CFLAGS = ${zlib_cflags}
+ZLIB_LDFLAGS = ${zlib_ldflags}
+
 # Flags needed to compile and link against talloc
 TALLOC_CFLAGS = ${talloc_cflags}
 TALLOC_LDFLAGS = ${talloc_ldflags}
@@ -882,6 +898,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
+		     \$(ZLIB_CFLAGS)					 \\
 		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
 		     -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)               \\
@@ -892,5 +909,5 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
 		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
-CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
+CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
 EOF
diff --git a/notmuch-client.h b/notmuch-client.h
index d110648..e1efbe0 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -450,7 +450,9 @@ typedef enum dump_formats {
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format);
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output);
 
 #include "command-line-arguments.h"
 #endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 21702d7..029f90a 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -21,10 +21,12 @@
 #include "notmuch-client.h"
 #include "hex-escape.h"
 #include "string-util.h"
+#include <zlib.h>
+
 
 static int
-database_dump_file (notmuch_database_t *notmuch, FILE *output,
-		    const char *query_str, int output_format)
+database_dump_file (notmuch_database_t *notmuch, gzFile output,
+			const char *query_str, int output_format)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
@@ -69,7 +71,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fprintf (output, "%s (", message_id);
+	    gzprintf (output, "%s (", message_id);
 	}
 
 	for (tags = notmuch_message_get_tags (message);
@@ -78,12 +80,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	    const char *tag_str = notmuch_tags_get (tags);
 
 	    if (! first)
-		fputs (" ", output);
+		gzputs (output, " ");
 
 	    first = 0;
 
 	    if (output_format == DUMP_FORMAT_SUP) {
-		fputs (tag_str, output);
+		gzputs (output, tag_str);
 	    } else {
 		if (hex_encode (notmuch, tag_str,
 				&buffer, &buffer_size) != HEX_SUCCESS) {
@@ -91,12 +93,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     tag_str);
 		    return EXIT_FAILURE;
 		}
-		fprintf (output, "+%s", buffer);
+		gzprintf (output, "+%s", buffer);
 	    }
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fputs (")\n", output);
+	    gzputs (output, ")\n");
 	} else {
 	    if (make_boolean_term (notmuch, "id", message_id,
 				   &buffer, &buffer_size)) {
@@ -104,7 +106,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     message_id, strerror (errno));
 		    return EXIT_FAILURE;
 	    }
-	    fprintf (output, " -- %s\n", buffer);
+	    gzprintf (output, " -- %s\n", buffer);
 	}
 
 	notmuch_message_destroy (message);
@@ -121,24 +123,33 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format)
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output)
 {
-    FILE *output = stdout;
+    gzFile output;
+    const char *mode = gzip_output ? "w9" : "wT";
+
     int ret;
 
     if (output_file_name) {
-	output = fopen (output_file_name, "w");
+	output = gzopen (output_file_name, mode);
 	if (output == NULL) {
 	    fprintf (stderr, "Error opening %s for writing: %s\n",
 		     output_file_name, strerror (errno));
 	    return EXIT_FAILURE;
 	}
+    } else {
+	output = gzdopen (fileno (stdout), mode);
     }
 
     ret = database_dump_file (notmuch, output, query_str, output_format);
 
-    if (output != stdout)
-	fclose (output);
+    /* XXX check error return */
+    gzflush (output, Z_FINISH);
+
+    if (output_file_name)
+	gzclose_w (output);
 
     return ret;
 }
@@ -158,6 +169,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
 
     int output_format = DUMP_FORMAT_BATCH_TAG;
+    notmuch_bool_t gzip_output = 0;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
@@ -165,6 +177,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
+	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -181,7 +194,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     ret = notmuch_database_dump (notmuch, output_file_name, query_str,
-				 output_format);
+				 output_format, gzip_output);
 
     notmuch_database_destroy (notmuch);
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-29  1:20   ` [PATCH] RFC: impliment gzipped output for notmuch dump David Bremner
@ 2014-03-29  7:16     ` Tomi Ollila
  2014-03-29  9:25     ` Jani Nikula
  2014-03-29 13:46     ` [PATCH] dump: support gzipped output David Bremner
  2 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2014-03-29  7:16 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, Mar 29 2014, David Bremner <david@tethera.net> wrote:

> ---
>
> This obviously needs at least one piece of tidying, and some tests.
>
> My motivation here is the ability to make gzipped backups from within
> notmuch, e.g. as part of an upgrade procedure.

+1

>
>
>  Makefile.local   |  2 +-
>  configure        | 19 ++++++++++++++++++-
>  notmuch-client.h |  4 +++-
>  notmuch-dump.c   | 41 +++++++++++++++++++++++++++--------------
>  4 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile.local b/Makefile.local
> index cb7b106..e5a20a7 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
>  # Smash together user's values with our extra values
>  FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CPPFLAGS) $(CFLAGS) $(WARN_CFLAGS) $(extra_cflags) $(CONFIGURE_CFLAGS)
>  FINAL_CXXFLAGS = $(CPPFLAGS) $(CXXFLAGS) $(WARN_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) $(CONFIGURE_CXXFLAGS)
> -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
> +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(ZLIB_LDFLAGS)
>  FINAL_NOTMUCH_LINKER = CC
>  ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
>  FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
> diff --git a/configure b/configure
> index 1d430b9..02ef785 100755
> --- a/configure
> +++ b/configure
> @@ -340,6 +340,18 @@ else
>      errors=$((errors + 1))
>  fi
>  
> +printf "Checking for zlib development files... "
> +have_zlib=0
> +if pkg-config --exists zlib; then
> +    printf "Yes.\n"
> +    have_zlib=1
> +    zlib_cflags=$(pkg-config --cflags zlib)
> +    zlib_ldflags=$(pkg-config --libs zlib)
> +else
> +    printf "No.\n"
> +    errors=$((errors + 1))
> +fi
> +
>  printf "Checking for talloc development files... "
>  if pkg-config --exists talloc; then
>      printf "Yes.\n"
> @@ -844,6 +856,10 @@ XAPIAN_LDFLAGS = ${xapian_ldflags}
>  GMIME_CFLAGS = ${gmime_cflags}
>  GMIME_LDFLAGS = ${gmime_ldflags}
>  
> +# Flags needed to compile and link against zlib
> +ZLIB_CFLAGS = ${zlib_cflags}
> +ZLIB_LDFLAGS = ${zlib_ldflags}
> +
>  # Flags needed to compile and link against talloc
>  TALLOC_CFLAGS = ${talloc_cflags}
>  TALLOC_LDFLAGS = ${talloc_ldflags}
> @@ -882,6 +898,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
>  		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
>  
>  CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
> +		     \$(ZLIB_CFLAGS)					 \\
>  		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
>  		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
>  		     -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)               \\
> @@ -892,5 +909,5 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
>  		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
>  
> -CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
> +CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
>  EOF
> diff --git a/notmuch-client.h b/notmuch-client.h
> index d110648..e1efbe0 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -450,7 +450,9 @@ typedef enum dump_formats {
>  int
>  notmuch_database_dump (notmuch_database_t *notmuch,
>  		       const char *output_file_name,
> -		       const char *query_str, dump_format_t output_format);
> +		       const char *query_str,
> +		       dump_format_t output_format,
> +		       notmuch_bool_t gzip_output);
>  
>  #include "command-line-arguments.h"
>  #endif
> diff --git a/notmuch-dump.c b/notmuch-dump.c
> index 21702d7..029f90a 100644
> --- a/notmuch-dump.c
> +++ b/notmuch-dump.c
> @@ -21,10 +21,12 @@
>  #include "notmuch-client.h"
>  #include "hex-escape.h"
>  #include "string-util.h"
> +#include <zlib.h>
> +
>  
>  static int
> -database_dump_file (notmuch_database_t *notmuch, FILE *output,
> -		    const char *query_str, int output_format)
> +database_dump_file (notmuch_database_t *notmuch, gzFile output,
> +			const char *query_str, int output_format)
>  {
>      notmuch_query_t *query;
>      notmuch_messages_t *messages;
> @@ -69,7 +71,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  	}
>  
>  	if (output_format == DUMP_FORMAT_SUP) {
> -	    fprintf (output, "%s (", message_id);
> +	    gzprintf (output, "%s (", message_id);
>  	}
>  
>  	for (tags = notmuch_message_get_tags (message);
> @@ -78,12 +80,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  	    const char *tag_str = notmuch_tags_get (tags);
>  
>  	    if (! first)
> -		fputs (" ", output);
> +		gzputs (output, " ");
>  
>  	    first = 0;
>  
>  	    if (output_format == DUMP_FORMAT_SUP) {
> -		fputs (tag_str, output);
> +		gzputs (output, tag_str);
>  	    } else {
>  		if (hex_encode (notmuch, tag_str,
>  				&buffer, &buffer_size) != HEX_SUCCESS) {
> @@ -91,12 +93,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  			     tag_str);
>  		    return EXIT_FAILURE;
>  		}
> -		fprintf (output, "+%s", buffer);
> +		gzprintf (output, "+%s", buffer);
>  	    }
>  	}
>  
>  	if (output_format == DUMP_FORMAT_SUP) {
> -	    fputs (")\n", output);
> +	    gzputs (output, ")\n");
>  	} else {
>  	    if (make_boolean_term (notmuch, "id", message_id,
>  				   &buffer, &buffer_size)) {
> @@ -104,7 +106,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  			     message_id, strerror (errno));
>  		    return EXIT_FAILURE;
>  	    }
> -	    fprintf (output, " -- %s\n", buffer);
> +	    gzprintf (output, " -- %s\n", buffer);
>  	}
>  
>  	notmuch_message_destroy (message);
> @@ -121,24 +123,33 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  int
>  notmuch_database_dump (notmuch_database_t *notmuch,
>  		       const char *output_file_name,
> -		       const char *query_str, dump_format_t output_format)
> +		       const char *query_str,
> +		       dump_format_t output_format,
> +		       notmuch_bool_t gzip_output)
>  {
> -    FILE *output = stdout;
> +    gzFile output;
> +    const char *mode = gzip_output ? "w9" : "wT";
> +
>      int ret;
>  
>      if (output_file_name) {
> -	output = fopen (output_file_name, "w");
> +	output = gzopen (output_file_name, mode);
>  	if (output == NULL) {
>  	    fprintf (stderr, "Error opening %s for writing: %s\n",
>  		     output_file_name, strerror (errno));
>  	    return EXIT_FAILURE;
>  	}
> +    } else {
> +	output = gzdopen (fileno (stdout), mode);
>      }
>  
>      ret = database_dump_file (notmuch, output, query_str, output_format);
>  
> -    if (output != stdout)
> -	fclose (output);
> +    /* XXX check error return */
> +    gzflush (output, Z_FINISH);
> +
> +    if (output_file_name)
> +	gzclose_w (output);
>  
>      return ret;
>  }
> @@ -158,6 +169,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>      int opt_index;
>  
>      int output_format = DUMP_FORMAT_BATCH_TAG;
> +    notmuch_bool_t gzip_output = 0;
>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
> @@ -165,6 +177,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
> +	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -181,7 +194,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      ret = notmuch_database_dump (notmuch, output_file_name, query_str,
> -				 output_format);
> +				 output_format, gzip_output);
>  
>      notmuch_database_destroy (notmuch);
>  
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-29  1:20   ` [PATCH] RFC: impliment gzipped output for notmuch dump David Bremner
  2014-03-29  7:16     ` Tomi Ollila
@ 2014-03-29  9:25     ` Jani Nikula
  2014-03-29 12:29       ` David Bremner
  2014-03-29 13:46     ` [PATCH] dump: support gzipped output David Bremner
  2 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2014-03-29  9:25 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, 29 Mar 2014, David Bremner <david@tethera.net> wrote:
> ---
>
> This obviously needs at least one piece of tidying, and some tests.
>
> My motivation here is the ability to make gzipped backups from within
> notmuch, e.g. as part of an upgrade procedure.

Is it reasonable to require zlib? It's possible to have conditional
build for this, but it will be laborous to do it cleanly. I could be
persuaded either way, perhaps more easily to just requiring it.

If you do require zlib, the missing dependencies error message from
configure should include a check for have_zlib and a note about it
similar to other required dependencies.

Also, it would seem natural to support gzipped input in restore as
well... do you think it would be silly to support gzipped output only
internally until we've added gzipped input too? This could be in a later
release in the future.


BR,
Jani.



>
>
>  Makefile.local   |  2 +-
>  configure        | 19 ++++++++++++++++++-
>  notmuch-client.h |  4 +++-
>  notmuch-dump.c   | 41 +++++++++++++++++++++++++++--------------
>  4 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile.local b/Makefile.local
> index cb7b106..e5a20a7 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
>  # Smash together user's values with our extra values
>  FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CPPFLAGS) $(CFLAGS) $(WARN_CFLAGS) $(extra_cflags) $(CONFIGURE_CFLAGS)
>  FINAL_CXXFLAGS = $(CPPFLAGS) $(CXXFLAGS) $(WARN_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) $(CONFIGURE_CXXFLAGS)
> -FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
> +FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(ZLIB_LDFLAGS)
>  FINAL_NOTMUCH_LINKER = CC
>  ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
>  FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
> diff --git a/configure b/configure
> index 1d430b9..02ef785 100755
> --- a/configure
> +++ b/configure
> @@ -340,6 +340,18 @@ else
>      errors=$((errors + 1))
>  fi
>  
> +printf "Checking for zlib development files... "
> +have_zlib=0
> +if pkg-config --exists zlib; then
> +    printf "Yes.\n"
> +    have_zlib=1
> +    zlib_cflags=$(pkg-config --cflags zlib)
> +    zlib_ldflags=$(pkg-config --libs zlib)
> +else
> +    printf "No.\n"
> +    errors=$((errors + 1))
> +fi
> +
>  printf "Checking for talloc development files... "
>  if pkg-config --exists talloc; then
>      printf "Yes.\n"
> @@ -844,6 +856,10 @@ XAPIAN_LDFLAGS = ${xapian_ldflags}
>  GMIME_CFLAGS = ${gmime_cflags}
>  GMIME_LDFLAGS = ${gmime_ldflags}
>  
> +# Flags needed to compile and link against zlib
> +ZLIB_CFLAGS = ${zlib_cflags}
> +ZLIB_LDFLAGS = ${zlib_ldflags}
> +
>  # Flags needed to compile and link against talloc
>  TALLOC_CFLAGS = ${talloc_cflags}
>  TALLOC_LDFLAGS = ${talloc_ldflags}
> @@ -882,6 +898,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
>  		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
>  
>  CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
> +		     \$(ZLIB_CFLAGS)					 \\
>  		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
>  		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
>  		     -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)               \\
> @@ -892,5 +909,5 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
>  		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
>  		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
>  
> -CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
> +CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
>  EOF
> diff --git a/notmuch-client.h b/notmuch-client.h
> index d110648..e1efbe0 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -450,7 +450,9 @@ typedef enum dump_formats {
>  int
>  notmuch_database_dump (notmuch_database_t *notmuch,
>  		       const char *output_file_name,
> -		       const char *query_str, dump_format_t output_format);
> +		       const char *query_str,
> +		       dump_format_t output_format,
> +		       notmuch_bool_t gzip_output);
>  
>  #include "command-line-arguments.h"
>  #endif
> diff --git a/notmuch-dump.c b/notmuch-dump.c
> index 21702d7..029f90a 100644
> --- a/notmuch-dump.c
> +++ b/notmuch-dump.c
> @@ -21,10 +21,12 @@
>  #include "notmuch-client.h"
>  #include "hex-escape.h"
>  #include "string-util.h"
> +#include <zlib.h>
> +
>  
>  static int
> -database_dump_file (notmuch_database_t *notmuch, FILE *output,
> -		    const char *query_str, int output_format)
> +database_dump_file (notmuch_database_t *notmuch, gzFile output,
> +			const char *query_str, int output_format)
>  {
>      notmuch_query_t *query;
>      notmuch_messages_t *messages;
> @@ -69,7 +71,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  	}
>  
>  	if (output_format == DUMP_FORMAT_SUP) {
> -	    fprintf (output, "%s (", message_id);
> +	    gzprintf (output, "%s (", message_id);
>  	}
>  
>  	for (tags = notmuch_message_get_tags (message);
> @@ -78,12 +80,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  	    const char *tag_str = notmuch_tags_get (tags);
>  
>  	    if (! first)
> -		fputs (" ", output);
> +		gzputs (output, " ");
>  
>  	    first = 0;
>  
>  	    if (output_format == DUMP_FORMAT_SUP) {
> -		fputs (tag_str, output);
> +		gzputs (output, tag_str);
>  	    } else {
>  		if (hex_encode (notmuch, tag_str,
>  				&buffer, &buffer_size) != HEX_SUCCESS) {
> @@ -91,12 +93,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  			     tag_str);
>  		    return EXIT_FAILURE;
>  		}
> -		fprintf (output, "+%s", buffer);
> +		gzprintf (output, "+%s", buffer);
>  	    }
>  	}
>  
>  	if (output_format == DUMP_FORMAT_SUP) {
> -	    fputs (")\n", output);
> +	    gzputs (output, ")\n");
>  	} else {
>  	    if (make_boolean_term (notmuch, "id", message_id,
>  				   &buffer, &buffer_size)) {
> @@ -104,7 +106,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  			     message_id, strerror (errno));
>  		    return EXIT_FAILURE;
>  	    }
> -	    fprintf (output, " -- %s\n", buffer);
> +	    gzprintf (output, " -- %s\n", buffer);
>  	}
>  
>  	notmuch_message_destroy (message);
> @@ -121,24 +123,33 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
>  int
>  notmuch_database_dump (notmuch_database_t *notmuch,
>  		       const char *output_file_name,
> -		       const char *query_str, dump_format_t output_format)
> +		       const char *query_str,
> +		       dump_format_t output_format,
> +		       notmuch_bool_t gzip_output)
>  {
> -    FILE *output = stdout;
> +    gzFile output;
> +    const char *mode = gzip_output ? "w9" : "wT";
> +
>      int ret;
>  
>      if (output_file_name) {
> -	output = fopen (output_file_name, "w");
> +	output = gzopen (output_file_name, mode);
>  	if (output == NULL) {
>  	    fprintf (stderr, "Error opening %s for writing: %s\n",
>  		     output_file_name, strerror (errno));
>  	    return EXIT_FAILURE;
>  	}
> +    } else {
> +	output = gzdopen (fileno (stdout), mode);
>      }
>  
>      ret = database_dump_file (notmuch, output, query_str, output_format);
>  
> -    if (output != stdout)
> -	fclose (output);
> +    /* XXX check error return */
> +    gzflush (output, Z_FINISH);
> +
> +    if (output_file_name)
> +	gzclose_w (output);
>  
>      return ret;
>  }
> @@ -158,6 +169,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>      int opt_index;
>  
>      int output_format = DUMP_FORMAT_BATCH_TAG;
> +    notmuch_bool_t gzip_output = 0;
>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
> @@ -165,6 +177,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
> +	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -181,7 +194,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      ret = notmuch_database_dump (notmuch, output_file_name, query_str,
> -				 output_format);
> +				 output_format, gzip_output);
>  
>      notmuch_database_destroy (notmuch);
>  
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Jani

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-29  9:25     ` Jani Nikula
@ 2014-03-29 12:29       ` David Bremner
  2014-03-29 13:02         ` Jani Nikula
  0 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2014-03-29 12:29 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Is it reasonable to require zlib? It's possible to have conditional
> build for this, but it will be laborous to do it cleanly. I could be
> persuaded either way, perhaps more easily to just requiring it.

If I recall correctly, xapian needs zlib.  At least on Debian this is a
hard dependency. What is needed additionally is the zlib headers.

> If you do require zlib, the missing dependencies error message from
> configure should include a check for have_zlib and a note about it
> similar to other required dependencies.

good point.

> Also, it would seem natural to support gzipped input in restore as
> well... do you think it would be silly to support gzipped output only
> internally until we've added gzipped input too? This could be in a later
> release in the future.

I couldn't decide which was sillier: to have have gzipped output but not
input, or to leave off the command line argument (it's something like 3
lines of diff). I lean to making it accessible from the CLI to better
support testing.

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-29 12:29       ` David Bremner
@ 2014-03-29 13:02         ` Jani Nikula
  2014-03-29 16:25           ` David Bremner
  0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2014-03-29 13:02 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, 29 Mar 2014, David Bremner <david@tethera.net> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>> Is it reasonable to require zlib? It's possible to have conditional
>> build for this, but it will be laborous to do it cleanly. I could be
>> persuaded either way, perhaps more easily to just requiring it.
>
> If I recall correctly, xapian needs zlib.  At least on Debian this is a
> hard dependency. What is needed additionally is the zlib headers.

Okay, no point in adding complexity in notmuch then.

> I couldn't decide which was sillier: to have have gzipped output but not
> input, or to leave off the command line argument (it's something like 3
> lines of diff). I lean to making it accessible from the CLI to better
> support testing.

Testability is a good point; otherwise we'd have to add an extra tool
for testing the interface, which is also silly. If we don't add restore
support now, I think we can consider this a documentation issue.

One more thing: do we need to ensure we don't emit gzipped stuff to the
terminal?

BR,
Jani.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] dump: support gzipped output
  2014-03-29  1:20   ` [PATCH] RFC: impliment gzipped output for notmuch dump David Bremner
  2014-03-29  7:16     ` Tomi Ollila
  2014-03-29  9:25     ` Jani Nikula
@ 2014-03-29 13:46     ` David Bremner
  2014-03-29 18:16       ` David Bremner
  2 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2014-03-29 13:46 UTC (permalink / raw)
  To: notmuch

The main goal is to support gzipped output for future internal
calls (e.g. from notmuch-new) to notmuch_database_dump.

The additional dependency is not very heavy since xapian already pulls
in zlib.
---

I had a quick look at supporting gzipped input for restore; I think it
just requires an implementation of getline that uses gzgetc or gzgets;
the decompression can be completely transparent to the user based on
magic number.

 INSTALL                   | 17 ++++++++++----
 Makefile.local            |  2 +-
 configure                 | 23 ++++++++++++++++---
 doc/man1/notmuch-dump.rst |  3 +++
 notmuch-client.h          |  4 +++-
 notmuch-dump.c            | 57 ++++++++++++++++++++++++++++++-----------------
 test/T240-dump-restore.sh | 12 ++++++++++
 7 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/INSTALL b/INSTALL
index 690b0ef..2754e52 100644
--- a/INSTALL
+++ b/INSTALL
@@ -20,8 +20,8 @@ configure stage.
 
 Dependencies
 ------------
-Notmuch depends on three libraries: Xapian, GMime 2.4 or 2.6, and
-Talloc which are each described below:
+Notmuch depends on four libraries: Xapian, GMime 2.4 or 2.6,
+Talloc, and zlib which are each described below:
 
 	Xapian
 	------
@@ -60,6 +60,15 @@ Talloc which are each described below:
 
 	Talloc is available from http://talloc.samba.org/
 
+	zlib
+	----
+
+	zlib is an extremely popular compression library. It is used
+	by Xapian, so if you installed that you will already have
+	zlib. You may need to install the zlib headers seperately.
+
+	zlib is available from http://zlib.net
+
 Building Documentation
 ----------------------
 
@@ -79,11 +88,11 @@ dependencies with a simple simple command line. For example:
 
   For Debian and similar:
 
-        sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx
+        sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev zlib1g-dev python-sphinx
 
   For Fedora and similar:
 
-	sudo yum install xapian-core-devel gmime-devel libtalloc-devel python-sphinx
+	sudo yum install xapian-core-devel gmime-devel libtalloc-devel zlib-devel python-sphinx
 
 On other systems, a similar command can be used, but the details of
 the package names may be different.
diff --git a/Makefile.local b/Makefile.local
index cb7b106..e5a20a7 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
 # Smash together user's values with our extra values
 FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CPPFLAGS) $(CFLAGS) $(WARN_CFLAGS) $(extra_cflags) $(CONFIGURE_CFLAGS)
 FINAL_CXXFLAGS = $(CPPFLAGS) $(CXXFLAGS) $(WARN_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) $(CONFIGURE_CXXFLAGS)
-FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
+FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(ZLIB_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
 FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
diff --git a/configure b/configure
index 1d430b9..89bb3f3 100755
--- a/configure
+++ b/configure
@@ -340,6 +340,18 @@ else
     errors=$((errors + 1))
 fi
 
+printf "Checking for zlib development files... "
+have_zlib=0
+if pkg-config --exists zlib; then
+    printf "Yes.\n"
+    have_zlib=1
+    zlib_cflags=$(pkg-config --cflags zlib)
+    zlib_ldflags=$(pkg-config --libs zlib)
+else
+    printf "No.\n"
+    errors=$((errors + 1))
+fi
+
 printf "Checking for talloc development files... "
 if pkg-config --exists talloc; then
     printf "Yes.\n"
@@ -519,11 +531,11 @@ case a simple command will install everything you need. For example:
 
 On Debian and similar systems:
 
-	sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev
+	sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev zlib1g-dev
 
 Or on Fedora and similar systems:
 
-	sudo yum install xapian-core-devel gmime-devel libtalloc-devel
+	sudo yum install xapian-core-devel gmime-devel libtalloc-devel zlib-devel
 
 On other systems, similar commands can be used, but the details of the
 package names may be different.
@@ -844,6 +856,10 @@ XAPIAN_LDFLAGS = ${xapian_ldflags}
 GMIME_CFLAGS = ${gmime_cflags}
 GMIME_LDFLAGS = ${gmime_ldflags}
 
+# Flags needed to compile and link against zlib
+ZLIB_CFLAGS = ${zlib_cflags}
+ZLIB_LDFLAGS = ${zlib_ldflags}
+
 # Flags needed to compile and link against talloc
 TALLOC_CFLAGS = ${talloc_cflags}
 TALLOC_LDFLAGS = ${talloc_ldflags}
@@ -882,6 +898,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
+		     \$(ZLIB_CFLAGS)					 \\
 		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
 		     -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)               \\
@@ -892,5 +909,5 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
 		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
-CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
+CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
 EOF
diff --git a/doc/man1/notmuch-dump.rst b/doc/man1/notmuch-dump.rst
index 17d1da5..d94cb4f 100644
--- a/doc/man1/notmuch-dump.rst
+++ b/doc/man1/notmuch-dump.rst
@@ -19,6 +19,9 @@ recreated from the messages themselves. The output of notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)
 
+``--gzip``
+    Compress the output in a format compatible with **gzip(1)**.
+
 ``--format=(sup|batch-tag)``
     Notmuch restore supports two plain text dump formats, both with one
     message-id per line, followed by a list of tags.
diff --git a/notmuch-client.h b/notmuch-client.h
index d110648..e1efbe0 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -450,7 +450,9 @@ typedef enum dump_formats {
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format);
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output);
 
 #include "command-line-arguments.h"
 #endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 21702d7..128a37d 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -21,10 +21,12 @@
 #include "notmuch-client.h"
 #include "hex-escape.h"
 #include "string-util.h"
+#include <zlib.h>
+
 
 static int
-database_dump_file (notmuch_database_t *notmuch, FILE *output,
-		    const char *query_str, int output_format)
+database_dump_file (notmuch_database_t *notmuch, gzFile output,
+			const char *query_str, int output_format)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
@@ -69,7 +71,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fprintf (output, "%s (", message_id);
+	    gzprintf (output, "%s (", message_id);
 	}
 
 	for (tags = notmuch_message_get_tags (message);
@@ -78,12 +80,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	    const char *tag_str = notmuch_tags_get (tags);
 
 	    if (! first)
-		fputs (" ", output);
+		gzputs (output, " ");
 
 	    first = 0;
 
 	    if (output_format == DUMP_FORMAT_SUP) {
-		fputs (tag_str, output);
+		gzputs (output, tag_str);
 	    } else {
 		if (hex_encode (notmuch, tag_str,
 				&buffer, &buffer_size) != HEX_SUCCESS) {
@@ -91,12 +93,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     tag_str);
 		    return EXIT_FAILURE;
 		}
-		fprintf (output, "+%s", buffer);
+		gzprintf (output, "+%s", buffer);
 	    }
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fputs (")\n", output);
+	    gzputs (output, ")\n");
 	} else {
 	    if (make_boolean_term (notmuch, "id", message_id,
 				   &buffer, &buffer_size)) {
@@ -104,7 +106,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     message_id, strerror (errno));
 		    return EXIT_FAILURE;
 	    }
-	    fprintf (output, " -- %s\n", buffer);
+	    gzprintf (output, " -- %s\n", buffer);
 	}
 
 	notmuch_message_destroy (message);
@@ -121,24 +123,37 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format)
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output)
 {
-    FILE *output = stdout;
+    gzFile output;
+    const char *mode = gzip_output ? "w9" : "wT";
+
     int ret;
 
-    if (output_file_name) {
-	output = fopen (output_file_name, "w");
-	if (output == NULL) {
-	    fprintf (stderr, "Error opening %s for writing: %s\n",
-		     output_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
+    if (output_file_name)
+	output = gzopen (output_file_name, mode);
+    else
+	output = gzdopen (fileno (stdout), mode);
+
+    if (output == NULL) {
+	fprintf (stderr, "Error opening %s for (gzip) writing: %s\n",
+		 output_file_name || "stdout", strerror (errno));
+	return EXIT_FAILURE;
     }
 
     ret = database_dump_file (notmuch, output, query_str, output_format);
 
-    if (output != stdout)
-	fclose (output);
+    /* unlike stdio, zlib needs explicit flushing */
+    if (gzflush (output, Z_FINISH)) {
+	fprintf (stderr, "Error flushing output: %s\n",
+		 gzerror (output, NULL));
+	return EXIT_FAILURE;
+    }
+
+    if (output_file_name)
+	gzclose_w (output);
 
     return ret;
 }
@@ -158,6 +173,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
 
     int output_format = DUMP_FORMAT_BATCH_TAG;
+    notmuch_bool_t gzip_output = 0;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
@@ -165,6 +181,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
+	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -181,7 +198,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     ret = notmuch_database_dump (notmuch, output_file_name, query_str,
-				 output_format);
+				 output_format, gzip_output);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh
index 0004438..d79aca8 100755
--- a/test/T240-dump-restore.sh
+++ b/test/T240-dump-restore.sh
@@ -68,6 +68,18 @@ test_begin_subtest "dump --output=outfile --"
 notmuch dump --output=dump-1-arg-dash.actual --
 test_expect_equal_file dump.expected dump-1-arg-dash.actual
 
+# gzipped output
+
+test_begin_subtest "dump --gzip"
+notmuch dump --gzip > dump-gzip.gz
+gunzip dump-gzip.gz
+test_expect_equal_file dump.expected dump-gzip
+
+test_begin_subtest "dump --gzip --output=outfile"
+notmuch dump --gzip --output=dump-gzip-outfile.gz
+gunzip dump-gzip-outfile.gz
+test_expect_equal_file dump.expected dump-gzip-outfile
+
 # Note, we assume all messages from cworth have a message-id
 # containing cworth.org
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] RFC: impliment gzipped output for notmuch dump
  2014-03-29 13:02         ` Jani Nikula
@ 2014-03-29 16:25           ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29 16:25 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

>
> One more thing: do we need to ensure we don't emit gzipped stuff to the
> terminal?
>

Well, there's a relatively small risk; the user has to explicitly give
the --gzip option.  It might mess up the terminal; in my experiments
there were no noticible ill effects from interrupting "notmuch dump
--gzip" spewing to the terminal (zsh in urxvt; ymmv).

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* (no subject)
  2014-03-29 13:46     ` [PATCH] dump: support gzipped output David Bremner
@ 2014-03-29 18:16       ` David Bremner
  2014-03-29 18:16         ` [Patch v2 1/3] dump: support gzipped output David Bremner
                           ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29 18:16 UTC (permalink / raw)
  To: notmuch


The first patch has been revised according to comments from Jani and
Tomi on IRC.  The next two are completely new. They pass the existing
tests, but they should probably get a skeptical eye, particularly the
implementation of gzreadline, since it's hard to know how well
existing tests exercise that code.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Patch v2 1/3] dump: support gzipped output
  2014-03-29 18:16       ` David Bremner
@ 2014-03-29 18:16         ` David Bremner
  2014-03-29 18:16         ` [Patch v2 2/3] util: add gzreadline David Bremner
  2014-03-29 18:16         ` [Patch v2 3/3] restore: transparently support gzipped input David Bremner
  2 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29 18:16 UTC (permalink / raw)
  To: notmuch

The main goal is to support gzipped output for future internal
calls (e.g. from notmuch-new) to notmuch_database_dump.

The additional dependency is not very heavy since xapian already pulls
in zlib.
---
 INSTALL                   | 17 ++++++++++----
 Makefile.local            |  2 +-
 configure                 | 28 ++++++++++++++++++++---
 doc/man1/notmuch-dump.rst |  3 +++
 notmuch-client.h          |  4 +++-
 notmuch-dump.c            | 57 ++++++++++++++++++++++++++++++-----------------
 test/T240-dump-restore.sh | 12 ++++++++++
 7 files changed, 94 insertions(+), 29 deletions(-)

diff --git a/INSTALL b/INSTALL
index 690b0ef..df318fa 100644
--- a/INSTALL
+++ b/INSTALL
@@ -20,8 +20,8 @@ configure stage.
 
 Dependencies
 ------------
-Notmuch depends on three libraries: Xapian, GMime 2.4 or 2.6, and
-Talloc which are each described below:
+Notmuch depends on four libraries: Xapian, GMime 2.4 or 2.6,
+Talloc, and zlib which are each described below:
 
 	Xapian
 	------
@@ -60,6 +60,15 @@ Talloc which are each described below:
 
 	Talloc is available from http://talloc.samba.org/
 
+	zlib
+	----
+
+	zlib is an extremely popular compression library. It is used
+	by Xapian, so if you installed that you will already have
+	zlib. You may need to install the zlib headers separately.
+
+	zlib is available from http://zlib.net
+
 Building Documentation
 ----------------------
 
@@ -79,11 +88,11 @@ dependencies with a simple simple command line. For example:
 
   For Debian and similar:
 
-        sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx
+        sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev zlib1g-dev python-sphinx
 
   For Fedora and similar:
 
-	sudo yum install xapian-core-devel gmime-devel libtalloc-devel python-sphinx
+	sudo yum install xapian-core-devel gmime-devel libtalloc-devel zlib-devel python-sphinx
 
 On other systems, a similar command can be used, but the details of
 the package names may be different.
diff --git a/Makefile.local b/Makefile.local
index cb7b106..e5a20a7 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -41,7 +41,7 @@ PV_FILE=bindings/python/notmuch/version.py
 # Smash together user's values with our extra values
 FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CPPFLAGS) $(CFLAGS) $(WARN_CFLAGS) $(extra_cflags) $(CONFIGURE_CFLAGS)
 FINAL_CXXFLAGS = $(CPPFLAGS) $(CXXFLAGS) $(WARN_CXXFLAGS) $(extra_cflags) $(extra_cxxflags) $(CONFIGURE_CXXFLAGS)
-FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
+FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) $(ZLIB_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
 FINAL_NOTMUCH_LDFLAGS += $(CONFIGURE_LDFLAGS)
diff --git a/configure b/configure
index 1d430b9..d685ab3 100755
--- a/configure
+++ b/configure
@@ -340,6 +340,18 @@ else
     errors=$((errors + 1))
 fi
 
+printf "Checking for zlib development files... "
+have_zlib=0
+if pkg-config --exists zlib; then
+    printf "Yes.\n"
+    have_zlib=1
+    zlib_cflags=$(pkg-config --cflags zlib)
+    zlib_ldflags=$(pkg-config --libs zlib)
+else
+    printf "No.\n"
+    errors=$((errors + 1))
+fi
+
 printf "Checking for talloc development files... "
 if pkg-config --exists talloc; then
     printf "Yes.\n"
@@ -496,6 +508,11 @@ EOF
 	echo "	Xapian library (including development files such as headers)"
 	echo "	http://xapian.org/"
     fi
+    if [ $have_zlib -eq 0 ]; then
+	echo "	zlib library (including development files such as headers)"
+	echo "	http://zlib.net/"
+	echo
+    fi
     if [ $have_gmime -eq 0 ]; then
 	echo "	Either GMime 2.4 library" $GMIME_24_VERSION_CTR "or GMime 2.6 library" $GMIME_26_VERSION_CTR
 	echo "	(including development files such as headers)"
@@ -519,11 +536,11 @@ case a simple command will install everything you need. For example:
 
 On Debian and similar systems:
 
-	sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev
+	sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev zlib1g-dev
 
 Or on Fedora and similar systems:
 
-	sudo yum install xapian-core-devel gmime-devel libtalloc-devel
+	sudo yum install xapian-core-devel gmime-devel libtalloc-devel zlib-devel
 
 On other systems, similar commands can be used, but the details of the
 package names may be different.
@@ -844,6 +861,10 @@ XAPIAN_LDFLAGS = ${xapian_ldflags}
 GMIME_CFLAGS = ${gmime_cflags}
 GMIME_LDFLAGS = ${gmime_ldflags}
 
+# Flags needed to compile and link against zlib
+ZLIB_CFLAGS = ${zlib_cflags}
+ZLIB_LDFLAGS = ${zlib_ldflags}
+
 # Flags needed to compile and link against talloc
 TALLOC_CFLAGS = ${talloc_cflags}
 TALLOC_LDFLAGS = ${talloc_ldflags}
@@ -882,6 +903,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
+		     \$(ZLIB_CFLAGS)					 \\
 		     \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\
 		     \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS)             \\
 		     -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR)               \\
@@ -892,5 +914,5 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
 		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
-CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)
+CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
 EOF
diff --git a/doc/man1/notmuch-dump.rst b/doc/man1/notmuch-dump.rst
index 17d1da5..d94cb4f 100644
--- a/doc/man1/notmuch-dump.rst
+++ b/doc/man1/notmuch-dump.rst
@@ -19,6 +19,9 @@ recreated from the messages themselves. The output of notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)
 
+``--gzip``
+    Compress the output in a format compatible with **gzip(1)**.
+
 ``--format=(sup|batch-tag)``
     Notmuch restore supports two plain text dump formats, both with one
     message-id per line, followed by a list of tags.
diff --git a/notmuch-client.h b/notmuch-client.h
index d110648..e1efbe0 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -450,7 +450,9 @@ typedef enum dump_formats {
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format);
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output);
 
 #include "command-line-arguments.h"
 #endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 21702d7..4875aba 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -21,10 +21,12 @@
 #include "notmuch-client.h"
 #include "hex-escape.h"
 #include "string-util.h"
+#include <zlib.h>
+
 
 static int
-database_dump_file (notmuch_database_t *notmuch, FILE *output,
-		    const char *query_str, int output_format)
+database_dump_file (notmuch_database_t *notmuch, gzFile output,
+			const char *query_str, int output_format)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
@@ -69,7 +71,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fprintf (output, "%s (", message_id);
+	    gzprintf (output, "%s (", message_id);
 	}
 
 	for (tags = notmuch_message_get_tags (message);
@@ -78,12 +80,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 	    const char *tag_str = notmuch_tags_get (tags);
 
 	    if (! first)
-		fputs (" ", output);
+		gzputs (output, " ");
 
 	    first = 0;
 
 	    if (output_format == DUMP_FORMAT_SUP) {
-		fputs (tag_str, output);
+		gzputs (output, tag_str);
 	    } else {
 		if (hex_encode (notmuch, tag_str,
 				&buffer, &buffer_size) != HEX_SUCCESS) {
@@ -91,12 +93,12 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     tag_str);
 		    return EXIT_FAILURE;
 		}
-		fprintf (output, "+%s", buffer);
+		gzprintf (output, "+%s", buffer);
 	    }
 	}
 
 	if (output_format == DUMP_FORMAT_SUP) {
-	    fputs (")\n", output);
+	    gzputs (output, ")\n");
 	} else {
 	    if (make_boolean_term (notmuch, "id", message_id,
 				   &buffer, &buffer_size)) {
@@ -104,7 +106,7 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 			     message_id, strerror (errno));
 		    return EXIT_FAILURE;
 	    }
-	    fprintf (output, " -- %s\n", buffer);
+	    gzprintf (output, " -- %s\n", buffer);
 	}
 
 	notmuch_message_destroy (message);
@@ -121,24 +123,37 @@ database_dump_file (notmuch_database_t *notmuch, FILE *output,
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
-		       const char *query_str, dump_format_t output_format)
+		       const char *query_str,
+		       dump_format_t output_format,
+		       notmuch_bool_t gzip_output)
 {
-    FILE *output = stdout;
+    gzFile output;
+    const char *mode = gzip_output ? "w9" : "wT";
+
     int ret;
 
-    if (output_file_name) {
-	output = fopen (output_file_name, "w");
-	if (output == NULL) {
-	    fprintf (stderr, "Error opening %s for writing: %s\n",
-		     output_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
+    if (output_file_name)
+	output = gzopen (output_file_name, mode);
+    else
+	output = gzdopen (fileno (stdout), mode);
+
+    if (output == NULL) {
+	fprintf (stderr, "Error opening %s for (gzip) writing: %s\n",
+		 output_file_name ? output_file_name : "stdout", strerror (errno));
+	return EXIT_FAILURE;
     }
 
     ret = database_dump_file (notmuch, output, query_str, output_format);
 
-    if (output != stdout)
-	fclose (output);
+    /* unlike stdio, zlib needs explicit flushing */
+    if (gzflush (output, Z_FINISH)) {
+	fprintf (stderr, "Error flushing output: %s\n",
+		 gzerror (output, NULL));
+	return EXIT_FAILURE;
+    }
+
+    if (output_file_name)
+	gzclose_w (output);
 
     return ret;
 }
@@ -158,6 +173,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
 
     int output_format = DUMP_FORMAT_BATCH_TAG;
+    notmuch_bool_t gzip_output = 0;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &output_format, "format", 'f',
@@ -165,6 +181,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
+	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -181,7 +198,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     ret = notmuch_database_dump (notmuch, output_file_name, query_str,
-				 output_format);
+				 output_format, gzip_output);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh
index 0004438..d79aca8 100755
--- a/test/T240-dump-restore.sh
+++ b/test/T240-dump-restore.sh
@@ -68,6 +68,18 @@ test_begin_subtest "dump --output=outfile --"
 notmuch dump --output=dump-1-arg-dash.actual --
 test_expect_equal_file dump.expected dump-1-arg-dash.actual
 
+# gzipped output
+
+test_begin_subtest "dump --gzip"
+notmuch dump --gzip > dump-gzip.gz
+gunzip dump-gzip.gz
+test_expect_equal_file dump.expected dump-gzip
+
+test_begin_subtest "dump --gzip --output=outfile"
+notmuch dump --gzip --output=dump-gzip-outfile.gz
+gunzip dump-gzip-outfile.gz
+test_expect_equal_file dump.expected dump-gzip-outfile
+
 # Note, we assume all messages from cworth have a message-id
 # containing cworth.org
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Patch v2 2/3] util: add gzreadline
  2014-03-29 18:16       ` David Bremner
  2014-03-29 18:16         ` [Patch v2 1/3] dump: support gzipped output David Bremner
@ 2014-03-29 18:16         ` David Bremner
  2014-03-30  8:30           ` Tomi Ollila
  2014-03-30 11:03           ` [Patch v2 2/3] " David Bremner
  2014-03-29 18:16         ` [Patch v2 3/3] restore: transparently support gzipped input David Bremner
  2 siblings, 2 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29 18:16 UTC (permalink / raw)
  To: notmuch

The idea is to provide a more or less drop in replacement for readline
to read from zlib/gzip streams.  Take the opportunity to replace
malloc with talloc.
---
 util/Makefile.local |  2 +-
 util/zlib-extra.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 util/zlib-extra.h   | 10 +++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 util/zlib-extra.c
 create mode 100644 util/zlib-extra.h

diff --git a/util/Makefile.local b/util/Makefile.local
index 29c0ce6..e2a5b65 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
-		  $(dir)/string-util.c $(dir)/talloc-extra.c
+		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/zlib-extra.c b/util/zlib-extra.c
new file mode 100644
index 0000000..cda369f
--- /dev/null
+++ b/util/zlib-extra.c
@@ -0,0 +1,59 @@
+/* zlib-extra.c -  Extra or enhanced routines for compressed I/O.
+ *
+ * Copyright (c) 2014 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner <david@tethera.net>
+ */
+
+#include "zlib-extra.h"
+#include <talloc.h>
+#include <stdio.h>
+#include <string.h>
+
+/* mimic POSIX/glibc getline, but on a zlib gzFile stream, and using talloc */
+ssize_t
+gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream) {
+
+    size_t len=*n;
+    char *buf = *lineptr;
+    size_t offset = 0;
+
+    if (len == 0 || buf == NULL) {
+	/* XXX arbitrary choice of initial size */
+	len = BUFSIZ;
+	buf = talloc_size (ctx, len);
+    }
+
+    while (1) {
+
+	if (!gzgets (stream, buf + offset, len))
+	    return -1;
+
+	/* XXX wasted effort re-counting length of whole line */
+	offset = strlen (buf);
+
+	if ( buf[offset-1] == '\n' )
+	    break;
+
+	len *= 2;
+	buf = talloc_realloc (ctx, buf, char, len);
+
+    }
+
+    *lineptr = buf;
+    *n = len;
+    return offset;
+}
diff --git a/util/zlib-extra.h b/util/zlib-extra.h
new file mode 100644
index 0000000..c18480f
--- /dev/null
+++ b/util/zlib-extra.h
@@ -0,0 +1,10 @@
+#ifndef _ZLIB_EXTRA_H
+#define _ZLIB_EXTRA_H
+
+#include <zlib.h>
+
+/* Like getline, but read from a gzFile. Allocation is with talloc */
+ssize_t
+gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream);
+
+#endif
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Patch v2 3/3] restore: transparently support gzipped input
  2014-03-29 18:16       ` David Bremner
  2014-03-29 18:16         ` [Patch v2 1/3] dump: support gzipped output David Bremner
  2014-03-29 18:16         ` [Patch v2 2/3] util: add gzreadline David Bremner
@ 2014-03-29 18:16         ` David Bremner
  2 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2014-03-29 18:16 UTC (permalink / raw)
  To: notmuch

We rely completely on zlib to do the right thing in detecting gzipped
input. Since our dump format is chosen to be 7 bit ascii, this should
be fine.
---
 notmuch-restore.c         | 30 +++++++++++++++---------------
 test/T240-dump-restore.sh | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index c54d513..50da480 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -22,6 +22,7 @@
 #include "hex-escape.h"
 #include "tag-util.h"
 #include "string-util.h"
+#include "zlib-extra.h"
 
 static regex_t regex;
 
@@ -128,7 +129,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     tag_op_list_t *tag_ops;
 
     char *input_file_name = NULL;
-    FILE *input = stdin;
+    gzFile input;
     char *line = NULL;
     void *line_ctx = NULL;
     size_t line_size;
@@ -163,13 +164,15 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     if (! accumulate)
 	flags |= TAG_FLAG_REMOVE_ALL;
 
-    if (input_file_name) {
-	input = fopen (input_file_name, "r");
-	if (input == NULL) {
-	    fprintf (stderr, "Error opening %s for reading: %s\n",
-		     input_file_name, strerror (errno));
-	    return EXIT_FAILURE;
-	}
+    if (input_file_name)
+	input = gzopen (input_file_name, "r");
+    else
+	input = gzdopen (fileno (stdin), "r");
+
+    if (input == NULL) {
+	fprintf (stderr, "Error opening %s for (gzip) reading: %s\n",
+		 input_file_name ? input_file_name : "stdin", strerror (errno));
+	return EXIT_FAILURE;
     }
 
     if (opt_index < argc) {
@@ -184,7 +187,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     do {
-	line_len = getline (&line, &line_size, input);
+	line_len = gzgetline (line_ctx, &line, &line_size, input);
 
 	/* empty input file not considered an error */
 	if (line_len < 0)
@@ -254,7 +257,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 	if (ret)
 	    break;
 
-    }  while ((line_len = getline (&line, &line_size, input)) != -1);
+    }  while ((line_len = gzgetline (line_ctx, &line, &line_size, input)) != -1);
 
     if (line_ctx != NULL)
 	talloc_free (line_ctx);
@@ -262,13 +265,10 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     if (input_format == DUMP_FORMAT_SUP)
 	regfree (&regex);
 
-    if (line)
-	free (line);
-
     notmuch_database_destroy (notmuch);
 
-    if (input != stdin)
-	fclose (input);
+    if (input_file_name != NULL)
+	gzclose_r (input);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh
index d79aca8..50d4d48 100755
--- a/test/T240-dump-restore.sh
+++ b/test/T240-dump-restore.sh
@@ -80,6 +80,20 @@ notmuch dump --gzip --output=dump-gzip-outfile.gz
 gunzip dump-gzip-outfile.gz
 test_expect_equal_file dump.expected dump-gzip-outfile
 
+test_begin_subtest "restoring gzipped stdin"
+notmuch dump --gzip --output=backup.gz
+notmuch tag +new_tag '*'
+notmuch restore < backup.gz
+notmuch dump --output=dump.actual
+test_expect_equal_file dump.expected dump.actual
+
+test_begin_subtest "restoring gzipped file"
+notmuch dump --gzip --output=backup.gz
+notmuch tag +new_tag '*'
+notmuch restore --input=backup.gz
+notmuch dump --output=dump.actual
+test_expect_equal_file dump.expected dump.actual
+
 # Note, we assume all messages from cworth have a message-id
 # containing cworth.org
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Patch v2 2/3] util: add gzreadline
  2014-03-29 18:16         ` [Patch v2 2/3] util: add gzreadline David Bremner
@ 2014-03-30  8:30           ` Tomi Ollila
  2014-03-30 11:23             ` [Patch v3] " David Bremner
  2014-03-30 11:03           ` [Patch v2 2/3] " David Bremner
  1 sibling, 1 reply; 23+ messages in thread
From: Tomi Ollila @ 2014-03-30  8:30 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, Mar 29 2014, David Bremner <david@tethera.net> wrote:

> The idea is to provide a more or less drop in replacement for readline
> to read from zlib/gzip streams.  Take the opportunity to replace
> malloc with talloc.
> ---

This series looks pretty good to me. I thought I found a place for real
improvement but I was wrong. Therefore I look in this for more detail ;D

>  util/Makefile.local |  2 +-
>  util/zlib-extra.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/zlib-extra.h   | 10 +++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 util/zlib-extra.c
>  create mode 100644 util/zlib-extra.h
>
> diff --git a/util/Makefile.local b/util/Makefile.local
> index 29c0ce6..e2a5b65 100644
> --- a/util/Makefile.local
> +++ b/util/Makefile.local
> @@ -4,7 +4,7 @@ dir := util
>  extra_cflags += -I$(srcdir)/$(dir)
>  
>  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
> -		  $(dir)/string-util.c $(dir)/talloc-extra.c
> +		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c
>  
>  libutil_modules := $(libutil_c_srcs:.c=.o)
>  
> diff --git a/util/zlib-extra.c b/util/zlib-extra.c
> new file mode 100644
> index 0000000..cda369f
> --- /dev/null
> +++ b/util/zlib-extra.c
> @@ -0,0 +1,59 @@
> +/* zlib-extra.c -  Extra or enhanced routines for compressed I/O.
> + *
> + * Copyright (c) 2014 David Bremner
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: David Bremner <david@tethera.net>
> + */
> +
> +#include "zlib-extra.h"
> +#include <talloc.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +/* mimic POSIX/glibc getline, but on a zlib gzFile stream, and using talloc */
> +ssize_t
> +gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream) {

The naming space of this function looks like the gz*functions in general;
But this adds a special talloc usage. getline() > gzgetline() conversion
should be consistent w/ other gz*() functions. I'd suggest to make slight
change to the function name to sanction the talloc usage. Ah, the fine
art & pain in naming considerations...

> +
> +    size_t len=*n;

spaces around '='

> +    char *buf = *lineptr;
> +    size_t offset = 0;
> +
> +    if (len == 0 || buf == NULL) {
> +	/* XXX arbitrary choice of initial size */

what does getline() use there.. could the 'XXX' just be dropped.

> +	len = BUFSIZ;
> +	buf = talloc_size (ctx, len);
> +    }
> +
> +    while (1) {
> +
> +	if (!gzgets (stream, buf + offset, len))
> +	    return -1;
> +
> +	/* XXX wasted effort re-counting length of whole line */
> +	offset = strlen (buf);

offset += strlen (buf + offset); would drop the whole line recounting...
just would that ever happen (gzgets() just usually returns the whole line
with '\n').. so many alternatives there...

...maybe just comment the usual case that full lines are returned.


Tomi


> +
> +	if ( buf[offset-1] == '\n' )
> +	    break;
> +
> +	len *= 2;
> +	buf = talloc_realloc (ctx, buf, char, len);
> +
> +    }
> +
> +    *lineptr = buf;
> +    *n = len;
> +    return offset;
> +}
> diff --git a/util/zlib-extra.h b/util/zlib-extra.h
> new file mode 100644
> index 0000000..c18480f
> --- /dev/null
> +++ b/util/zlib-extra.h
> @@ -0,0 +1,10 @@
> +#ifndef _ZLIB_EXTRA_H
> +#define _ZLIB_EXTRA_H
> +
> +#include <zlib.h>
> +
> +/* Like getline, but read from a gzFile. Allocation is with talloc */
> +ssize_t
> +gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream);
> +
> +#endif
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2 2/3] util: add gzreadline
  2014-03-29 18:16         ` [Patch v2 2/3] util: add gzreadline David Bremner
  2014-03-30  8:30           ` Tomi Ollila
@ 2014-03-30 11:03           ` David Bremner
  1 sibling, 0 replies; 23+ messages in thread
From: David Bremner @ 2014-03-30 11:03 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:


> +	if (!gzgets (stream, buf + offset, len))
> +	    return -1;

I did find one bug by setting the initial line size to 2. This should
read "len - offset".

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Patch v3] util: add gzreadline
  2014-03-30  8:30           ` Tomi Ollila
@ 2014-03-30 11:23             ` David Bremner
  2014-03-30 12:45               ` Tomi Ollila
  0 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2014-03-30 11:23 UTC (permalink / raw)
  To: notmuch

The idea is to provide a more or less drop in replacement for readline
to read from zlib/gzip streams.  Take the opportunity to replace
malloc with talloc.
---

This corrects one nasty bug, and a few style/optimization issues brought up by Tomi.

I'm not sure about the name. I agree in principle it would be good to
signal "talloc". I'm just not sure the best way to do that.

I'm also not sure about the error handling. the "real" getline sets
errno. Should we?

 util/Makefile.local |  2 +-
 util/zlib-extra.c   | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 util/zlib-extra.h   | 10 +++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 util/zlib-extra.c
 create mode 100644 util/zlib-extra.h

diff --git a/util/Makefile.local b/util/Makefile.local
index 29c0ce6..e2a5b65 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
-		  $(dir)/string-util.c $(dir)/talloc-extra.c
+		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/zlib-extra.c b/util/zlib-extra.c
new file mode 100644
index 0000000..7afe175
--- /dev/null
+++ b/util/zlib-extra.c
@@ -0,0 +1,62 @@
+/* zlib-extra.c -  Extra or enhanced routines for compressed I/O.
+ *
+ * Copyright (c) 2014 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner <david@tethera.net>
+ */
+
+#include "zlib-extra.h"
+#include <talloc.h>
+#include <stdio.h>
+#include <string.h>
+
+/* mimic POSIX/glibc getline, but on a zlib gzFile stream, and using talloc */
+ssize_t
+gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream) {
+
+    size_t len = *n;
+    char *buf = *lineptr;
+    size_t offset = 0;
+
+    if (len == 0 || buf == NULL) {
+	/* same as getdelim from gnulib */
+	len = 120;
+	buf = talloc_size (ctx, len);
+	if (buf == NULL)
+	    return -1;
+    }
+
+    while (1) {
+
+	if (!gzgets (stream, buf + offset, len - offset))
+	    return -1;
+
+	offset += strlen (buf+offset);
+
+	if ( buf[offset-1] == '\n' )
+	    break;
+
+	len *= 2;
+	buf = talloc_realloc (ctx, buf, char, len);
+	if (buf == NULL)
+	    return -1;
+
+    }
+
+    *lineptr = buf;
+    *n = len;
+    return offset;
+}
diff --git a/util/zlib-extra.h b/util/zlib-extra.h
new file mode 100644
index 0000000..c18480f
--- /dev/null
+++ b/util/zlib-extra.h
@@ -0,0 +1,10 @@
+#ifndef _ZLIB_EXTRA_H
+#define _ZLIB_EXTRA_H
+
+#include <zlib.h>
+
+/* Like getline, but read from a gzFile. Allocation is with talloc */
+ssize_t
+gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream);
+
+#endif
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Patch v3] util: add gzreadline
  2014-03-30 11:23             ` [Patch v3] " David Bremner
@ 2014-03-30 12:45               ` Tomi Ollila
  2014-03-30 14:37                 ` David Bremner
  0 siblings, 1 reply; 23+ messages in thread
From: Tomi Ollila @ 2014-03-30 12:45 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sun, Mar 30 2014, David Bremner <david@tethera.net> wrote:

> The idea is to provide a more or less drop in replacement for readline
> to read from zlib/gzip streams.  Take the opportunity to replace
> malloc with talloc.
> ---
>
> This corrects one nasty bug, and a few style/optimization issues brought up by Tomi.
>
> I'm not sure about the name. I agree in principle it would be good to
> signal "talloc". I'm just not sure the best way to do that.

how about just gz_getline(). If there were gzgetline() one would expect it's
function "signature" be gzgetline(char **lineptr, size_t *m gzFile *stream);
(consistent with other '' -> gz'' conversions). for gz_getline() there
would be no expectations.

>
> I'm also not sure about the error handling. the "real" getline sets
> errno. Should we?

In comparison with gzgets() return value

"On success, gzgets() shall return a pointer to buf. Otherwise, gzgets()
"shall return Z_NULL. Applications may examine the cause using gzerror().
"
"Errors
"
"On error, gzgets() shall return Z_NULL.

In light of this I'd say no. can we set out-of-memory using some public
api so it is available using gzerror()


Rest of the style issues inline ;p

Tomi

>
>  util/Makefile.local |  2 +-
>  util/zlib-extra.c   | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/zlib-extra.h   | 10 +++++++++
>  3 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100644 util/zlib-extra.c
>  create mode 100644 util/zlib-extra.h
>
> diff --git a/util/Makefile.local b/util/Makefile.local
> index 29c0ce6..e2a5b65 100644
> --- a/util/Makefile.local
> +++ b/util/Makefile.local
> @@ -4,7 +4,7 @@ dir := util
>  extra_cflags += -I$(srcdir)/$(dir)
>  
>  libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
> -		  $(dir)/string-util.c $(dir)/talloc-extra.c
> +		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c
>  
>  libutil_modules := $(libutil_c_srcs:.c=.o)
>  
> diff --git a/util/zlib-extra.c b/util/zlib-extra.c
> new file mode 100644
> index 0000000..7afe175
> --- /dev/null
> +++ b/util/zlib-extra.c
> @@ -0,0 +1,62 @@
> +/* zlib-extra.c -  Extra or enhanced routines for compressed I/O.
> + *
> + * Copyright (c) 2014 David Bremner
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: David Bremner <david@tethera.net>
> + */
> +
> +#include "zlib-extra.h"
> +#include <talloc.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +/* mimic POSIX/glibc getline, but on a zlib gzFile stream, and using talloc */
> +ssize_t
> +gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream) {
> +

opening brace ({) starting function content to it's own line.

> +    size_t len = *n;
> +    char *buf = *lineptr;
> +    size_t offset = 0;
> +
> +    if (len == 0 || buf == NULL) {
> +	/* same as getdelim from gnulib */
> +	len = 120;
> +	buf = talloc_size (ctx, len);
> +	if (buf == NULL)
> +	    return -1;
> +    }
> +
> +    while (1) {
> +

extra empty line ?

> +	if (!gzgets (stream, buf + offset, len - offset))

! gzgets

> +	    return -1;
> +
> +	offset += strlen (buf+offset);

(buf + offset)

> +
> +	if ( buf[offset-1] == '\n' )

[ offset - 1 ]

> +	    break;
> +
> +	len *= 2;
> +	buf = talloc_realloc (ctx, buf, char, len);
> +	if (buf == NULL)
> +	    return -1;
> +

extra empty line

> +    }
> +
> +    *lineptr = buf;
> +    *n = len;
> +    return offset;
> +}
> diff --git a/util/zlib-extra.h b/util/zlib-extra.h
> new file mode 100644
> index 0000000..c18480f
> --- /dev/null
> +++ b/util/zlib-extra.h
> @@ -0,0 +1,10 @@
> +#ifndef _ZLIB_EXTRA_H
> +#define _ZLIB_EXTRA_H
> +
> +#include <zlib.h>
> +
> +/* Like getline, but read from a gzFile. Allocation is with talloc */
> +ssize_t
> +gzgetline (void *ctx, char **lineptr, size_t *n, gzFile stream);
> +
> +#endif
> -- 
> 1.9.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v3] util: add gzreadline
  2014-03-30 12:45               ` Tomi Ollila
@ 2014-03-30 14:37                 ` David Bremner
  2014-03-30 16:13                   ` Tomi Ollila
  0 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2014-03-30 14:37 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

> "On error, gzgets() shall return Z_NULL.
>
> In light of this I'd say no. can we set out-of-memory using some public
> api so it is available using gzerror()
>
>

Since we already "broke" the API, what about

1) start a "util_status_t" enum, later unify that with the hex_ return codes.

2) 

util_status_t
gz_getline (void *ctx, char **buf, size_t *bufsize, ssize_t *bytes_read,
           gzFile stream);

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v3] util: add gzreadline
  2014-03-30 14:37                 ` David Bremner
@ 2014-03-30 16:13                   ` Tomi Ollila
  0 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2014-03-30 16:13 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sun, Mar 30 2014, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> "On error, gzgets() shall return Z_NULL.
>>
>> In light of this I'd say no. can we set out-of-memory using some public
>> api so it is available using gzerror()
>>
>>
>
> Since we already "broke" the API, what about
>
> 1) start a "util_status_t" enum, later unify that with the hex_ return codes.
>
> 2) 
>
> util_status_t
> gz_getline (void *ctx, char **buf, size_t *bufsize, ssize_t *bytes_read,
>            gzFile stream);

that looks unobjectionable to me :D

>
> d

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/2] cli: notmuch dump abstractions
  2014-03-25 17:07 [PATCH 0/2] cli: notmuch dump abstractions Jani Nikula
                   ` (2 preceding siblings ...)
  2014-03-26 22:01 ` [PATCH 0/2] cli: notmuch dump abstractions Mark Walters
@ 2014-03-30 22:33 ` David Bremner
  3 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2014-03-30 22:33 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> These are non-functional prep work for letting us do database dumps from
> elsewhere in notmuch, e.g. automatically before a database upgrade.
>
> BR,
> Jani.

series pushed

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-03-30 22:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-25 17:07 [PATCH 0/2] cli: notmuch dump abstractions Jani Nikula
2014-03-25 17:07 ` [PATCH 1/2] cli: abstract database dumping from the dump command Jani Nikula
2014-03-25 17:07 ` [PATCH 2/2] cli: abstract dump file open " Jani Nikula
2014-03-25 17:48   ` [PATCH v2] " Jani Nikula
2014-03-26 22:01 ` [PATCH 0/2] cli: notmuch dump abstractions Mark Walters
2014-03-29  1:20   ` [PATCH] RFC: impliment gzipped output for notmuch dump David Bremner
2014-03-29  7:16     ` Tomi Ollila
2014-03-29  9:25     ` Jani Nikula
2014-03-29 12:29       ` David Bremner
2014-03-29 13:02         ` Jani Nikula
2014-03-29 16:25           ` David Bremner
2014-03-29 13:46     ` [PATCH] dump: support gzipped output David Bremner
2014-03-29 18:16       ` David Bremner
2014-03-29 18:16         ` [Patch v2 1/3] dump: support gzipped output David Bremner
2014-03-29 18:16         ` [Patch v2 2/3] util: add gzreadline David Bremner
2014-03-30  8:30           ` Tomi Ollila
2014-03-30 11:23             ` [Patch v3] " David Bremner
2014-03-30 12:45               ` Tomi Ollila
2014-03-30 14:37                 ` David Bremner
2014-03-30 16:13                   ` Tomi Ollila
2014-03-30 11:03           ` [Patch v2 2/3] " David Bremner
2014-03-29 18:16         ` [Patch v2 3/3] restore: transparently support gzipped input David Bremner
2014-03-30 22:33 ` [PATCH 0/2] cli: notmuch dump abstractions David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).