[PATCH v2 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
LIBNOTMUCH_VERSION_* macros. Check that the values of those
match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
---
edited patch file, reapplied and checked results. resent.
devel/release-checks.sh | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index d6410ad..7be57df 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -77,37 +77,36 @@ case $VERSION in
*) verfail "'$VERSION' is a single number" ;;
esac
-_set_version_components ()
-{
- VERSION_MAJOR=$1
- VERSION_MINOR=$2
- VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or "null" (string)
-}
+echo -n "Checking that LIBNOTMUCH version macros & variables match ... "
+# lib/notmuch.h
+LIBNOTMUCH_MAJOR_VERSION=broken
+LIBNOTMUCH_MINOR_VERSION=broken
+LIBNOTMUCH_MICRO_VERSION=broken
+# lib/Makefile.local
+LIBNOTMUCH_VERSION_MAJOR=borken
+LIBNOTMUCH_VERSION_MINOR=borken
+LIBNOTMUCH_VERSION_RELEASE=borken
+
+eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
+ && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
-IFS=.
-_set_version_components $VERSION
-IFS=$DEFAULT_IFS
+eval `awk 'NF == 3 && $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ && $2 == "=" \
+ && $3 ~ /^[0-9]+$/ { print $1 "=" $3 }' lib/Makefile.local`
-echo -n "Checking that libnotmuch version macros match $VERSION... "
-NOTMUCH_MAJOR_VERSION=broken
-NOTMUCH_MINOR_VERSION=broken
-NOTMUCH_MICRO_VERSION=broken
-eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
- && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
check_version_component ()
{
- eval local v1=\$VERSION_$1
- eval local v2=\$NOTMUCH_$1_VERSION
+ eval local v1=\$LIBNOTMUCH_$1_VERSION
+ eval local v2=\$LIBNOTMUCH_VERSION_$2
if [ $v1 != $v2 ]
- thenappend_emsg "NOTMUCH_$1_VERSION is defined as '$v2' in
lib/notmuch.h instead of '$v1'"
+ thenappend_emsg "LIBNOTMUCH_$1_VERSION ($v1) does not equal
LIBNOTMUCH_VERSION_$2 ($v2)"
fi
}
old_emsg_count=$emsg_count
-check_version_component MAJOR
-check_version_component MINOR
-check_version_component MICRO
+check_version_component MAJOR MAJOR
+check_version_component MINOR MINOR
+check_version_component MICRO RELEASE
[ $old_emsg_count = $emsg_count ] && echo Yes. || echo No.
echo -n "Checking that this is Debian package for notmuch... "
--
1.8.0
[PATCH 1/1] emacs: Makefile.local: HAVE_EMACS usage fixes
On Sat, Nov 30 2013, David Bremner wrote: > Tomi Ollila writes: > >> >> ifeq ($(WITH_EMACS),1) >> ifeq ($(HAVE_EMACS),1) >> all: $(emacs_bytecode) >> +install-emacs: $(emacs_bytecode) >> endif >> > > It's a bit confusing that we have both HAVE_EMACS and WITH_EMACS. Should > this be documented somewhere? Otherwise the patch seems sensible. > > See also the discussion at id:528B455A.70200 at iki.fi. Eventually it's probably good idea to merge HAVE_EMACS to WITH_EMACS but doing in a good way requires quite a few ./configure changes (and while doing that one have to check that some consistency is kept). Eventually it would probably be nice that in case *_EMACS is not set (is 0) the clean rules stay (provided that those do not require emacs) but build rules are ifdeffed out... In the meantime my patch here maintains status quo where things can be done in case HAVE_EMACS is set (1) -- emacs is available in the system, while keeping the cleanup rules in case *_EMACS is not set... and this patch also fixes the compilation whenever there is no emacs in system. I could add short notes of what HAVE_EMACS & WITH_EMACS mean but although it is bit confusing that we currently have both, those are nevertheless quite descriptive themselves. > > d Tomi > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] lib: add return status to database close and destroy
On Wed, 04 Dec 2013, Austin Clements wrote:
> Quoth Jani Nikula on Dec 01 at 3:13 pm:
>> notmuch_database_close may fail in Xapian ->flush() or ->close(), so
>> report the status. Similarly for notmuch_database_destroy which calls
>> close.
>>
>> This is required for notmuch insert to report error status if message
>> indexing failed.
>>
>> Bump the notmuch version to allow clients to conditional build against
>> both the current release and the next release (current git master).
>> ---
>> lib/database.cc | 18 ++
>> lib/notmuch.h | 17 ++---
>> 2 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/database.cc b/lib/database.cc
>> index f395061..98e2c31 100644
>> --- a/lib/database.cc
>> +++ b/lib/database.cc
>> @@ -767,14 +767,17 @@ notmuch_database_open (const char *path,
>> return status;
>> }
>>
>> -void
>> +notmuch_status_t
>> notmuch_database_close (notmuch_database_t *notmuch)
>> {
>> +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>> +
>> try {
>> if (notmuch->xapian_db != NULL &&
>> notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
>> (static_cast
>> (notmuch->xapian_db))->flush ();
>> } catch (const Xapian::Error ) {
>> +status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>> if (! notmuch->exception_reported) {
>> fprintf (stderr, "Error: A Xapian exception occurred flushing
>> database: %s\n",
>> error.get_msg().c_str());
>> @@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
>> notmuch->xapian_db->close();
>> } catch (const Xapian::Error ) {
>> /* do nothing */
>> +status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>> }
>> }
>>
>> @@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
>> notmuch->value_range_processor = NULL;
>> delete notmuch->date_range_processor;
>> notmuch->date_range_processor = NULL;
>> +
>> +return status;
>> }
>>
>> #if HAVE_XAPIAN_COMPACT
>> @@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,
>>
>>DONE:
>> if (notmuch)
>> -notmuch_database_destroy (notmuch);
>> +ret = notmuch_database_destroy (notmuch);
>
> This will clobber the error code on most of the error paths. Maybe
> you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?
Good point, will fix.
BR,
Jani.
>
>>
>> talloc_free (local);
>>
>> @@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),
>> }
>> #endif
>>
>> -void
>> +notmuch_status_t
>> notmuch_database_destroy (notmuch_database_t *notmuch)
>> {
>> -notmuch_database_close (notmuch);
>> +notmuch_status_t status;
>> +
>> +status = notmuch_database_close (notmuch);
>> talloc_free (notmuch);
>> +
>> +return status;
>> }
>>
>> const char *
>> diff --git a/lib/notmuch.h b/lib/notmuch.h
>> index 7c3a30c..dbdce86 100644
>> --- a/lib/notmuch.h
>> +++ b/lib/notmuch.h
>> @@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS
>> #endif
>>
>> #define NOTMUCH_MAJOR_VERSION 0
>> -#define NOTMUCH_MINOR_VERSION 17
>> +#define NOTMUCH_MINOR_VERSION 18
>> #define NOTMUCH_MICRO_VERSION 0
>
> This is actually what got me thinking about
> id:1386173986-9624-1-git-send-email-amdragon at mit.edu (which obviously
> conflicts). In particular, the Python bindings don't have access to
> these macros, and can only use the soname version. (I think the Go
> ones can use the macros; the Ruby ones certainly can.)
>
>>
>> /*
>> @@ -236,8 +236,16 @@ notmuch_database_open (const char *path,
>> *
>> * notmuch_database_close can be called multiple times. Later calls
>> * have no effect.
>> + *
>> + * Return value:
>> + *
>> + * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
>> + *
>> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the
>> + * database has been closed but there are no guarantees the
>> + * changes to the database, if any, have been flushed to disk.
>> */
>> -void
>> +notmuch_status_t
>> notmuch_database_close (notmuch_database_t *database);
>>
>> /* A callback invoked by notmuch_database_compact to notify the user
>> @@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,
>>
>> /* Destroy the notmuch database, closing it if necessary and freeing
>> * all associated resources.
>> + *
>> + * Return value as in notmuch_database_close if the database was open;
>> + * notmuch_database_destroy itself has no failure modes.
>> */
>> -void
>> +notmuch_status_t
>> notmuch_database_destroy (notmuch_database_t *database);
>>
>> /* Return the database path of the given database.
>
> Regarding bindings (since you asked me to think about them), these
> should be a safe changes from an ABI perspective (though obviously the
> bindings will need changes to take advantage of the new return code).
[PATCH 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
On Wed, 04 Dec 2013, Tomi Ollila wrote:
> NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
> LIBNOTMUCH_VERSION_* macros. Check that the values of those
> match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
> ---
> devel/release-checks.sh | 41 -
> 1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/devel/release-checks.sh b/devel/release-checks.sh
> index d6410ad..7be57df 100755
> --- a/devel/release-checks.sh
> +++ b/devel/release-checks.sh
> @@ -77,37 +77,36 @@ case $VERSION in
> *) verfail "'$VERSION' is a single number" ;;
> esac
>
> -_set_version_components ()
> -{
> - VERSION_MAJOR=$1
> - VERSION_MINOR=$2
> - VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or "null" (string)
> -}
> +echo -n "Checking that LIBNOTMUCH version macros & variables match ... "
> +# lib/notmuch.h
> +LIBNOTMUCH_MAJOR_VERSION=broken
> +LIBNOTMUCH_MINOR_VERSION=broken
> +LIBNOTMUCH_MICRO_VERSION=broken
> +# lib/Makefile.local
> +LIBNOTMUCH_VERSION_MAJOR=broken
> +LIBNOTMUCH_VERSION_MINOR=broken
> +LIBNOTMUCH_VERSION_RELEASE=broken
Does the test pass if both values are "broken"? Should the other set be
borken? Am I being too pessimistic? :)
At a glance, the patch looks good, but admittedly didn't spend too much
time on it.
BR,
Jani.
> +
> +eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
> + && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
>
> -IFS=.
> -_set_version_components $VERSION
> -IFS=$DEFAULT_IFS
> +eval `awk 'NF == 3 && $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ && $2 == "=" \
> + && $3 ~ /^[0-9]+$/ { print $1 "=" $3 }' lib/Makefile.local`
>
> -echo -n "Checking that libnotmuch version macros match $VERSION... "
> -NOTMUCH_MAJOR_VERSION=broken
> -NOTMUCH_MINOR_VERSION=broken
> -NOTMUCH_MICRO_VERSION=broken
> -eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
> - && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
>
> check_version_component ()
> {
> - eval local v1=\$VERSION_$1
> - eval local v2=\$NOTMUCH_$1_VERSION
> + eval local v1=\$LIBNOTMUCH_$1_VERSION
> + eval local v2=\$LIBNOTMUCH_VERSION_$2
> if [ $v1 != $v2 ]
> - thenappend_emsg "NOTMUCH_$1_VERSION is defined as '$v2' in
> lib/notmuch.h instead of '$v1'"
> + thenappend_emsg "LIBNOTMUCH_$1_VERSION ($v1) does not equal
> LIBNOTMUCH_VERSION_$2 ($v2)"
> fi
> }
>
> old_emsg_count=$emsg_count
> -check_version_component MAJOR
> -check_version_component MINOR
> -check_version_component MICRO
> +check_version_component MAJOR MAJOR
> +check_version_component MINOR MINOR
> +check_version_component MICRO RELEASE
> [ $old_emsg_count = $emsg_count ] && echo Yes. || echo No.
>
> echo -n "Checking that this is Debian package for notmuch... "
> --
> 1.8.0
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
LIBNOTMUCH_VERSION_* macros. Check that the values of those
match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
---
devel/release-checks.sh | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index d6410ad..7be57df 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -77,37 +77,36 @@ case $VERSION in
*) verfail "'$VERSION' is a single number" ;;
esac
-_set_version_components ()
-{
- VERSION_MAJOR=$1
- VERSION_MINOR=$2
- VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or "null" (string)
-}
+echo -n "Checking that LIBNOTMUCH version macros & variables match ... "
+# lib/notmuch.h
+LIBNOTMUCH_MAJOR_VERSION=broken
+LIBNOTMUCH_MINOR_VERSION=broken
+LIBNOTMUCH_MICRO_VERSION=broken
+# lib/Makefile.local
+LIBNOTMUCH_VERSION_MAJOR=broken
+LIBNOTMUCH_VERSION_MINOR=broken
+LIBNOTMUCH_VERSION_RELEASE=broken
+
+eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
+ && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
-IFS=.
-_set_version_components $VERSION
-IFS=$DEFAULT_IFS
+eval `awk 'NF == 3 && $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ && $2 == "=" \
+ && $3 ~ /^[0-9]+$/ { print $1 "=" $3 }' lib/Makefile.local`
-echo -n "Checking that libnotmuch version macros match $VERSION... "
-NOTMUCH_MAJOR_VERSION=broken
-NOTMUCH_MINOR_VERSION=broken
-NOTMUCH_MICRO_VERSION=broken
-eval `awk 'NF == 3 && $1 == "#define" && $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
- && $3 ~ /^[0-9]+$/ { print $2 "=" $3 }' lib/notmuch.h`
check_version_component ()
{
- eval local v1=\$VERSION_$1
- eval local v2=\$NOTMUCH_$1_VERSION
+ eval local v1=\$LIBNOTMUCH_$1_VERSION
+ eval local v2=\$LIBNOTMUCH_VERSION_$2
if [ $v1 != $v2 ]
- thenappend_emsg "NOTMUCH_$1_VERSION is defined as '$v2' in
lib/notmuch.h instead of '$v1'"
+ thenappend_emsg "LIBNOTMUCH_$1_VERSION ($v1) does not equal
LIBNOTMUCH_VERSION_$2 ($v2)"
fi
}
old_emsg_count=$emsg_count
-check_version_component MAJOR
-check_version_component MINOR
-check_version_component MICRO
+check_version_component MAJOR MAJOR
+check_version_component MINOR MINOR
+check_version_component MICRO RELEASE
[ $old_emsg_count = $emsg_count ] && echo Yes. || echo No.
echo -n "Checking that this is Debian package for notmuch... "
--
1.8.0
[PATCH 1/3] lib: Make VERSION macros agree with soname version
On Wed, 04 Dec 2013, Austin Clements wrote: > We have two distinct "library version" numbers: the soname version and > the version macros. We need both for different reasons: the version > macros enable easy compile-time version detection (and conditional > compilation), while the soname version enables runtime version > detection (which includes the version checking done by things like the > Python bindings). > > However, currently, these two version numbers are different, which is > unnecessary and can lead to confusion (especially in things like > Debian, which include the soname version in the package name). This > patch makes them the same by bumping the version macros up to agree > with the soname version. The patches look good to me. Thanks for spotting and fixing this in the nick of time before releasing. BR, Jani. > > (We should probably keep the version number in just one place so they > can't get out of sync, but that can be done in another patch.) > --- > lib/Makefile.local | 3 +++ > lib/notmuch.h | 8 ++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/lib/Makefile.local b/lib/Makefile.local > index 155ac02..cd2c60d 100644 > --- a/lib/Makefile.local > +++ b/lib/Makefile.local > @@ -18,6 +18,9 @@ LIBNOTMUCH_VERSION_MINOR = 0 > # simply compatible changes to the implementation). > LIBNOTMUCH_VERSION_RELEASE = 0 > > +# Note: Don't forget to change the VERSION macros in notmuch.h when > +# any of the above change. > + > ifeq ($(PLATFORM),MACOSX) > LIBRARY_SUFFIX = dylib > # On OS X, library version numbers go before suffix. > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 7c3a30c..42188a8 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -41,8 +41,12 @@ NOTMUCH_BEGIN_DECLS > #define TRUE 1 > #endif > > -#define NOTMUCH_MAJOR_VERSION0 > -#define NOTMUCH_MINOR_VERSION17 > +/* > + * The library version number. This must agree with the soname > + * version in Makefile.local. > + */ > +#define NOTMUCH_MAJOR_VERSION3 > +#define NOTMUCH_MINOR_VERSION0 > #define NOTMUCH_MICRO_VERSION0 > > /* > -- > 1.8.4.rc3
[PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage
On Wed, 04 Dec 2013, Austin Clements wrote:
> I just tried to use this and realized it hadn't been pushed yet.
>
> This series LGTM except one minor nit below and the fact that it
> introduces a lot of tab-indented code in sections of test-lib.sh that
> appear to be space-indented. Given that test-lib.sh is already a mess
> of indentation styles, I don't know if we care, but it would be nice if
> its entropy were at least non-increasing.
Ignore this comment. Apparently I was confused by show-mode
transforming leading tabs into spaces.
> On Mon, 25 Nov 2013, Tomi Ollila wrote:
>> When NOTMUCH_TEST_QUIET environment variable is set to non-null value
>> messages when new test script starts and when test PASSes are disabled.
>> This eases picking the cases when tests FAIL (as those are still printed).
>> ---
>> test/README | 8
>> test/basic | 4 ++--
>> test/test-lib.sh | 11 ++-
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/test/README b/test/README
>> index d12cff2..79a9b1b 100644
>> --- a/test/README
>> +++ b/test/README
>> @@ -74,10 +74,18 @@ the tests in one of the following ways.
>>
>> TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
>> TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
>> make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
>>
>> +Quiet Execution
>> +---
>> +
>> +Normally, when new script starts and when test PASSes you get a message
>> +printed on screen. This printing can be disabled by setting the
>> +NOTMUCH_TEST_QUIET variable to a non-null value. Message on test
>> +failures and skips are still printed.
>> +
>> Skipping Tests
>> --
>> If, for any reason, you need to skip one or more tests, you can do so
>> by setting the NOTMUCH_SKIP_TESTS variable to the name of one or more
>> sections of tests.
>> diff --git a/test/basic b/test/basic
>> index 64eb7d7..f7eed32 100755
>> --- a/test/basic
>> +++ b/test/basic
>> @@ -72,16 +72,16 @@ suppress_diff_date() {
>> sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
>> -e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
>> }
>>
>> test_begin_subtest "Ensure that test output is suppressed unless the test
>> fails"
>> -output=$(cd $TEST_DIRECTORY; ./test-verbose 2>&1 | suppress_diff_date)
>> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 2>&1 |
>> suppress_diff_date)
>> expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
>> test_expect_equal "$output" "$expected"
>>
>> test_begin_subtest "Ensure that -v does not suppress test output"
>> -output=$(cd $TEST_DIRECTORY; ./test-verbose -v 2>&1 | suppress_diff_date)
>> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 2>&1 |
>> suppress_diff_date)
>> expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
>> # Do not include the results of test-verbose in totals
>> rm $TEST_DIRECTORY/test-results/test-verbose
>> rm -r $TEST_DIRECTORY/tmp.test-verbose
>> test_expect_equal "$output" "$expected"
>> diff --git a/test/test-lib.sh b/test/test-lib.sh
>> index 34e0db6..9d4a807 100644
>> --- a/test/test-lib.sh
>> +++ b/test/test-lib.sh
>> @@ -196,11 +196,14 @@ print_test_description ()
>> test -z "$test_description_printed" || return 0
>> echo
>> echo $this_test: "Testing ${test_description}"
>> test_description_printed=1
>> }
>> -print_test_description
>> +if [ -z "$NOTMUCH_TEST_QUIET" ]
>> +then
>> +print_test_description
>> +fi
>>
>> exec 5>&1
>>
>> test_failure=0
>> test_count=0
>> @@ -715,20 +718,26 @@ test_ok_ () {
>> if test "$test_subtest_known_broken_" = "t"; then
>> test_known_broken_ok_
>> return
>> fi
>> test_success=$(($test_success + 1))
>> +if test -n "$NOTMUCH_TEST_QUIET"; then
>> +return 0
>> +fi
>> say_color pass "%-6s" "PASS"
>> echo " $test_subtest_name"
>> }
>>
>> test_failure_ () {
>> if test "$test_subtest_known_broken_" = "t"; then
>> test_known_broken_failure_ "$@"
>> return
>> fi
>> test_failure=$(($test_failure + 1))
>> +if test -n "$NOTMUCH_TEST_QUIET"; then
>
> Strictly speaking, this test isn't necessary, right?
>
>> +print_test_description
>> +fi
>> test_failure_message_ "FAIL" "$test_subtest_name" "$@"
>> test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
>> return 1
>> }
>>
>> --
>> 1.8.4.2
[PATCH 1/2] lib: add return status to database close and destroy
Quoth Jani Nikula on Dec 01 at 3:13 pm:
> notmuch_database_close may fail in Xapian ->flush() or ->close(), so
> report the status. Similarly for notmuch_database_destroy which calls
> close.
>
> This is required for notmuch insert to report error status if message
> indexing failed.
>
> Bump the notmuch version to allow clients to conditional build against
> both the current release and the next release (current git master).
> ---
> lib/database.cc | 18 ++
> lib/notmuch.h | 17 ++---
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index f395061..98e2c31 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -767,14 +767,17 @@ notmuch_database_open (const char *path,
> return status;
> }
>
> -void
> +notmuch_status_t
> notmuch_database_close (notmuch_database_t *notmuch)
> {
> +notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
> +
> try {
> if (notmuch->xapian_db != NULL &&
> notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
> (static_cast
> (notmuch->xapian_db))->flush ();
> } catch (const Xapian::Error ) {
> + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> if (! notmuch->exception_reported) {
> fprintf (stderr, "Error: A Xapian exception occurred flushing
> database: %s\n",
>error.get_msg().c_str());
> @@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
> notmuch->xapian_db->close();
> } catch (const Xapian::Error ) {
> /* do nothing */
> + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> }
> }
>
> @@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
> notmuch->value_range_processor = NULL;
> delete notmuch->date_range_processor;
> notmuch->date_range_processor = NULL;
> +
> +return status;
> }
>
> #if HAVE_XAPIAN_COMPACT
> @@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,
>
>DONE:
> if (notmuch)
> - notmuch_database_destroy (notmuch);
> + ret = notmuch_database_destroy (notmuch);
This will clobber the error code on most of the error paths. Maybe
you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?
>
> talloc_free (local);
>
> @@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),
> }
> #endif
>
> -void
> +notmuch_status_t
> notmuch_database_destroy (notmuch_database_t *notmuch)
> {
> -notmuch_database_close (notmuch);
> +notmuch_status_t status;
> +
> +status = notmuch_database_close (notmuch);
> talloc_free (notmuch);
> +
> +return status;
> }
>
> const char *
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 7c3a30c..dbdce86 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS
> #endif
>
> #define NOTMUCH_MAJOR_VERSION0
> -#define NOTMUCH_MINOR_VERSION17
> +#define NOTMUCH_MINOR_VERSION18
> #define NOTMUCH_MICRO_VERSION0
This is actually what got me thinking about
id:1386173986-9624-1-git-send-email-amdragon at mit.edu (which obviously
conflicts). In particular, the Python bindings don't have access to
these macros, and can only use the soname version. (I think the Go
ones can use the macros; the Ruby ones certainly can.)
>
> /*
> @@ -236,8 +236,16 @@ notmuch_database_open (const char *path,
> *
> * notmuch_database_close can be called multiple times. Later calls
> * have no effect.
> + *
> + * Return value:
> + *
> + * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the
> + * database has been closed but there are no guarantees the
> + * changes to the database, if any, have been flushed to disk.
> */
> -void
> +notmuch_status_t
> notmuch_database_close (notmuch_database_t *database);
>
> /* A callback invoked by notmuch_database_compact to notify the user
> @@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,
>
> /* Destroy the notmuch database, closing it if necessary and freeing
> * all associated resources.
> + *
> + * Return value as in notmuch_database_close if the database was open;
> + * notmuch_database_destroy itself has no failure modes.
> */
> -void
> +notmuch_status_t
> notmuch_database_destroy (notmuch_database_t *database);
>
> /* Return the database path of the given database.
Regarding bindings (since you asked me to think about them), these
should be a safe changes from an ABI perspective (though obviously the
bindings will need changes to take advantage of the new return code).
[PATCH 3/3] lib: Bump library version from 3.0.0 to 3.1.0
This version of the library introduces LIBNOTMUCH_CHECK_VERSION and the *_VERSION macros. Bumping the version number is also necessary to make the comment on LIBNOTMUCH_CHECK_VERSION no longer a lie. --- lib/Makefile.local | 2 +- lib/notmuch.h | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index cd2c60d..c56cba9 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -11,7 +11,7 @@ LIBNOTMUCH_VERSION_MAJOR = 3 # the time of release for any additions to the library interface, # (and when it is incremented, the release version of the library should # be reset to 0). -LIBNOTMUCH_VERSION_MINOR = 0 +LIBNOTMUCH_VERSION_MINOR = 1 # The release version the library interface. This should be incremented at # the time of release if there have been no changes to the interface, (but diff --git a/lib/notmuch.h b/lib/notmuch.h index cb108ef..d30768d 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -46,7 +46,7 @@ NOTMUCH_BEGIN_DECLS * version in Makefile.local. */ #define LIBNOTMUCH_MAJOR_VERSION 3 -#define LIBNOTMUCH_MINOR_VERSION 0 +#define LIBNOTMUCH_MINOR_VERSION 1 #define LIBNOTMUCH_MICRO_VERSION 0 /* @@ -55,11 +55,11 @@ NOTMUCH_BEGIN_DECLS * Return true if the library being compiled against is of the * specified version or above. For example: * - * #if LIBNOTMUCH_CHECK_VERSION(3, 0, 0) - * (code requiring libnotmuch 3.0.0 or above) + * #if LIBNOTMUCH_CHECK_VERSION(3, 1, 0) + * (code requiring libnotmuch 3.1.0 or above) * #endif * - * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.0.0; you + * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.1.0; you * can use #if !defined(NOTMUCH_CHECK_VERSION) to check for versions * prior to that. */ -- 1.8.4.rc3
[PATCH 2/3] lib: Replace NOTMUCH_*_VERSION with LIBNOTMUCH_*_VERSION
This makes it clear that these macros refer to the *library* version, and not to the notmuch application-level release. Since there are no consumers of these macros yet, this is now or never. --- lib/notmuch.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/notmuch.h b/lib/notmuch.h index 42188a8..cb108ef 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -45,9 +45,9 @@ NOTMUCH_BEGIN_DECLS * The library version number. This must agree with the soname * version in Makefile.local. */ -#define NOTMUCH_MAJOR_VERSION 3 -#define NOTMUCH_MINOR_VERSION 0 -#define NOTMUCH_MICRO_VERSION 0 +#define LIBNOTMUCH_MAJOR_VERSION 3 +#define LIBNOTMUCH_MINOR_VERSION 0 +#define LIBNOTMUCH_MICRO_VERSION 0 /* * Check the version of the notmuch library being compiled against. @@ -55,19 +55,19 @@ NOTMUCH_BEGIN_DECLS * Return true if the library being compiled against is of the * specified version or above. For example: * - * #if NOTMUCH_CHECK_VERSION(0, 18, 0) - * (code requiring notmuch 0.18 or above) + * #if LIBNOTMUCH_CHECK_VERSION(3, 0, 0) + * (code requiring libnotmuch 3.0.0 or above) * #endif * - * NOTMUCH_CHECK_VERSION has been defined since version 0.17.0; you + * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.0.0; you * can use #if !defined(NOTMUCH_CHECK_VERSION) to check for versions * prior to that. */ -#define NOTMUCH_CHECK_VERSION (major, minor, micro)\ -(NOTMUCH_MAJOR_VERSION > (major) || \ - (NOTMUCH_MAJOR_VERSION == (major) && NOTMUCH_MINOR_VERSION > (minor)) || \ - (NOTMUCH_MAJOR_VERSION == (major) && NOTMUCH_MINOR_VERSION == (minor) && \ - NOTMUCH_MICRO_VERSION >= (micro))) +#define LIBNOTMUCH_CHECK_VERSION (major, minor, micro) \ +(LIBNOTMUCH_MAJOR_VERSION > (major) || \ + (LIBNOTMUCH_MAJOR_VERSION == (major) && LIBNOTMUCH_MINOR_VERSION > (minor)) || \ + (LIBNOTMUCH_MAJOR_VERSION == (major) && LIBNOTMUCH_MINOR_VERSION == (minor) && \ + LIBNOTMUCH_MICRO_VERSION >= (micro))) typedef int notmuch_bool_t; -- 1.8.4.rc3
[PATCH 1/3] lib: Make VERSION macros agree with soname version
We have two distinct "library version" numbers: the soname version and the version macros. We need both for different reasons: the version macros enable easy compile-time version detection (and conditional compilation), while the soname version enables runtime version detection (which includes the version checking done by things like the Python bindings). However, currently, these two version numbers are different, which is unnecessary and can lead to confusion (especially in things like Debian, which include the soname version in the package name). This patch makes them the same by bumping the version macros up to agree with the soname version. (We should probably keep the version number in just one place so they can't get out of sync, but that can be done in another patch.) --- lib/Makefile.local | 3 +++ lib/notmuch.h | 8 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index 155ac02..cd2c60d 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -18,6 +18,9 @@ LIBNOTMUCH_VERSION_MINOR = 0 # simply compatible changes to the implementation). LIBNOTMUCH_VERSION_RELEASE = 0 +# Note: Don't forget to change the VERSION macros in notmuch.h when +# any of the above change. + ifeq ($(PLATFORM),MACOSX) LIBRARY_SUFFIX = dylib # On OS X, library version numbers go before suffix. diff --git a/lib/notmuch.h b/lib/notmuch.h index 7c3a30c..42188a8 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -41,8 +41,12 @@ NOTMUCH_BEGIN_DECLS #define TRUE 1 #endif -#define NOTMUCH_MAJOR_VERSION 0 -#define NOTMUCH_MINOR_VERSION 17 +/* + * The library version number. This must agree with the soname + * version in Makefile.local. + */ +#define NOTMUCH_MAJOR_VERSION 3 +#define NOTMUCH_MINOR_VERSION 0 #define NOTMUCH_MICRO_VERSION 0 /* -- 1.8.4.rc3
[PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage
I just tried to use this and realized it hadn't been pushed yet.
This series LGTM except one minor nit below and the fact that it
introduces a lot of tab-indented code in sections of test-lib.sh that
appear to be space-indented. Given that test-lib.sh is already a mess
of indentation styles, I don't know if we care, but it would be nice if
its entropy were at least non-increasing.
On Mon, 25 Nov 2013, Tomi Ollila wrote:
> When NOTMUCH_TEST_QUIET environment variable is set to non-null value
> messages when new test script starts and when test PASSes are disabled.
> This eases picking the cases when tests FAIL (as those are still printed).
> ---
> test/README | 8
> test/basic | 4 ++--
> test/test-lib.sh | 11 ++-
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/test/README b/test/README
> index d12cff2..79a9b1b 100644
> --- a/test/README
> +++ b/test/README
> @@ -74,10 +74,18 @@ the tests in one of the following ways.
>
> TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
> TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
> make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
>
> +Quiet Execution
> +---
> +
> +Normally, when new script starts and when test PASSes you get a message
> +printed on screen. This printing can be disabled by setting the
> +NOTMUCH_TEST_QUIET variable to a non-null value. Message on test
> +failures and skips are still printed.
> +
> Skipping Tests
> --
> If, for any reason, you need to skip one or more tests, you can do so
> by setting the NOTMUCH_SKIP_TESTS variable to the name of one or more
> sections of tests.
> diff --git a/test/basic b/test/basic
> index 64eb7d7..f7eed32 100755
> --- a/test/basic
> +++ b/test/basic
> @@ -72,16 +72,16 @@ suppress_diff_date() {
> sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
> -e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
> }
>
> test_begin_subtest "Ensure that test output is suppressed unless the test
> fails"
> -output=$(cd $TEST_DIRECTORY; ./test-verbose 2>&1 | suppress_diff_date)
> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 2>&1 |
> suppress_diff_date)
> expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
> test_expect_equal "$output" "$expected"
>
> test_begin_subtest "Ensure that -v does not suppress test output"
> -output=$(cd $TEST_DIRECTORY; ./test-verbose -v 2>&1 | suppress_diff_date)
> +output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 2>&1 |
> suppress_diff_date)
> expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
> # Do not include the results of test-verbose in totals
> rm $TEST_DIRECTORY/test-results/test-verbose
> rm -r $TEST_DIRECTORY/tmp.test-verbose
> test_expect_equal "$output" "$expected"
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 34e0db6..9d4a807 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -196,11 +196,14 @@ print_test_description ()
> test -z "$test_description_printed" || return 0
> echo
> echo $this_test: "Testing ${test_description}"
> test_description_printed=1
> }
> -print_test_description
> +if [ -z "$NOTMUCH_TEST_QUIET" ]
> +then
> + print_test_description
> +fi
>
> exec 5>&1
>
> test_failure=0
> test_count=0
> @@ -715,20 +718,26 @@ test_ok_ () {
> if test "$test_subtest_known_broken_" = "t"; then
> test_known_broken_ok_
> return
> fi
> test_success=$(($test_success + 1))
> + if test -n "$NOTMUCH_TEST_QUIET"; then
> + return 0
> + fi
> say_color pass "%-6s" "PASS"
> echo " $test_subtest_name"
> }
>
> test_failure_ () {
> if test "$test_subtest_known_broken_" = "t"; then
> test_known_broken_failure_ "$@"
> return
> fi
> test_failure=$(($test_failure + 1))
> + if test -n "$NOTMUCH_TEST_QUIET"; then
Strictly speaking, this test isn't necessary, right?
> + print_test_description
> + fi
> test_failure_message_ "FAIL" "$test_subtest_name" "$@"
> test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
> return 1
> }
>
> --
> 1.8.4.2
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH WIP v2 0/5] emacs: show: redesign unread/read logic
On Sun, 01 Dec 2013, Mark Walters wrote: > This is further wip of the message at > id:1385285551-5158-1-git-send-email-markwalters1009 at gmail.com > see there for some discussion of the design. > > This series is still definitely wip: one reason for posting is so > people can play with different strategies for marking read > easily. (As WIP tree's unread handling is broken and the tests need updating.) > > The series consists of three parts: the first 4 patches add the notion > of seen: this means the user has seen the message but the message has > typically not been marked read yet. The seen messages are marked read > when the user quits the show buffer unless the user quits with > prefix-arg quit. In all cases an informative message is shown. > > The fifth patch adds a psot-command-hook stub for updating the seen > status. This seems a natural place to do the update as it means > however the user navugates around the buffer (eg next-message or > page-down etc) the update gets done. > > This is intended to be an easy place for other people to try out their > own mark read strategies. > > The final patch implements something pretty close to what I would like > for marking seen/read. A message is deemed seen provided the user has > seen the top of the message, and has seen either the bottom of the > message or a point at least some customisable number of lines into the > message. The customisable number of lines can either be a fixed number > e.g. 20, or a number depending on the height of the current window > e.g. the default is 3/4 of the window height. > > The idea is a message seen if the user has seen the entire message, or > enough of it they have to have noticed it. The figure of 3/4 also > means that the notmuch commands like next-message which place the top > of the message at the top of the window automatically mark the message > seen as either the whole message or at least one window full must be > visible. > > I would be very grateful for any comments on whether this behaves as > people would expect, what they would want instead etc Hi Mark, thanks for working on this. I had tons of mail reading to catch up, so this was a good opportunity to try the patches. I'll try to be objective and constructive next, but up front, just so there's no doubt: I don't like it. I think my issues boil down to the series containing two pretty significant changes at once: how to decide if a message was read and when to apply the tag changes to reflect that. I found it confusing that messages were not being tagged -unread while I was viewing the thread. I found it even more confusing to get a message "Marked N messages read" on quitting show view with no feedback on *which* messages were read, and often the N didn't feel right either. And I think that's the problem: I wanted to see the new heuristics on deciding whether a message was read in action, but I got zero immediate feedback on it! My suggestion is to drop the delay in tag changes for now, and focus on the part that decides whether a message was read or not. Do the tag changes immediately when you consider a message "seen". I think this way we get a better feel of how well the heuristics really work, and we can make it just right. I think that's the bug in we currently have in notmuch, and delaying the tag changes doesn't contribute to fixing it. Indeed I think the delay makes it *harder* to fix. Afterwards, we could add the delayed tag changes (although hopefully as an option) if desired. And keeping that in mind, AFAICT you wouldn't need to rework your patches all that much. How does that sound? BR, Jani.
Re: [PATCH WIP v2 0/5] emacs: show: redesign unread/read logic
On Sun, 01 Dec 2013, Mark Walters markwalters1...@gmail.com wrote: This is further wip of the message at id:1385285551-5158-1-git-send-email-markwalters1...@gmail.com see there for some discussion of the design. This series is still definitely wip: one reason for posting is so people can play with different strategies for marking read easily. (As WIP tree's unread handling is broken and the tests need updating.) The series consists of three parts: the first 4 patches add the notion of seen: this means the user has seen the message but the message has typically not been marked read yet. The seen messages are marked read when the user quits the show buffer unless the user quits with prefix-arg quit. In all cases an informative message is shown. The fifth patch adds a psot-command-hook stub for updating the seen status. This seems a natural place to do the update as it means however the user navugates around the buffer (eg next-message or page-down etc) the update gets done. This is intended to be an easy place for other people to try out their own mark read strategies. The final patch implements something pretty close to what I would like for marking seen/read. A message is deemed seen provided the user has seen the top of the message, and has seen either the bottom of the message or a point at least some customisable number of lines into the message. The customisable number of lines can either be a fixed number e.g. 20, or a number depending on the height of the current window e.g. the default is 3/4 of the window height. The idea is a message seen if the user has seen the entire message, or enough of it they have to have noticed it. The figure of 3/4 also means that the notmuch commands like next-message which place the top of the message at the top of the window automatically mark the message seen as either the whole message or at least one window full must be visible. I would be very grateful for any comments on whether this behaves as people would expect, what they would want instead etc Hi Mark, thanks for working on this. I had tons of mail reading to catch up, so this was a good opportunity to try the patches. I'll try to be objective and constructive next, but up front, just so there's no doubt: I don't like it. I think my issues boil down to the series containing two pretty significant changes at once: how to decide if a message was read and when to apply the tag changes to reflect that. I found it confusing that messages were not being tagged -unread while I was viewing the thread. I found it even more confusing to get a message Marked N messages read on quitting show view with no feedback on *which* messages were read, and often the N didn't feel right either. And I think that's the problem: I wanted to see the new heuristics on deciding whether a message was read in action, but I got zero immediate feedback on it! My suggestion is to drop the delay in tag changes for now, and focus on the part that decides whether a message was read or not. Do the tag changes immediately when you consider a message seen. I think this way we get a better feel of how well the heuristics really work, and we can make it just right. I think that's the bug in we currently have in notmuch, and delaying the tag changes doesn't contribute to fixing it. Indeed I think the delay makes it *harder* to fix. Afterwards, we could add the delayed tag changes (although hopefully as an option) if desired. And keeping that in mind, AFAICT you wouldn't need to rework your patches all that much. How does that sound? BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] lib: Bump library version from 3.0.0 to 3.1.0
This version of the library introduces LIBNOTMUCH_CHECK_VERSION and the *_VERSION macros. Bumping the version number is also necessary to make the comment on LIBNOTMUCH_CHECK_VERSION no longer a lie. --- lib/Makefile.local | 2 +- lib/notmuch.h | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index cd2c60d..c56cba9 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -11,7 +11,7 @@ LIBNOTMUCH_VERSION_MAJOR = 3 # the time of release for any additions to the library interface, # (and when it is incremented, the release version of the library should # be reset to 0). -LIBNOTMUCH_VERSION_MINOR = 0 +LIBNOTMUCH_VERSION_MINOR = 1 # The release version the library interface. This should be incremented at # the time of release if there have been no changes to the interface, (but diff --git a/lib/notmuch.h b/lib/notmuch.h index cb108ef..d30768d 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -46,7 +46,7 @@ NOTMUCH_BEGIN_DECLS * version in Makefile.local. */ #define LIBNOTMUCH_MAJOR_VERSION 3 -#define LIBNOTMUCH_MINOR_VERSION 0 +#define LIBNOTMUCH_MINOR_VERSION 1 #define LIBNOTMUCH_MICRO_VERSION 0 /* @@ -55,11 +55,11 @@ NOTMUCH_BEGIN_DECLS * Return true if the library being compiled against is of the * specified version or above. For example: * - * #if LIBNOTMUCH_CHECK_VERSION(3, 0, 0) - * (code requiring libnotmuch 3.0.0 or above) + * #if LIBNOTMUCH_CHECK_VERSION(3, 1, 0) + * (code requiring libnotmuch 3.1.0 or above) * #endif * - * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.0.0; you + * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.1.0; you * can use #if !defined(NOTMUCH_CHECK_VERSION) to check for versions * prior to that. */ -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] lib: Make VERSION macros agree with soname version
We have two distinct library version numbers: the soname version and the version macros. We need both for different reasons: the version macros enable easy compile-time version detection (and conditional compilation), while the soname version enables runtime version detection (which includes the version checking done by things like the Python bindings). However, currently, these two version numbers are different, which is unnecessary and can lead to confusion (especially in things like Debian, which include the soname version in the package name). This patch makes them the same by bumping the version macros up to agree with the soname version. (We should probably keep the version number in just one place so they can't get out of sync, but that can be done in another patch.) --- lib/Makefile.local | 3 +++ lib/notmuch.h | 8 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index 155ac02..cd2c60d 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -18,6 +18,9 @@ LIBNOTMUCH_VERSION_MINOR = 0 # simply compatible changes to the implementation). LIBNOTMUCH_VERSION_RELEASE = 0 +# Note: Don't forget to change the VERSION macros in notmuch.h when +# any of the above change. + ifeq ($(PLATFORM),MACOSX) LIBRARY_SUFFIX = dylib # On OS X, library version numbers go before suffix. diff --git a/lib/notmuch.h b/lib/notmuch.h index 7c3a30c..42188a8 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -41,8 +41,12 @@ NOTMUCH_BEGIN_DECLS #define TRUE 1 #endif -#define NOTMUCH_MAJOR_VERSION 0 -#define NOTMUCH_MINOR_VERSION 17 +/* + * The library version number. This must agree with the soname + * version in Makefile.local. + */ +#define NOTMUCH_MAJOR_VERSION 3 +#define NOTMUCH_MINOR_VERSION 0 #define NOTMUCH_MICRO_VERSION 0 /* -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3] lib: Replace NOTMUCH_*_VERSION with LIBNOTMUCH_*_VERSION
This makes it clear that these macros refer to the *library* version, and not to the notmuch application-level release. Since there are no consumers of these macros yet, this is now or never. --- lib/notmuch.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/notmuch.h b/lib/notmuch.h index 42188a8..cb108ef 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -45,9 +45,9 @@ NOTMUCH_BEGIN_DECLS * The library version number. This must agree with the soname * version in Makefile.local. */ -#define NOTMUCH_MAJOR_VERSION 3 -#define NOTMUCH_MINOR_VERSION 0 -#define NOTMUCH_MICRO_VERSION 0 +#define LIBNOTMUCH_MAJOR_VERSION 3 +#define LIBNOTMUCH_MINOR_VERSION 0 +#define LIBNOTMUCH_MICRO_VERSION 0 /* * Check the version of the notmuch library being compiled against. @@ -55,19 +55,19 @@ NOTMUCH_BEGIN_DECLS * Return true if the library being compiled against is of the * specified version or above. For example: * - * #if NOTMUCH_CHECK_VERSION(0, 18, 0) - * (code requiring notmuch 0.18 or above) + * #if LIBNOTMUCH_CHECK_VERSION(3, 0, 0) + * (code requiring libnotmuch 3.0.0 or above) * #endif * - * NOTMUCH_CHECK_VERSION has been defined since version 0.17.0; you + * LIBNOTMUCH_CHECK_VERSION has been defined since version 3.0.0; you * can use #if !defined(NOTMUCH_CHECK_VERSION) to check for versions * prior to that. */ -#define NOTMUCH_CHECK_VERSION (major, minor, micro)\ -(NOTMUCH_MAJOR_VERSION (major) || \ - (NOTMUCH_MAJOR_VERSION == (major) NOTMUCH_MINOR_VERSION (minor)) || \ - (NOTMUCH_MAJOR_VERSION == (major) NOTMUCH_MINOR_VERSION == (minor) \ - NOTMUCH_MICRO_VERSION = (micro))) +#define LIBNOTMUCH_CHECK_VERSION (major, minor, micro) \ +(LIBNOTMUCH_MAJOR_VERSION (major) || \ + (LIBNOTMUCH_MAJOR_VERSION == (major) LIBNOTMUCH_MINOR_VERSION (minor)) || \ + (LIBNOTMUCH_MAJOR_VERSION == (major) LIBNOTMUCH_MINOR_VERSION == (minor) \ + LIBNOTMUCH_MICRO_VERSION = (micro))) typedef int notmuch_bool_t; -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage
I just tried to use this and realized it hadn't been pushed yet.
This series LGTM except one minor nit below and the fact that it
introduces a lot of tab-indented code in sections of test-lib.sh that
appear to be space-indented. Given that test-lib.sh is already a mess
of indentation styles, I don't know if we care, but it would be nice if
its entropy were at least non-increasing.
On Mon, 25 Nov 2013, Tomi Ollila tomi.oll...@iki.fi wrote:
When NOTMUCH_TEST_QUIET environment variable is set to non-null value
messages when new test script starts and when test PASSes are disabled.
This eases picking the cases when tests FAIL (as those are still printed).
---
test/README | 8
test/basic | 4 ++--
test/test-lib.sh | 11 ++-
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/test/README b/test/README
index d12cff2..79a9b1b 100644
--- a/test/README
+++ b/test/README
@@ -74,10 +74,18 @@ the tests in one of the following ways.
TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
+Quiet Execution
+---
+
+Normally, when new script starts and when test PASSes you get a message
+printed on screen. This printing can be disabled by setting the
+NOTMUCH_TEST_QUIET variable to a non-null value. Message on test
+failures and skips are still printed.
+
Skipping Tests
--
If, for any reason, you need to skip one or more tests, you can do so
by setting the NOTMUCH_SKIP_TESTS variable to the name of one or more
sections of tests.
diff --git a/test/basic b/test/basic
index 64eb7d7..f7eed32 100755
--- a/test/basic
+++ b/test/basic
@@ -72,16 +72,16 @@ suppress_diff_date() {
sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
-e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
}
test_begin_subtest Ensure that test output is suppressed unless the test
fails
-output=$(cd $TEST_DIRECTORY; ./test-verbose 21 | suppress_diff_date)
+output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 21 |
suppress_diff_date)
expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
test_expect_equal $output $expected
test_begin_subtest Ensure that -v does not suppress test output
-output=$(cd $TEST_DIRECTORY; ./test-verbose -v 21 | suppress_diff_date)
+output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 21 |
suppress_diff_date)
expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
# Do not include the results of test-verbose in totals
rm $TEST_DIRECTORY/test-results/test-verbose
rm -r $TEST_DIRECTORY/tmp.test-verbose
test_expect_equal $output $expected
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 34e0db6..9d4a807 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -196,11 +196,14 @@ print_test_description ()
test -z $test_description_printed || return 0
echo
echo $this_test: Testing ${test_description}
test_description_printed=1
}
-print_test_description
+if [ -z $NOTMUCH_TEST_QUIET ]
+then
+ print_test_description
+fi
exec 51
test_failure=0
test_count=0
@@ -715,20 +718,26 @@ test_ok_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_ok_
return
fi
test_success=$(($test_success + 1))
+ if test -n $NOTMUCH_TEST_QUIET; then
+ return 0
+ fi
say_color pass %-6s PASS
echo $test_subtest_name
}
test_failure_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_failure_ $@
return
fi
test_failure=$(($test_failure + 1))
+ if test -n $NOTMUCH_TEST_QUIET; then
Strictly speaking, this test isn't necessary, right?
+ print_test_description
+ fi
test_failure_message_ FAIL $test_subtest_name $@
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
return 1
}
--
1.8.4.2
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] lib: add return status to database close and destroy
Quoth Jani Nikula on Dec 01 at 3:13 pm:
notmuch_database_close may fail in Xapian -flush() or -close(), so
report the status. Similarly for notmuch_database_destroy which calls
close.
This is required for notmuch insert to report error status if message
indexing failed.
Bump the notmuch version to allow clients to conditional build against
both the current release and the next release (current git master).
---
lib/database.cc | 18 ++
lib/notmuch.h | 17 ++---
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index f395061..98e2c31 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -767,14 +767,17 @@ notmuch_database_open (const char *path,
return status;
}
-void
+notmuch_status_t
notmuch_database_close (notmuch_database_t *notmuch)
{
+notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+
try {
if (notmuch-xapian_db != NULL
notmuch-mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
(static_cast Xapian::WritableDatabase *
(notmuch-xapian_db))-flush ();
} catch (const Xapian::Error error) {
+ status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch-exception_reported) {
fprintf (stderr, Error: A Xapian exception occurred flushing
database: %s\n,
error.get_msg().c_str());
@@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
notmuch-xapian_db-close();
} catch (const Xapian::Error error) {
/* do nothing */
+ status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
}
}
@@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
notmuch-value_range_processor = NULL;
delete notmuch-date_range_processor;
notmuch-date_range_processor = NULL;
+
+return status;
}
#if HAVE_XAPIAN_COMPACT
@@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,
DONE:
if (notmuch)
- notmuch_database_destroy (notmuch);
+ ret = notmuch_database_destroy (notmuch);
This will clobber the error code on most of the error paths. Maybe
you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?
talloc_free (local);
@@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),
}
#endif
-void
+notmuch_status_t
notmuch_database_destroy (notmuch_database_t *notmuch)
{
-notmuch_database_close (notmuch);
+notmuch_status_t status;
+
+status = notmuch_database_close (notmuch);
talloc_free (notmuch);
+
+return status;
}
const char *
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7c3a30c..dbdce86 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS
#endif
#define NOTMUCH_MAJOR_VERSION0
-#define NOTMUCH_MINOR_VERSION17
+#define NOTMUCH_MINOR_VERSION18
#define NOTMUCH_MICRO_VERSION0
This is actually what got me thinking about
id:1386173986-9624-1-git-send-email-amdra...@mit.edu (which obviously
conflicts). In particular, the Python bindings don't have access to
these macros, and can only use the soname version. (I think the Go
ones can use the macros; the Ruby ones certainly can.)
/*
@@ -236,8 +236,16 @@ notmuch_database_open (const char *path,
*
* notmuch_database_close can be called multiple times. Later calls
* have no effect.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the
+ * database has been closed but there are no guarantees the
+ * changes to the database, if any, have been flushed to disk.
*/
-void
+notmuch_status_t
notmuch_database_close (notmuch_database_t *database);
/* A callback invoked by notmuch_database_compact to notify the user
@@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,
/* Destroy the notmuch database, closing it if necessary and freeing
* all associated resources.
+ *
+ * Return value as in notmuch_database_close if the database was open;
+ * notmuch_database_destroy itself has no failure modes.
*/
-void
+notmuch_status_t
notmuch_database_destroy (notmuch_database_t *database);
/* Return the database path of the given database.
Regarding bindings (since you asked me to think about them), these
should be a safe changes from an ABI perspective (though obviously the
bindings will need changes to take advantage of the new return code).
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
LIBNOTMUCH_VERSION_* macros. Check that the values of those
match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
---
devel/release-checks.sh | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index d6410ad..7be57df 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -77,37 +77,36 @@ case $VERSION in
*) verfail '$VERSION' is a single number ;;
esac
-_set_version_components ()
-{
- VERSION_MAJOR=$1
- VERSION_MINOR=$2
- VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or null (string)
-}
+echo -n Checking that LIBNOTMUCH version macros variables match ...
+# lib/notmuch.h
+LIBNOTMUCH_MAJOR_VERSION=broken
+LIBNOTMUCH_MINOR_VERSION=broken
+LIBNOTMUCH_MICRO_VERSION=broken
+# lib/Makefile.local
+LIBNOTMUCH_VERSION_MAJOR=broken
+LIBNOTMUCH_VERSION_MINOR=broken
+LIBNOTMUCH_VERSION_RELEASE=broken
+
+eval `awk 'NF == 3 $1 == #define $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
+$3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
-IFS=.
-_set_version_components $VERSION
-IFS=$DEFAULT_IFS
+eval `awk 'NF == 3 $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ $2 == = \
+$3 ~ /^[0-9]+$/ { print $1 = $3 }' lib/Makefile.local`
-echo -n Checking that libnotmuch version macros match $VERSION...
-NOTMUCH_MAJOR_VERSION=broken
-NOTMUCH_MINOR_VERSION=broken
-NOTMUCH_MICRO_VERSION=broken
-eval `awk 'NF == 3 $1 == #define $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
-$3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
check_version_component ()
{
- eval local v1=\$VERSION_$1
- eval local v2=\$NOTMUCH_$1_VERSION
+ eval local v1=\$LIBNOTMUCH_$1_VERSION
+ eval local v2=\$LIBNOTMUCH_VERSION_$2
if [ $v1 != $v2 ]
- thenappend_emsg NOTMUCH_$1_VERSION is defined as '$v2' in
lib/notmuch.h instead of '$v1'
+ thenappend_emsg LIBNOTMUCH_$1_VERSION ($v1) does not equal
LIBNOTMUCH_VERSION_$2 ($v2)
fi
}
old_emsg_count=$emsg_count
-check_version_component MAJOR
-check_version_component MINOR
-check_version_component MICRO
+check_version_component MAJOR MAJOR
+check_version_component MINOR MINOR
+check_version_component MICRO RELEASE
[ $old_emsg_count = $emsg_count ] echo Yes. || echo No.
echo -n Checking that this is Debian package for notmuch...
--
1.8.0
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3] lib: Make VERSION macros agree with soname version
On Wed, 04 Dec 2013, Austin Clements amdra...@mit.edu wrote: We have two distinct library version numbers: the soname version and the version macros. We need both for different reasons: the version macros enable easy compile-time version detection (and conditional compilation), while the soname version enables runtime version detection (which includes the version checking done by things like the Python bindings). However, currently, these two version numbers are different, which is unnecessary and can lead to confusion (especially in things like Debian, which include the soname version in the package name). This patch makes them the same by bumping the version macros up to agree with the soname version. The patches look good to me. Thanks for spotting and fixing this in the nick of time before releasing. BR, Jani. (We should probably keep the version number in just one place so they can't get out of sync, but that can be done in another patch.) --- lib/Makefile.local | 3 +++ lib/notmuch.h | 8 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index 155ac02..cd2c60d 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -18,6 +18,9 @@ LIBNOTMUCH_VERSION_MINOR = 0 # simply compatible changes to the implementation). LIBNOTMUCH_VERSION_RELEASE = 0 +# Note: Don't forget to change the VERSION macros in notmuch.h when +# any of the above change. + ifeq ($(PLATFORM),MACOSX) LIBRARY_SUFFIX = dylib # On OS X, library version numbers go before suffix. diff --git a/lib/notmuch.h b/lib/notmuch.h index 7c3a30c..42188a8 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -41,8 +41,12 @@ NOTMUCH_BEGIN_DECLS #define TRUE 1 #endif -#define NOTMUCH_MAJOR_VERSION0 -#define NOTMUCH_MINOR_VERSION17 +/* + * The library version number. This must agree with the soname + * version in Makefile.local. + */ +#define NOTMUCH_MAJOR_VERSION3 +#define NOTMUCH_MINOR_VERSION0 #define NOTMUCH_MICRO_VERSION0 /* -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
On Wed, 04 Dec 2013, Tomi Ollila tomi.oll...@iki.fi wrote:
NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
LIBNOTMUCH_VERSION_* macros. Check that the values of those
match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
---
devel/release-checks.sh | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index d6410ad..7be57df 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -77,37 +77,36 @@ case $VERSION in
*) verfail '$VERSION' is a single number ;;
esac
-_set_version_components ()
-{
- VERSION_MAJOR=$1
- VERSION_MINOR=$2
- VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or null (string)
-}
+echo -n Checking that LIBNOTMUCH version macros variables match ...
+# lib/notmuch.h
+LIBNOTMUCH_MAJOR_VERSION=broken
+LIBNOTMUCH_MINOR_VERSION=broken
+LIBNOTMUCH_MICRO_VERSION=broken
+# lib/Makefile.local
+LIBNOTMUCH_VERSION_MAJOR=broken
+LIBNOTMUCH_VERSION_MINOR=broken
+LIBNOTMUCH_VERSION_RELEASE=broken
Does the test pass if both values are broken? Should the other set be
borken? Am I being too pessimistic? :)
At a glance, the patch looks good, but admittedly didn't spend too much
time on it.
BR,
Jani.
+
+eval `awk 'NF == 3 $1 == #define $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
+ $3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
-IFS=.
-_set_version_components $VERSION
-IFS=$DEFAULT_IFS
+eval `awk 'NF == 3 $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ $2 == = \
+ $3 ~ /^[0-9]+$/ { print $1 = $3 }' lib/Makefile.local`
-echo -n Checking that libnotmuch version macros match $VERSION...
-NOTMUCH_MAJOR_VERSION=broken
-NOTMUCH_MINOR_VERSION=broken
-NOTMUCH_MICRO_VERSION=broken
-eval `awk 'NF == 3 $1 == #define $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
- $3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
check_version_component ()
{
- eval local v1=\$VERSION_$1
- eval local v2=\$NOTMUCH_$1_VERSION
+ eval local v1=\$LIBNOTMUCH_$1_VERSION
+ eval local v2=\$LIBNOTMUCH_VERSION_$2
if [ $v1 != $v2 ]
- thenappend_emsg NOTMUCH_$1_VERSION is defined as '$v2' in
lib/notmuch.h instead of '$v1'
+ thenappend_emsg LIBNOTMUCH_$1_VERSION ($v1) does not equal
LIBNOTMUCH_VERSION_$2 ($v2)
fi
}
old_emsg_count=$emsg_count
-check_version_component MAJOR
-check_version_component MINOR
-check_version_component MICRO
+check_version_component MAJOR MAJOR
+check_version_component MINOR MINOR
+check_version_component MICRO RELEASE
[ $old_emsg_count = $emsg_count ] echo Yes. || echo No.
echo -n Checking that this is Debian package for notmuch...
--
1.8.0
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] lib: add return status to database close and destroy
On Wed, 04 Dec 2013, Austin Clements amdra...@mit.edu wrote:
Quoth Jani Nikula on Dec 01 at 3:13 pm:
notmuch_database_close may fail in Xapian -flush() or -close(), so
report the status. Similarly for notmuch_database_destroy which calls
close.
This is required for notmuch insert to report error status if message
indexing failed.
Bump the notmuch version to allow clients to conditional build against
both the current release and the next release (current git master).
---
lib/database.cc | 18 ++
lib/notmuch.h | 17 ++---
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index f395061..98e2c31 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -767,14 +767,17 @@ notmuch_database_open (const char *path,
return status;
}
-void
+notmuch_status_t
notmuch_database_close (notmuch_database_t *notmuch)
{
+notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+
try {
if (notmuch-xapian_db != NULL
notmuch-mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
(static_cast Xapian::WritableDatabase *
(notmuch-xapian_db))-flush ();
} catch (const Xapian::Error error) {
+status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch-exception_reported) {
fprintf (stderr, Error: A Xapian exception occurred flushing
database: %s\n,
error.get_msg().c_str());
@@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
notmuch-xapian_db-close();
} catch (const Xapian::Error error) {
/* do nothing */
+status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
}
}
@@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
notmuch-value_range_processor = NULL;
delete notmuch-date_range_processor;
notmuch-date_range_processor = NULL;
+
+return status;
}
#if HAVE_XAPIAN_COMPACT
@@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,
DONE:
if (notmuch)
-notmuch_database_destroy (notmuch);
+ret = notmuch_database_destroy (notmuch);
This will clobber the error code on most of the error paths. Maybe
you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?
Good point, will fix.
BR,
Jani.
talloc_free (local);
@@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),
}
#endif
-void
+notmuch_status_t
notmuch_database_destroy (notmuch_database_t *notmuch)
{
-notmuch_database_close (notmuch);
+notmuch_status_t status;
+
+status = notmuch_database_close (notmuch);
talloc_free (notmuch);
+
+return status;
}
const char *
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7c3a30c..dbdce86 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS
#endif
#define NOTMUCH_MAJOR_VERSION 0
-#define NOTMUCH_MINOR_VERSION 17
+#define NOTMUCH_MINOR_VERSION 18
#define NOTMUCH_MICRO_VERSION 0
This is actually what got me thinking about
id:1386173986-9624-1-git-send-email-amdra...@mit.edu (which obviously
conflicts). In particular, the Python bindings don't have access to
these macros, and can only use the soname version. (I think the Go
ones can use the macros; the Ruby ones certainly can.)
/*
@@ -236,8 +236,16 @@ notmuch_database_open (const char *path,
*
* notmuch_database_close can be called multiple times. Later calls
* have no effect.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the
+ * database has been closed but there are no guarantees the
+ * changes to the database, if any, have been flushed to disk.
*/
-void
+notmuch_status_t
notmuch_database_close (notmuch_database_t *database);
/* A callback invoked by notmuch_database_compact to notify the user
@@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,
/* Destroy the notmuch database, closing it if necessary and freeing
* all associated resources.
+ *
+ * Return value as in notmuch_database_close if the database was open;
+ * notmuch_database_destroy itself has no failure modes.
*/
-void
+notmuch_status_t
notmuch_database_destroy (notmuch_database_t *database);
/* Return the database path of the given database.
Regarding bindings (since you asked me to think about them), these
should be a safe changes from an ABI perspective (though obviously the
bindings will need changes to take advantage of the new return code).
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/1] emacs: Makefile.local: HAVE_EMACS usage fixes
On Sat, Nov 30 2013, David Bremner da...@tethera.net wrote: Tomi Ollila tomi.oll...@iki.fi writes: ifeq ($(WITH_EMACS),1) ifeq ($(HAVE_EMACS),1) all: $(emacs_bytecode) +install-emacs: $(emacs_bytecode) endif It's a bit confusing that we have both HAVE_EMACS and WITH_EMACS. Should this be documented somewhere? Otherwise the patch seems sensible. See also the discussion at id:528b455a.70...@iki.fi. Eventually it's probably good idea to merge HAVE_EMACS to WITH_EMACS but doing in a good way requires quite a few ./configure changes (and while doing that one have to check that some consistency is kept). Eventually it would probably be nice that in case *_EMACS is not set (is 0) the clean rules stay (provided that those do not require emacs) but build rules are ifdeffed out... In the meantime my patch here maintains status quo where things can be done in case HAVE_EMACS is set (1) -- emacs is available in the system, while keeping the cleanup rules in case *_EMACS is not set... and this patch also fixes the compilation whenever there is no emacs in system. I could add short notes of what HAVE_EMACS WITH_EMACS mean but although it is bit confusing that we currently have both, those are nevertheless quite descriptive themselves. d Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/1] devel/release-checks.sh: adjust to LIBNOTMUCH version checks
NOTMUCH_VERSION_* macros in lib/notmuch.h are replaced with
LIBNOTMUCH_VERSION_* macros. Check that the values of those
match the LIBNOTMUCH_*_VERSION values in lib/Makefile.local.
---
edited patch file, reapplied and checked results. resent.
devel/release-checks.sh | 41 -
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index d6410ad..7be57df 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -77,37 +77,36 @@ case $VERSION in
*) verfail '$VERSION' is a single number ;;
esac
-_set_version_components ()
-{
- VERSION_MAJOR=$1
- VERSION_MINOR=$2
- VERSION_MICRO=${3:-0} # set to 0 in case $3 is unset or null (string)
-}
+echo -n Checking that LIBNOTMUCH version macros variables match ...
+# lib/notmuch.h
+LIBNOTMUCH_MAJOR_VERSION=broken
+LIBNOTMUCH_MINOR_VERSION=broken
+LIBNOTMUCH_MICRO_VERSION=broken
+# lib/Makefile.local
+LIBNOTMUCH_VERSION_MAJOR=borken
+LIBNOTMUCH_VERSION_MINOR=borken
+LIBNOTMUCH_VERSION_RELEASE=borken
+
+eval `awk 'NF == 3 $1 == #define $2 ~ /^LIBNOTMUCH_[A-Z]+_VERSION$/ \
+$3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
-IFS=.
-_set_version_components $VERSION
-IFS=$DEFAULT_IFS
+eval `awk 'NF == 3 $1 ~ /^LIBNOTMUCH_VERSION_[A-Z]+$/ $2 == = \
+$3 ~ /^[0-9]+$/ { print $1 = $3 }' lib/Makefile.local`
-echo -n Checking that libnotmuch version macros match $VERSION...
-NOTMUCH_MAJOR_VERSION=broken
-NOTMUCH_MINOR_VERSION=broken
-NOTMUCH_MICRO_VERSION=broken
-eval `awk 'NF == 3 $1 == #define $2 ~ /^NOTMUCH_[A-Z]+_VERSION$/ \
-$3 ~ /^[0-9]+$/ { print $2 = $3 }' lib/notmuch.h`
check_version_component ()
{
- eval local v1=\$VERSION_$1
- eval local v2=\$NOTMUCH_$1_VERSION
+ eval local v1=\$LIBNOTMUCH_$1_VERSION
+ eval local v2=\$LIBNOTMUCH_VERSION_$2
if [ $v1 != $v2 ]
- thenappend_emsg NOTMUCH_$1_VERSION is defined as '$v2' in
lib/notmuch.h instead of '$v1'
+ thenappend_emsg LIBNOTMUCH_$1_VERSION ($v1) does not equal
LIBNOTMUCH_VERSION_$2 ($v2)
fi
}
old_emsg_count=$emsg_count
-check_version_component MAJOR
-check_version_component MINOR
-check_version_component MICRO
+check_version_component MAJOR MAJOR
+check_version_component MINOR MINOR
+check_version_component MICRO RELEASE
[ $old_emsg_count = $emsg_count ] echo Yes. || echo No.
echo -n Checking that this is Debian package for notmuch...
--
1.8.0
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] test: implement and document NOTMUCH_TEST_QUIET variable usage
On Wed, 04 Dec 2013, Austin Clements amdra...@mit.edu wrote:
I just tried to use this and realized it hadn't been pushed yet.
This series LGTM except one minor nit below and the fact that it
introduces a lot of tab-indented code in sections of test-lib.sh that
appear to be space-indented. Given that test-lib.sh is already a mess
of indentation styles, I don't know if we care, but it would be nice if
its entropy were at least non-increasing.
Ignore this comment. Apparently I was confused by show-mode
transforming leading tabs into spaces.
On Mon, 25 Nov 2013, Tomi Ollila tomi.oll...@iki.fi wrote:
When NOTMUCH_TEST_QUIET environment variable is set to non-null value
messages when new test script starts and when test PASSes are disabled.
This eases picking the cases when tests FAIL (as those are still printed).
---
test/README | 8
test/basic | 4 ++--
test/test-lib.sh | 11 ++-
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/test/README b/test/README
index d12cff2..79a9b1b 100644
--- a/test/README
+++ b/test/README
@@ -74,10 +74,18 @@ the tests in one of the following ways.
TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
+Quiet Execution
+---
+
+Normally, when new script starts and when test PASSes you get a message
+printed on screen. This printing can be disabled by setting the
+NOTMUCH_TEST_QUIET variable to a non-null value. Message on test
+failures and skips are still printed.
+
Skipping Tests
--
If, for any reason, you need to skip one or more tests, you can do so
by setting the NOTMUCH_SKIP_TESTS variable to the name of one or more
sections of tests.
diff --git a/test/basic b/test/basic
index 64eb7d7..f7eed32 100755
--- a/test/basic
+++ b/test/basic
@@ -72,16 +72,16 @@ suppress_diff_date() {
sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
-e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
}
test_begin_subtest Ensure that test output is suppressed unless the test
fails
-output=$(cd $TEST_DIRECTORY; ./test-verbose 21 | suppress_diff_date)
+output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 21 |
suppress_diff_date)
expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
test_expect_equal $output $expected
test_begin_subtest Ensure that -v does not suppress test output
-output=$(cd $TEST_DIRECTORY; ./test-verbose -v 21 | suppress_diff_date)
+output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 21 |
suppress_diff_date)
expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
# Do not include the results of test-verbose in totals
rm $TEST_DIRECTORY/test-results/test-verbose
rm -r $TEST_DIRECTORY/tmp.test-verbose
test_expect_equal $output $expected
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 34e0db6..9d4a807 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -196,11 +196,14 @@ print_test_description ()
test -z $test_description_printed || return 0
echo
echo $this_test: Testing ${test_description}
test_description_printed=1
}
-print_test_description
+if [ -z $NOTMUCH_TEST_QUIET ]
+then
+print_test_description
+fi
exec 51
test_failure=0
test_count=0
@@ -715,20 +718,26 @@ test_ok_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_ok_
return
fi
test_success=$(($test_success + 1))
+if test -n $NOTMUCH_TEST_QUIET; then
+return 0
+fi
say_color pass %-6s PASS
echo $test_subtest_name
}
test_failure_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_failure_ $@
return
fi
test_failure=$(($test_failure + 1))
+if test -n $NOTMUCH_TEST_QUIET; then
Strictly speaking, this test isn't necessary, right?
+print_test_description
+fi
test_failure_message_ FAIL $test_subtest_name $@
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
return 1
}
--
1.8.4.2
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] lib: introduce notmuch_database_new for initializing a database handle
Quoth Jani Nikula on Dec 01 at 3:14 pm:
There is a need for setting options before opening a database, such as
setting a logging function to use instead of writing to stdout or
stderr. It would be possible to do this by adding new parameters to
notmuch_database_create and notmuch_database_open, but maintaining a
backwards compatible API and ABI when new options are added becomes
burdensome.
Instead, split the opaque database object creation from
notmuch_database_create and notmuch_database_open into a new
notmuch_database_new call, to allow operations on the handle before
create and open. This creates API and ABI breakage now, but allows
easier future extensions.
The notmuch_database_new call becomes a natural pair to the already
existing notmuch_database_destroy, and it should be possible to call
open/close multiple times using an initialized handle.
A high-level comment about the API: Currently, an allocated
notmuch_database_t has two memory states, if you will: open and
closed. (I wish it didn't have any memory states, and was on the
fence about this API for a while until I realized that the ship had
already sailed.) It's pretty clear how all of the notmuch APIs will
behave in both states (modulo some bounded non-determinism in the
closed state). I think this patch introduces a new pre-open state,
and I don't know how most of the notmuch APIs behave in that state.
My guess is poorly. If it's feasible, I'd much rather a fresh baked
notmuch_database_t act like it's in the closed state, including that
notmuch_database_{create,open} are well-defined as transitions from
closed state to open state (even if the closed state was reached by
calling notmuch_database_close). Or, if we do have a pre-open
state, it should at least be well-specified what that means
(preferably the specification is *not* most APIs segfault).
Orthogonally -- and this may be a complete pipe dream of mine -- if we
just had a way to return more detailed error information than a simple
error code from notmuch_database_{create,open}, I think we wouldn't
need any of this. Everything that these functions currently log
(modulo one warning) is error details, so if we could return the error
details *with the error* or somehow make them accessible, we wouldn't
need a logger at this point (or at several other points in the
library).
---
lib/database.cc | 64
lib/notmuch.h| 52 --
notmuch-compact.c| 11 -
notmuch-count.c | 10 ++--
notmuch-dump.c | 10 ++--
notmuch-insert.c | 10 ++--
notmuch-new.c| 14 +++-
notmuch-reply.c | 10 ++--
notmuch-restore.c| 10 ++--
notmuch-search.c | 10 ++--
notmuch-show.c | 10 ++--
notmuch-tag.c| 10 ++--
test/random-corpus.c | 10 ++--
test/symbol-test.cc | 3 ++-
14 files changed, 166 insertions(+), 68 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 98e2c31..386b93a 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -539,10 +539,21 @@ parse_references (void *ctx,
}
notmuch_status_t
-notmuch_database_create (const char *path, notmuch_database_t **database)
+notmuch_database_new (notmuch_database_t **notmuch)
The naming of this is unfortunate... Other APIs use x_create to
allocate objects (e.g., notmuch_query_create, several internal APIs).
I would lean towards calling this function notmuch_database_create,
but that leaves the question of what to call the other. While we're
breaking APIs, would it be completely crazy to merge open and create
into one API with an extra mode to indicate creation (it can be its
own mode because creation implies read/write)? (Or, in UNIX
tradition, we could call this function notmuch_database_create and the
other notmuch_database_creat.) notmuch_database_create is already
just a shell around notmuch_database_open (we could keep it as a
separate function, but just make it internal).
+{
+/* Note: try to avoid error conditions! No error printing! */
+
+*notmuch = talloc_zero (NULL, notmuch_database_t);
+if (! *notmuch)
+ return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+notmuch_status_t
+notmuch_database_create (notmuch_database_t *notmuch, const char *path)
{
This should fail if passed a database that is already open.
notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
-notmuch_database_t *notmuch = NULL;
char *notmuch_path = NULL;
struct stat st;
int err;
@@ -579,25 +590,18 @@ notmuch_database_create (const char *path,
notmuch_database_t **database)
goto DONE;
}
-status = notmuch_database_open (path,
- NOTMUCH_DATABASE_MODE_READ_WRITE,
- notmuch);
+status = notmuch_database_open (notmuch, path,
+
