[PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

[PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Amadeusz Żołnowski]

If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
it's not so easy to pass $(srcdir) which is required to find notmuch.h
when srcdir and builddir are separate directories.
---
 test/.gitignore     |    1 +
 test/Makefile.local |    5 ++++-
 test/basic          |    2 +-
 test/symbol-hiding  |    3 +--
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 9e97052..7e30e8d 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,4 +1,5 @@
 test-results
 corpus.mail
 smtp-dummy
+symbol-test
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 8eb0433..a672fd3 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -11,8 +11,11 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
 	$(call quiet,CC) $^ -o $@
 
+$(dir)/symbol-test: $(dir)/symbol-test.o
+	$(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
+
 .PHONY: test check
-test:	all $(dir)/smtp-dummy
+test:	all $(dir)/smtp-dummy $(dir)/symbol-test
 	@${dir}/notmuch-test $(OPTIONS)
 
 check: test
diff --git a/test/basic b/test/basic
index 38db2ba..5463bf8 100755
--- a/test/basic
+++ b/test/basic
@@ -56,7 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(ls -1 $TEST_DIRECTORY/ | \
     sed -r -e "/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
 	   -e "/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
-	   -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d" \
+	   -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc|symbol-test|symbol-test.o)/d" \
 	   -e "/^(test.expected-output|.*~)/d" \
 	   -e "/^(gnupg-secret-key.asc)/d" \
 	   -e "/^(gnupg-secret-key.NOTE)/d" \
diff --git a/test/symbol-hiding b/test/symbol-hiding
index d0b31ae..f67b653 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -12,13 +12,12 @@ test_description='exception symbol hiding'
 . ./test-lib.sh
 
 run_test(){
-    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
+    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test 2>&1)
 }
 
 output="A Xapian exception occurred opening database: Couldn't stat 'fakedb/.notmuch/xapian'
 caught No chert database found at path \`./nonexistant'"
 
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
 mkdir -p fakedb/.notmuch
 test_expect_success 'running test' run_test
 test_begin_subtest 'checking output'
-- 
1.7.8.rc3

[PATCH 2/2] Fixed warnings for test/symbol-test.cc.

[Amadeusz Żołnowski]

---
 test/symbol-test.cc |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 1de06ea..bfbe38f 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -1,14 +1,13 @@
 #include <stdio.h>
 #include <xapian.h>
 #include <notmuch.h>
-main (int argc, char **argv){
+int main (){
 
-    notmuch_database_t *notmuch
-      = notmuch_database_open ("fakedb",
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
+  //notmuch_database_t *notmuch =
+  notmuch_database_open ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);
 
   try{
-    (void)new Xapian::WritableDatabase ("./nonexistant",					Xapian::DB_OPEN);
+    (void)new Xapian::WritableDatabase ("./nonexistant", Xapian::DB_OPEN);
   } catch (const Xapian::Error &error) {
     printf("caught %s\n",error.get_msg().c_str());
     return 0;
-- 
1.7.8.rc3

Re: [PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

[David Bremner]

On Fri, 25 Nov 2011 11:54:51 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
>  
> +$(dir)/symbol-test: $(dir)/symbol-test.o
> +	$(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian

I'm a bit surprised no -I is neaded here. Is that taken care in the CC call?

>  .PHONY: test check
> -test:	all $(dir)/smtp-dummy
> +test:	all $(dir)/smtp-dummy $(dir)/symbol-test
>  	@${dir}/notmuch-test $(OPTIONS)

The instructions were already wrong, but maybe they should be updated to
point out that calling the test scripts directly requires smtp-dummy and
symbol-test already be built. Or maybe the tests should check for these
binaries as pre-reqs along with emacs, gpg, etc... 

> +	   -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc|symbol-test|symbol-test.o)/d" \

I wonder if this should be shortened (and made a bit sloppier) with the
use of regexes.

Re: [PATCH 2/2] Fixed warnings for test/symbol-test.cc.

[David Bremner]

On Fri, 25 Nov 2011 11:54:52 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> ---
> +  //notmuch_database_t *notmuch =
> +  notmuch_database_open ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);

Leaving commented out code in is for people without version control ;).

Shouldn't this cast to void?

>  
>    try{
> -    (void)new Xapian::WritableDatabase ("./nonexistant",					Xapian::DB_OPEN);
> +    (void)new Xapian::WritableDatabase ("./nonexistant", Xapian::DB_OPEN);

This seems to be whitespace only?  I'm not against whitespace cleanup,
but please put it in seperate commits. Or am I staring past a real
change here?

d

Re: [PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Amadeusz Żołnowski]

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

Excerpts from David Bremner's message of 2011-11-25 14:04:44 +0100:
> On Fri, 25 Nov 2011 11:54:51 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> >  
> > +$(dir)/symbol-test: $(dir)/symbol-test.o
> > +    $(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
> 
> I'm a bit surprised no -I is neaded here. Is that taken care in the CC
> call?

-I is for headers which are not used when linking.  symbol-test.o is
compiled according to rule defined earlier in some parent Makefile.


> >  .PHONY: test check
> > -test:    all $(dir)/smtp-dummy
> > +test:    all $(dir)/smtp-dummy $(dir)/symbol-test
> >      @${dir}/notmuch-test $(OPTIONS)
> 
> The instructions were already wrong, but maybe they should be updated to
> point out that calling the test scripts directly requires smtp-dummy and
> symbol-test already be built. Or maybe the tests should check for these
> binaries as pre-reqs along with emacs, gpg, etc... 
> 
> > +       -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc|symbol-test|symbol-test.o)/d" \
> 
> I wonder if this should be shortened (and made a bit sloppier) with the
> use of regexes.

Thanks it's so straightforward.  I found this line quickly without deep
analysis of all test-framework. :-)  It could only be made more flexible
- put list of this files into some separate text file.  But that's
subject for another patch.  Purpose of this one is to make building out
of tree working.


--
Amadeusz Żołnowski

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH 2/3] Fix warnings for test/symbol-test.cc.

[Amadeusz Żołnowski]

---
 test/symbol-test.cc |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 1de06ea..ec250b2 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -1,10 +1,9 @@
 #include <stdio.h>
 #include <xapian.h>
 #include <notmuch.h>
-main (int argc, char **argv){
+int main (){
 
-    notmuch_database_t *notmuch
-      = notmuch_database_open ("fakedb",
+  (void)notmuch_database_open ("fakedb",
 				     NOTMUCH_DATABASE_MODE_READ_ONLY);
 
   try{
-- 
1.7.8.rc3

[PATCH 3/3] Whitespaces cleanup.

[Amadeusz Żołnowski]

---
 test/symbol-test.cc |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index ec250b2..1548ca4 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -1,16 +1,17 @@
 #include <stdio.h>
 #include <xapian.h>
 #include <notmuch.h>
-int main (){
 
-  (void)notmuch_database_open ("fakedb",
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
 
-  try{
-    (void)new Xapian::WritableDatabase ("./nonexistant",					Xapian::DB_OPEN);
+int main() {
+  (void) notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);
+
+  try {
+    (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
   } catch (const Xapian::Error &error) {
-    printf("caught %s\n",error.get_msg().c_str());
+    printf("caught %s\n", error.get_msg().c_str());
     return 0;
   }
+
   return 1;
 }
-- 
1.7.8.rc3

Re: [PATCH 1/2] Build symbol-test with make instead of hardcoding in symbol-hiding.

[David Bremner]

On Fri, 25 Nov 2011 11:54:51 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.
> ---

Sorry to do this to you, but I pushed Dmitry's patch to test/basic
first, so can you please rebase?

thanks

d

[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Amadeusz Żołnowski]

If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
it's not so easy to pass $(srcdir) which is required to find notmuch.h
when srcdir and builddir are separate directories.
---
 test/.gitignore     |    1 +
 test/Makefile.local |    5 ++++-
 test/basic          |    2 +-
 test/symbol-hiding  |    3 +--
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 9e97052..7e30e8d 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,4 +1,5 @@
 test-results
 corpus.mail
 smtp-dummy
+symbol-test
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9b3308a..b77f721 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -11,8 +11,11 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
 	$(call quiet,CC) $^ -o $@
 
+$(dir)/symbol-test: $(dir)/symbol-test.o
+	$(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
+
 .PHONY: test check
-test:	all $(dir)/smtp-dummy
+test:	all $(dir)/smtp-dummy $(dir)/symbol-test
 	@${dir}/notmuch-test $(OPTIONS)
 
 check: test
diff --git a/test/basic b/test/basic
index f258d1f..4edf831 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ 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)$/d" | \
+    sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" | \
     sort)
 test_expect_equal "$tests_in_suite" "$available"
 
diff --git a/test/symbol-hiding b/test/symbol-hiding
index d0b31ae..f67b653 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -12,13 +12,12 @@ test_description='exception symbol hiding'
 . ./test-lib.sh
 
 run_test(){
-    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
+    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test 2>&1)
 }
 
 output="A Xapian exception occurred opening database: Couldn't stat 'fakedb/.notmuch/xapian'
 caught No chert database found at path \`./nonexistant'"
 
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
 mkdir -p fakedb/.notmuch
 test_expect_success 'running test' run_test
 test_begin_subtest 'checking output'
-- 
1.7.8.rc3

Re: [PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Amadeusz Żołnowski]

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

Excerpts from Amadeusz Żołnowski's message of 2011-11-26 21:51:21 +0100:
> If symbol-test is built in symbol-hiding with hardcoded g++
> invokation, it's not so easy to pass $(srcdir) which is required to
> find notmuch.h when srcdir and builddir are separate directories.

Sorry, I've just realised that I've forgot to add symbol-test to clean
target.  Following patch includes clean, too.

-- 
Amadeusz Żołnowski

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Amadeusz Żołnowski]

If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
it's not so easy to pass $(srcdir) which is required to find notmuch.h
when srcdir and builddir are separate directories.
---
 test/.gitignore     |    1 +
 test/Makefile.local |    7 +++++--
 test/basic          |    2 +-
 test/symbol-hiding  |    3 +--
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 9e97052..7e30e8d 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,4 +1,5 @@
 test-results
 corpus.mail
 smtp-dummy
+symbol-test
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9b3308a..646779e 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -11,10 +11,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
 	$(call quiet,CC) $^ -o $@
 
+$(dir)/symbol-test: $(dir)/symbol-test.o
+	$(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
+
 .PHONY: test check
-test:	all $(dir)/smtp-dummy
+test:	all $(dir)/smtp-dummy $(dir)/symbol-test
 	@${dir}/notmuch-test $(OPTIONS)
 
 check: test
 
-CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o
+CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test $(dir)/symbol-test.o
diff --git a/test/basic b/test/basic
index f258d1f..4edf831 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ 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)$/d" | \
+    sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" | \
     sort)
 test_expect_equal "$tests_in_suite" "$available"
 
diff --git a/test/symbol-hiding b/test/symbol-hiding
index d0b31ae..f67b653 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -12,13 +12,12 @@ test_description='exception symbol hiding'
 . ./test-lib.sh
 
 run_test(){
-    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
+    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test 2>&1)
 }
 
 output="A Xapian exception occurred opening database: Couldn't stat 'fakedb/.notmuch/xapian'
 caught No chert database found at path \`./nonexistant'"
 
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
 mkdir -p fakedb/.notmuch
 test_expect_success 'running test' run_test
 test_begin_subtest 'checking output'
-- 
1.7.8.rc3

Re: [PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

[David Bremner]

On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.

pushed,

d

Re: [PATCH] Build symbol-test with make instead of hardcoding in symbol-hiding.

[Dmitry Kurochkin]

On Sat, 26 Nov 2011 22:14:20 +0100, Amadeusz Żołnowski <aidecoe@aidecoe.name> wrote:
> If symbol-test is built in symbol-hiding with hardcoded g++ invokation,
> it's not so easy to pass $(srcdir) which is required to find notmuch.h
> when srcdir and builddir are separate directories.
> ---
>  test/.gitignore     |    1 +
>  test/Makefile.local |    7 +++++--
>  test/basic          |    2 +-
>  test/symbol-hiding  |    3 +--
>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/test/.gitignore b/test/.gitignore
> index 9e97052..7e30e8d 100644
> --- a/test/.gitignore
> +++ b/test/.gitignore
> @@ -1,4 +1,5 @@
>  test-results
>  corpus.mail
>  smtp-dummy
> +symbol-test
>  tmp.*
> diff --git a/test/Makefile.local b/test/Makefile.local
> index 9b3308a..646779e 100644
> --- a/test/Makefile.local
> +++ b/test/Makefile.local
> @@ -11,10 +11,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
>  $(dir)/smtp-dummy: $(smtp_dummy_modules)
>  	$(call quiet,CC) $^ -o $@
>  
> +$(dir)/symbol-test: $(dir)/symbol-test.o
> +	$(call quiet,CC) $^ -o $@ -Llib -lnotmuch -lxapian
> +
>  .PHONY: test check
> -test:	all $(dir)/smtp-dummy
> +test:	all $(dir)/smtp-dummy $(dir)/symbol-test
>  	@${dir}/notmuch-test $(OPTIONS)
>  
>  check: test
>  
> -CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o
> +CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test $(dir)/symbol-test.o
> diff --git a/test/basic b/test/basic
> index f258d1f..4edf831 100755
> --- a/test/basic
> +++ b/test/basic
> @@ -54,7 +54,7 @@ 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)$/d" | \
> +    sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" | \

Can you please keep this list alphabetically sorted?

Regards,
  Dmitry

>      sort)
>  test_expect_equal "$tests_in_suite" "$available"
>  
> diff --git a/test/symbol-hiding b/test/symbol-hiding
> index d0b31ae..f67b653 100755
> --- a/test/symbol-hiding
> +++ b/test/symbol-hiding
> @@ -12,13 +12,12 @@ test_description='exception symbol hiding'
>  . ./test-lib.sh
>  
>  run_test(){
> -    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
> +    result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test 2>&1)
>  }
>  
>  output="A Xapian exception occurred opening database: Couldn't stat 'fakedb/.notmuch/xapian'
>  caught No chert database found at path \`./nonexistant'"
>  
> -g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
>  mkdir -p fakedb/.notmuch
>  test_expect_success 'running test' run_test
>  test_begin_subtest 'checking output'
> -- 
> 1.7.8.rc3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch