[PATCH v2 0/9] FreeBSD Support Patches

[PATCH v2 0/9] FreeBSD Support Patches

[Mike Kelly]

Hi,

Here's a second attempt at adding FreeBSD support. Hopefully most of the
concerns raised about the initial patch set have been addressed.

I'll send my "I don't have emacs" patches in a separate email.

[PATCH v2 1/9] configure: Add support for FreeBSD.

[Mike Kelly]

This makes FreeBSD a recognized platform. Follow up patches make it
work properly.
---
 configure |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 5602be2..344f413 100755
--- a/configure
+++ b/configure
@@ -360,6 +360,10 @@ elif [ $uname = "SunOS" ] ; then
     printf "Solaris.\n"
     platform=SOLARIS
     linker_resolves_library_dependencies=0
+elif [ $uname = "FreeBSD" ] ; then
+    printf "FreeBSD.\n"
+    platform=FREEBSD
+    linker_resolves_library_dependencies=0
 elif [ $uname = "Linux" ] || [ $uname = "GNU" ] ; then
     printf "$uname\n"
     platform="$uname"
@@ -649,7 +653,7 @@ HAVE_GETLINE = ${have_getline}
 # build its own version)
 HAVE_STRCASESTR = ${have_strcasestr}
 
-# Supported platforms (so far) are: LINUX, MACOSX, SOLARIS
+# Supported platforms (so far) are: LINUX, MACOSX, SOLARIS, FREEBSD
 PLATFORM = ${platform}
 
 # Whether the linker will automatically resolve the dependency of one
-- 
1.7.10.2

[PATCH v2 2/9] test/Makefile.local: Use $(XAPIAN_LDFLAGS) for symbol-test

[Mike Kelly]

On FreeBSD, and probably anywhere else someone installed xapian to
some other prefix, we need to use XAPIAN_LDFLAGS to make the linker can
actually find libxapian.
---
 test/Makefile.local |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/Makefile.local b/test/Makefile.local
index 4a6a4b1..c071aee 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -17,7 +17,7 @@ $(dir)/smtp-dummy: $(smtp_dummy_modules)
 	$(call quiet,CC) $^ -o $@
 
 $(dir)/symbol-test: $(dir)/symbol-test.o
-	$(call quiet,CXX) $^ -o $@ -Llib -lnotmuch -lxapian
+	$(call quiet,CXX) $^ -o $@ -Llib -lnotmuch $(XAPIAN_LDFLAGS)
 
 .PHONY: test check
 
-- 
1.7.10.2

[PATCH v2 3/9] Makefile.local: define _GNU_SOURCE

[Mike Kelly]

Several places currently define _GNU_SOURCE above the #includes to make
sure that things like 'getline(3)' are available, but defining these in
one place also helps to improve portability to other platforms where
these may not be available otherwise.
---
 Makefile.local           |    4 ++--
 compat/have_getline.c    |    1 -
 compat/have_strcasestr.c |    1 -
 lib/notmuch-private.h    |    3 ---
 notmuch-client.h         |    3 ---
 5 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index 53b4a0d..48826b6 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -39,8 +39,8 @@ GPG_FILE=$(SHA1_FILE).asc
 PV_FILE=bindings/python/notmuch/version.py
 
 # Smash together user's values with our extra values
-FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags)
-FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags)
+FINAL_CFLAGS = -D_GNU_SOURCE -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags)
+FINAL_CXXFLAGS = -D_GNU_SOURCE $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags)
 FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)
 FINAL_NOTMUCH_LINKER = CC
 ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
diff --git a/compat/have_getline.c b/compat/have_getline.c
index a8bcd17..30f0e8f 100644
--- a/compat/have_getline.c
+++ b/compat/have_getline.c
@@ -1,4 +1,3 @@
-#define _GNU_SOURCE
 #include <stdio.h>
 #include <sys/types.h>
 
diff --git a/compat/have_strcasestr.c b/compat/have_strcasestr.c
index c0fb762..e637b46 100644
--- a/compat/have_strcasestr.c
+++ b/compat/have_strcasestr.c
@@ -1,4 +1,3 @@
-#define _GNU_SOURCE
 #include <strings.h>
 
 int main()
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..95afc3f 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -21,9 +21,6 @@
 #ifndef NOTMUCH_PRIVATE_H
 #define NOTMUCH_PRIVATE_H
 
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE /* For getline and asprintf */
-#endif
 #include <stdio.h>
 
 #include "compat.h"
diff --git a/notmuch-client.h b/notmuch-client.h
index 19b7f01..3964229 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -21,9 +21,6 @@
 #ifndef NOTMUCH_CLIENT_H
 #define NOTMUCH_CLIENT_H
 
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE /* for getline */
-#endif
 #include <stdio.h>
 
 #include "compat.h"
-- 
1.7.10.2

[PATCH v2 4/9] test/smtp-dummy.c: fix compilation on FreeBSD

[Mike Kelly]

Use the more portable netint/in.h, instead of netint/ip.h, to include
htons(3), etc.
---
 test/smtp-dummy.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 3801a5e..86d4316 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -37,7 +37,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
-#include <netinet/ip.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
 #include <netdb.h>
 #include <unistd.h>
 
-- 
1.7.10.2

[PATCH v2 5/9] tests: Test against source man pages.

[Mike Kelly]

Without this, help-test tests against the installed man pages, rather
than the default ones.
---
 test/test-lib.sh |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 06aaea2..da792b7 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1061,6 +1061,7 @@ find_notmuch_path ()
 # Test the binaries we have just built.  The tests are kept in
 # test/ subdirectory and are run in 'trash directory' subdirectory.
 TEST_DIRECTORY=$(pwd)
+notmuch_path=`find_notmuch_path "$TEST_DIRECTORY"`
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -1121,11 +1122,15 @@ then
 	PATH=$GIT_VALGRIND/bin:$PATH
 	GIT_EXEC_PATH=$GIT_VALGRIND/bin
 	export GIT_VALGRIND
+	test -n "$notmuch_path" && MANPATH="$notmuch_path/man:$MANPATH"
 else # normal case
-	notmuch_path=`find_notmuch_path "$TEST_DIRECTORY"`
-	test -n "$notmuch_path" && PATH="$notmuch_path:$PATH"
+	if test -n "$notmuch_path"
+		then
+			PATH="$notmuch_path:$PATH"
+			MANPATH="$notmuch_path/man:$MANPATH"
+		fi
 fi
-export PATH
+export PATH MANPATH
 
 # Test repository
 test="tmp.$(basename "$0" .sh)"
-- 
1.7.10.2

[PATCH v2 6/9] test/basic: use portable args for find

[Mike Kelly]

`-executable` isn't available in FreeBSD's version of find, so use a
more portable version, `-perm +111`.
---
 test/basic |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/basic b/test/basic
index d6aed24..3b635c8 100755
--- a/test/basic
+++ b/test/basic
@@ -53,8 +53,8 @@ test_expect_code 2 'failure to clean up causes the test to fail' '
 test_begin_subtest 'Ensure that all available tests will be run by notmuch-test'
 eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
-available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf '%f\n' | \
-    sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" | \
+available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -perm +111 | \
+    sed -r -e "s,.*/,," -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" | \
     sort)
 test_expect_equal "$tests_in_suite" "$available"
 
-- 
1.7.10.2

[PATCH v2 7/9] test/count: fix numeric comparision tests on FreeBSD

[Mike Kelly]

FreeBSD's `wc -l` includes some white space in front of the number.
Instead, we add a test_expect_equal_num() to test-lib.sh, which ensures
we do a proper numeric comparision, and in a portable way.
---
 test/count       |    8 ++++----
 test/test-lib.sh |   22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/test/count b/test/count
index 300b171..902c7c6 100755
--- a/test/count
+++ b/test/count
@@ -7,22 +7,22 @@ add_email_corpus
 SEARCH="\"*\""
 
 test_begin_subtest "message count is the default for notmuch count"
-test_expect_equal \
+test_expect_equal_num \
     "`notmuch search --output=messages ${SEARCH} | wc -l`" \
     "`notmuch count ${SEARCH}`"
 
 test_begin_subtest "message count with --output=messages"
-test_expect_equal \
+test_expect_equal_num \
     "`notmuch search --output=messages ${SEARCH} | wc -l`" \
     "`notmuch count --output=messages ${SEARCH}`"
 
 test_begin_subtest "thread count with --output=threads"
-test_expect_equal \
+test_expect_equal_num \
     "`notmuch search --output=threads ${SEARCH} | wc -l`" \
     "`notmuch count --output=threads ${SEARCH}`"
 
 test_begin_subtest "thread count is the default for notmuch search"
-test_expect_equal \
+test_expect_equal_num \
     "`notmuch search ${SEARCH} | wc -l`" \
     "`notmuch count --output=threads ${SEARCH}`"
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index da792b7..2ac92f7 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -512,6 +512,28 @@ test_expect_equal_file ()
     fi
 }
 
+# Like test_expect_equal, but does a numeric comparision instead of a
+# string comparision
+test_expect_equal_num () {
+	exec 1>&6 2>&7		# Restore stdout and stderr
+	inside_subtest=
+	test "$#" = 3 && { prereq=$1; shift; } || prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
+
+	output="$1"
+	expected="$2"
+	if ! test_skip "$test_subtest_name"
+	then
+		if test "$output" -eq "$expected"
+		then
+			test_ok_ "$test_subtest_name"
+		else
+			test_failure_ "$test_subtest_name" "$output != $expected"
+		fi
+	fi
+}
+
 test_emacs_expect_t () {
 	test "$#" = 2 && { prereq=$1; shift; } || prereq=
 	test "$#" = 1 ||
-- 
1.7.10.2

[PATCH v2 8/9] test/atomicity: use a more portable 'cp' form

[Mike Kelly]

-a already implies -r/-R, so no need for both.

FreeBSD's cp complains:

  cp: the -R and -r options may not be specified together
---
 test/atomicity |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/atomicity b/test/atomicity
index 6df0a00..1c786fa 100755
--- a/test/atomicity
+++ b/test/atomicity
@@ -51,7 +51,7 @@ if test_require_external_prereq gdb; then
 
     # Prepare a snapshot of the updated maildir.  The gdb script will
     # update the database in this snapshot as it goes.
-    cp -ra $MAIL_DIR $MAIL_DIR.snap
+    cp -a $MAIL_DIR $MAIL_DIR.snap
     cp ${NOTMUCH_CONFIG} ${NOTMUCH_CONFIG}.snap
     NOTMUCH_CONFIG=${NOTMUCH_CONFIG}.snap notmuch config set database.path $MAIL_DIR.snap
 
-- 
1.7.10.2

[PATCH v2 9/9] tests: bsd sed doesn't allow replacing '\n'

[Mike Kelly]

Various workarounds still don't act quite the same way. So, just use
perl, which seems to act the way these tests expect.
---
 test/maildir-sync |    2 +-
 test/multipart    |   22 +++++++++++-----------
 test/test-lib.sh  |    2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/test/maildir-sync b/test/maildir-sync
index d72ec07..313238b 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -8,7 +8,7 @@ test_description="maildir synchronization"
 # --format=json" output includes some newlines. Also, need to avoid
 # including the local value of MAIL_DIR in the result.
 filter_show_json() {
-    sed -e 's/, /,\n/g'  | sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
+    perl -pe 's/, /,\n/g' | sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
     echo
 }
 
diff --git a/test/multipart b/test/multipart
index 72d3927..b15eea0 100755
--- a/test/multipart
+++ b/test/multipart
@@ -319,7 +319,7 @@ test_expect_success \
     "notmuch show --format=text --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=json --part=0, full message"
-notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -337,7 +337,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=1, message body"
-notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -354,7 +354,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=2, multipart/mixed"
-notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -369,7 +369,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=3, rfc822 part"
-notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -381,7 +381,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=4, rfc822's multipart/alternative"
-notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -392,7 +392,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=5, rfc822's html part"
-notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -401,7 +401,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=6, rfc822's text part"
-notmuch show --format=json --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -410,7 +410,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=7, inline attachment"
-notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -419,7 +419,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=8, plain text part"
-notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -428,7 +428,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--format=json --part=9, pgp signature (unverified)"
-notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
+notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' | perl -pe 's|{"id":|\n{"id":|g' >OUTPUT
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
@@ -669,4 +669,4 @@ notmuch show --format=raw --part=3 id:base64-part-with-crlf > crlf.out
 echo -n -e "\xEF\x0D\x0A" > crlf.expected
 test_expect_equal_file crlf.out crlf.expected
 
-test_done
\ No newline at end of file
+test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2ac92f7..7a3602e 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -587,7 +587,7 @@ notmuch_show_sanitize_all ()
 
 notmuch_json_show_sanitize ()
 {
-    sed -e 's|, |,\n |g' | \
+    perl -pe 's|, |,\n |g' | \
 	sed \
 	-e 's|"id": "[^"]*",|"id": "XXXXX",|' \
 	-e 's|"filename": "[^"]*",|"filename": "YYYYY",|'
-- 
1.7.10.2

Re: [PATCH v2 0/9] FreeBSD Support Patches

[Tomi Ollila]

On Wed, May 30 2012, Mike Kelly <pioto@pioto.org> wrote:

> Hi,
>
> Here's a second attempt at adding FreeBSD support. Hopefully most of the
> concerns raised about the initial patch set have been addressed.

LGTM and patches apply, compiles fine and related tests pass.

Tomi

Re: [PATCH v2 7/9] test/count: fix numeric comparision tests on FreeBSD

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

On Wed, May 30 2012, Mike Kelly <pioto@pioto.org> wrote:
> FreeBSD's `wc -l` includes some white space in front of the number.
> Instead, we add a test_expect_equal_num() to test-lib.sh, which ensures
> we do a proper numeric comparision, and in a portable way.

Is there a way we can avoid adding another new test function here?  This
new function is almost completely identical with the old one, except
that it uses "test .. -eq" instead of "[ .. =".  If the problem is just
that the arguments occasionally contain superfluous spaces, I would
rather see the existing function just strip the spaces.  Of maybe add an
option to the test to ask for arithmetic rather than string comparison.
That would be preferable to introducing an new function that is
confusingly similar to an existing one.

> +# Like test_expect_equal, but does a numeric comparision instead of a
> +# string comparision
> +test_expect_equal_num () {
> +	exec 1>&6 2>&7		# Restore stdout and stderr
> +	inside_subtest=
> +	test "$#" = 3 && { prereq=$1; shift; } || prereq=
> +	test "$#" = 2 ||
> +	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
> +
> +	output="$1"
> +	expected="$2"
> +	if ! test_skip "$test_subtest_name"
> +	then
> +		if test "$output" -eq "$expected"
> +		then
> +			test_ok_ "$test_subtest_name"
> +		else
> +			test_failure_ "$test_subtest_name" "$output != $expected"
> +		fi
> +	fi
> +}

Also, the new function is missing the diff output, which is important
for debugging, and the syntax is different from the standard we've been
using:

  if ...; then

We try to maintain syntax consistency across the code base.  Thanks.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v2 7/9] test/count: fix numeric comparision tests on FreeBSD

[Tomi Ollila]

On Wed, May 30 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:

> On Wed, May 30 2012, Mike Kelly <pioto@pioto.org> wrote:
>> FreeBSD's `wc -l` includes some white space in front of the number.
>> Instead, we add a test_expect_equal_num() to test-lib.sh, which ensures
>> we do a proper numeric comparision, and in a portable way.
>
> Is there a way we can avoid adding another new test function here?  This
> new function is almost completely identical with the old one, except
> that it uses "test .. -eq" instead of "[ .. =".  If the problem is just
> that the arguments occasionally contain superfluous spaces, I would
> rather see the existing function just strip the spaces.  Of maybe add an
> option to the test to ask for arithmetic rather than string comparison.
> That would be preferable to introducing an new function that is
> confusingly similar to an existing one.

I liked Mike's suggestion as I also thought the options; stripping in
test_expect_equal cannot be done as there might be whitespace differences
which mean failure arguments to test_expect_equal cannot be given without
quotes as variable might be split into multiple args. if test_expect_equal
were changed to take comparison argument (like '-eq' or '=')  then all
calls would need to be changes (and function name could be changed to
something more generic...) None of these sounded as good options...

But, there is at least one option more:

test_expect_equal \
-    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
+    "$((`notmuch search --output=messages ${SEARCH} | wc -l`))" \
    "`notmuch count --output=messages ${SEARCH}`"

this makes arithmetic evaluation -- no operation to be done but
the spaces around the value are dropped.

>
> jamie.

Tomi

Re: [PATCH v2 3/9] Makefile.local: define _GNU_SOURCE

[Jani Nikula]

On Wed, 30 May 2012, Mike Kelly <pioto@pioto.org> wrote:
> Several places currently define _GNU_SOURCE above the #includes to make
> sure that things like 'getline(3)' are available, but defining these in
> one place also helps to improve portability to other platforms where
> these may not be available otherwise.
> ---
>  Makefile.local           |    4 ++--
>  compat/have_getline.c    |    1 -
>  compat/have_strcasestr.c |    1 -
>  lib/notmuch-private.h    |    3 ---
>  notmuch-client.h         |    3 ---
>  5 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile.local b/Makefile.local
> index 53b4a0d..48826b6 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -39,8 +39,8 @@ GPG_FILE=$(SHA1_FILE).asc
>  PV_FILE=bindings/python/notmuch/version.py
>  
>  # Smash together user's values with our extra values
> -FINAL_CFLAGS = -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags)
> -FINAL_CXXFLAGS = $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags)
> +FINAL_CFLAGS = -D_GNU_SOURCE -DNOTMUCH_VERSION=$(VERSION) $(CFLAGS) $(WARN_CFLAGS) $(CONFIGURE_CFLAGS) $(extra_cflags)
> +FINAL_CXXFLAGS = -D_GNU_SOURCE $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(extra_cflags) $(extra_cxxflags)
>  FINAL_NOTMUCH_LDFLAGS = $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS)

Sorry for neither being very clear the last time, nor having the time to
look into this. Shouldn't -D_GNU_SOURCE be in the configure script? We
could add it to CFLAGS there.

BR,
Jani.

>  FINAL_NOTMUCH_LINKER = CC
>  ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1)
> diff --git a/compat/have_getline.c b/compat/have_getline.c
> index a8bcd17..30f0e8f 100644
> --- a/compat/have_getline.c
> +++ b/compat/have_getline.c
> @@ -1,4 +1,3 @@
> -#define _GNU_SOURCE
>  #include <stdio.h>
>  #include <sys/types.h>
>  
> diff --git a/compat/have_strcasestr.c b/compat/have_strcasestr.c
> index c0fb762..e637b46 100644
> --- a/compat/have_strcasestr.c
> +++ b/compat/have_strcasestr.c
> @@ -1,4 +1,3 @@
> -#define _GNU_SOURCE
>  #include <strings.h>
>  
>  int main()
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index bfb4111..95afc3f 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -21,9 +21,6 @@
>  #ifndef NOTMUCH_PRIVATE_H
>  #define NOTMUCH_PRIVATE_H
>  
> -#ifndef _GNU_SOURCE
> -#define _GNU_SOURCE /* For getline and asprintf */
> -#endif
>  #include <stdio.h>
>  
>  #include "compat.h"
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..3964229 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -21,9 +21,6 @@
>  #ifndef NOTMUCH_CLIENT_H
>  #define NOTMUCH_CLIENT_H
>  
> -#ifndef _GNU_SOURCE
> -#define _GNU_SOURCE /* for getline */
> -#endif
>  #include <stdio.h>
>  
>  #include "compat.h"
> -- 
> 1.7.10.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 7/9] test/count: fix numeric comparision tests on FreeBSD

[Jani Nikula]

On Wed, 30 May 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, May 30 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
>
>> On Wed, May 30 2012, Mike Kelly <pioto@pioto.org> wrote:
>>> FreeBSD's `wc -l` includes some white space in front of the number.
>>> Instead, we add a test_expect_equal_num() to test-lib.sh, which ensures
>>> we do a proper numeric comparision, and in a portable way.
>>
>> Is there a way we can avoid adding another new test function here?  This
>> new function is almost completely identical with the old one, except
>> that it uses "test .. -eq" instead of "[ .. =".  If the problem is just
>> that the arguments occasionally contain superfluous spaces, I would
>> rather see the existing function just strip the spaces.  Of maybe add an
>> option to the test to ask for arithmetic rather than string comparison.
>> That would be preferable to introducing an new function that is
>> confusingly similar to an existing one.
>
> I liked Mike's suggestion as I also thought the options; stripping in
> test_expect_equal cannot be done as there might be whitespace differences
> which mean failure arguments to test_expect_equal cannot be given without
> quotes as variable might be split into multiple args. if test_expect_equal
> were changed to take comparison argument (like '-eq' or '=')  then all
> calls would need to be changes (and function name could be changed to
> something more generic...) None of these sounded as good options...
>
> But, there is at least one option more:
>
> test_expect_equal \
> -    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
> +    "$((`notmuch search --output=messages ${SEARCH} | wc -l`))" \
>     "`notmuch count --output=messages ${SEARCH}`"
>
> this makes arithmetic evaluation -- no operation to be done but
> the spaces around the value are dropped.

But it's non-obvious. How about just adding '| tr -d " "' to the end and
be done with it?

J.


>
>>
>> jamie.
>
> Tomi
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 0/9] FreeBSD Support Patches

[David Bremner]

Mike Kelly <pioto@pioto.org> writes:

> Hi,
>
> Here's a second attempt at adding FreeBSD support. Hopefully most of the
> concerns raised about the initial patch set have been addressed.

I pushed (finally) patches 1,2,4,5,6 and 8. The others either no longer
apply, or there were some outstanding issues in the discussion on the
list.

d

Re: [PATCH v2 0/9] FreeBSD Support Patches

[Mike Kelly]

Thanks, I'll try to take care of the concerns with the other 2 patches
at some point, but I haven't had as much free time lately as I'd like
to have.

On Sat, Sep 1, 2012 at 10:41 PM, David Bremner <david@tethera.net> wrote:
> Mike Kelly <pioto@pioto.org> writes:
>
>> Hi,
>>
>> Here's a second attempt at adding FreeBSD support. Hopefully most of the
>> concerns raised about the initial patch set have been addressed.
>
> I pushed (finally) patches 1,2,4,5,6 and 8. The others either no longer
> apply, or there were some outstanding issues in the discussion on the
> list.
>
> d



-- 
Mike Kelly