unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] tests: add compatibility layer
@ 2016-12-20 19:47 mp39590
  2016-12-20 22:24 ` David Bremner
  0 siblings, 1 reply; 16+ messages in thread
From: mp39590 @ 2016-12-20 19:47 UTC (permalink / raw)
  To: notmuch

From: Mikhail <mp39590@gmail.com>

Allow to expand aliases in test scritps and add glue to alias native BSD
utils into GNU equivalents.
---
 test/README      |  6 ++++++
 test/test-lib.sh | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/test/README b/test/README
index 104a120e..094e1d33 100644
--- a/test/README
+++ b/test/README
@@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
 
 e.g. env PATH=/opt/gnu/bin:$PATH make test
 
+For FreeBSD you will need to install coreutils, which provides GNU
+versions of basic utils like 'date' or 'wc'. Also you will need to
+install latest gdb from ports or packages and provide path to it in
+BSD_GDB variable before executing the tests, native FreeBSD gdb will not
+work.
+
 Running Tests
 -------------
 The easiest way to run tests is to say "make test", (or simply run the
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f55d2c67..96c1e095 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -26,6 +26,23 @@ fi
 # Make sure echo builtin does not expand backslash-escape sequences by default.
 shopt -u xpg_echo
 
+# OS independent functions
+#
+# Alias native BSD utilities to usable GNU equivalents.
+case `uname` in
+FreeBSD)
+	# allow using aliases in scripts
+	shopt -s expand_aliases
+
+	alias date=gdate
+	alias base64=gbase64
+	alias gdb=$BSD_GDB
+	alias wc=gwc
+	alias sed="gsed"
+	alias sha256sum=gsha256sum
+	;;
+esac
+
 this_test=${0##*/}
 this_test=${this_test%.sh}
 this_test_bare=${this_test#T[0-9][0-9][0-9]-}
-- 
2.11.0

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

* Re: [PATCH] tests: add compatibility layer
  2016-12-20 19:47 [PATCH] tests: add compatibility layer mp39590
@ 2016-12-20 22:24 ` David Bremner
  2016-12-21  5:12   ` Tomi Ollila
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: David Bremner @ 2016-12-20 22:24 UTC (permalink / raw)
  To: mp39590, notmuch

mp39590@gmail.com writes:

>  
> +# OS independent functions
> +#
> +# Alias native BSD utilities to usable GNU equivalents.
> +case `uname` in
> +FreeBSD)
> +	# allow using aliases in scripts
> +	shopt -s expand_aliases
> +
> +	alias date=gdate
> +	alias base64=gbase64
> +	alias gdb=$BSD_GDB
> +	alias wc=gwc
> +	alias sed="gsed"
> +	alias sha256sum=gsha256sum
> +	;;
> +esac
> +

What about adding (most of) this to sh.config by the configure script?
I'd like to centralize all hacky platform detection there.

Also, why is gsed in quotes but none of the others are?

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

* Re: [PATCH] tests: add compatibility layer
  2016-12-20 22:24 ` David Bremner
@ 2016-12-21  5:12   ` Tomi Ollila
  2016-12-21 14:56   ` Mikhail
  2016-12-22  9:31   ` mp39590
  2 siblings, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2016-12-21  5:12 UTC (permalink / raw)
  To: David Bremner, mp39590, notmuch

On Wed, Dec 21 2016, David Bremner <david@tethera.net> wrote:

> mp39590@gmail.com writes:
>
>>  
>> +# OS independent functions
>> +#
>> +# Alias native BSD utilities to usable GNU equivalents.
>> +case `uname` in
>> +FreeBSD)
>> +	# allow using aliases in scripts
>> +	shopt -s expand_aliases
>> +
>> +	alias date=gdate
>> +	alias base64=gbase64
>> +	alias gdb=$BSD_GDB
>> +	alias wc=gwc
>> +	alias sed="gsed"
>> +	alias sha256sum=gsha256sum
>> +	;;
>> +esac
>> +
>
> What about adding (most of) this to sh.config by the configure script?
> I'd like to centralize all hacky platform detection there.

configure already checks kernel name $(uname) and -s is default so
that info could be written to sh.config. (uname=$uname or something)

the alias thing cannot be done in sh.config as that is bash spesific...

... but, alias is not needed; sed () { gsed "$@"; } eg...

... but still, I'd not start messing sh.config with anything else
than variable settings... until there is need for such thing outside
test scripts... 

so far gwc would not be needed is $((`... | wc -l`))'s are used there.
have to check where gsha256sum is currently used...

..

Tomi


>
> Also, why is gsed in quotes but none of the others are?
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH] tests: add compatibility layer
  2016-12-20 22:24 ` David Bremner
  2016-12-21  5:12   ` Tomi Ollila
@ 2016-12-21 14:56   ` Mikhail
  2016-12-22  9:31   ` mp39590
  2 siblings, 0 replies; 16+ messages in thread
From: Mikhail @ 2016-12-21 14:56 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

David wrote 21.12.2016, 1:24:45:

> mp39590@gmail.com writes:

>>  
>> +# OS independent functions
>> +#
>> +# Alias native BSD utilities to usable GNU equivalents.
>> +case `uname` in
>> +FreeBSD)
>> +     # allow using aliases in scripts
>> +     shopt -s expand_aliases
>> +
>> +     alias date=gdate
>> +     alias base64=gbase64
>> +     alias gdb=$BSD_GDB
>> +     alias wc=gwc
>> +     alias sed="gsed"
>> +     alias sha256sum=gsha256sum
>> +     ;;
>> +esac
>> +

> What about adding (most of) this to sh.config by the configure script?
> I'd like to centralize all hacky platform detection there.

Do   you   mean   something   like  this?  -  Checking  $platform  and
conditionally  adding  shopt  and  aliases.  But one must be sure that
every script that source sh.config is a bash script.

diff --git a/configure b/configure
index 72db26df..ef6f657f 100755
--- a/configure
+++ b/configure
@@ -1205,6 +1205,23 @@ NOTMUCH_PYTHON=${python}
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
 EOF
 
+# Alias native BSD utilities to usable GNU equivalents.
+if [ $platform == FREEBSD ]; then
+       cat >> sh.config <<EOF
+
+# Allow using aliases in bash test scripts
+shopt -s expand_aliases
+
+# Substitute BSD utils with GNU ones (sysutils/coreutils required)
+alias date=gdate
+alias base64=gbase64
+alias gdb=\$BSD_GDB
+alias wc=gwc
+alias sed=gsed
+alias sha256sum=gsha256sum
+EOF
+fi
+
 # Finally, after everything configured, inform the user how to continue.
 cat <<EOF

> Also, why is gsed in quotes but none of the others are?
Fixed, thanks.

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

* [PATCH] tests: add compatibility layer
  2016-12-20 22:24 ` David Bremner
  2016-12-21  5:12   ` Tomi Ollila
  2016-12-21 14:56   ` Mikhail
@ 2016-12-22  9:31   ` mp39590
  2016-12-31 11:03     ` David Bremner
  2016-12-31 15:17     ` Tomi Ollila
  2 siblings, 2 replies; 16+ messages in thread
From: mp39590 @ 2016-12-22  9:31 UTC (permalink / raw)
  To: notmuch

From: Mikhail <mp39590@gmail.com>

Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
additional shim for platform specifics.

Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
---
 configure                | 3 +++
 test/README              | 6 ++++++
 test/test-lib-FREEBSD.sh | 8 ++++++++
 test/test-lib-common.sh  | 5 +++++
 4 files changed, 22 insertions(+)
 create mode 100644 test/test-lib-FREEBSD.sh

diff --git a/configure b/configure
index 72db26df..7ba9b9eb 100755
--- a/configure
+++ b/configure
@@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
 # Are the ruby development files (and ruby) available? If not skip
 # building/testing ruby bindings.
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
+
+# Platform we are run on
+PLATFORM=${platform}
 EOF
 
 # Finally, after everything configured, inform the user how to continue.
diff --git a/test/README b/test/README
index 104a120e..094e1d33 100644
--- a/test/README
+++ b/test/README
@@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
 
 e.g. env PATH=/opt/gnu/bin:$PATH make test
 
+For FreeBSD you will need to install coreutils, which provides GNU
+versions of basic utils like 'date' or 'wc'. Also you will need to
+install latest gdb from ports or packages and provide path to it in
+BSD_GDB variable before executing the tests, native FreeBSD gdb will not
+work.
+
 Running Tests
 -------------
 The easiest way to run tests is to say "make test", (or simply run the
diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
new file mode 100644
index 00000000..24079689
--- /dev/null
+++ b/test/test-lib-FREEBSD.sh
@@ -0,0 +1,8 @@
+# Use GNU Coreutils instead of a native BSD utils
+
+date () { gdate "$@"; }
+base64 () { gbase64 "$@"; }
+gdb () { $BSD_GDB "$@"; }
+wc () { gwc "$@"; }
+sed () { gsed "$@"; }
+sha256sum () { gsha256sum "$@"; }
diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
index 03ef1d2d..1c8d7f6e 100644
--- a/test/test-lib-common.sh
+++ b/test/test-lib-common.sh
@@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
 # configure output
 . $notmuch_path/sh.config || exit 1
 
+# load OS specifics
+if [ -e ./test-lib-$PLATFORM.sh ]; then
+	. ./test-lib-$PLATFORM.sh
+fi
+
 if test -n "$valgrind"
 then
 	make_symlink () {
-- 
2.11.0

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

* Re: [PATCH] tests: add compatibility layer
  2016-12-22  9:31   ` mp39590
@ 2016-12-31 11:03     ` David Bremner
  2017-01-02 13:57       ` mp39590
  2016-12-31 15:17     ` Tomi Ollila
  1 sibling, 1 reply; 16+ messages in thread
From: David Bremner @ 2016-12-31 11:03 UTC (permalink / raw)
  To: mp39590, notmuch

mp39590@gmail.com writes:

> From: Mikhail <mp39590@gmail.com>
>
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
>
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.

We had quite a bit of discussion of this on IRC. My (subjective) summary
is that

- we are generally fine with test-lib-${PLATFORM}.sh
- we prefer functions to aliases (fixed in the latest version)
- some people are uncomfortable with mandating gnu coreutils to run the
  tests on FreeBSD

With respect to the last point, I agree it's not ideal, but it is better
than not running them at all. We can incrementally remove the dependence
on coreutils later if someone is motivated. This solution has the
advantage of being easy, and not imposing a burden on people not using
that platform.

The only thing that still bugs me about this is the variable
BSD_GDB. I'm a bit surprised that the same approach used for coreutils
(i.e. rely on PATH) does not work. I guess because the names are the
same it's less reliable for gdb.  If we do a seperate variable, I'd
prefer to use NOTMUCH_GDB (in line with NOTMUCH_PYTHON) since BSD_GDB
suggests the native one.  This could (eventually) be computed by
configure, but this need not block this patch.

Happy New-Year-In-Some-Timezones,

David

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

* Re: [PATCH] tests: add compatibility layer
  2016-12-22  9:31   ` mp39590
  2016-12-31 11:03     ` David Bremner
@ 2016-12-31 15:17     ` Tomi Ollila
  1 sibling, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2016-12-31 15:17 UTC (permalink / raw)
  To: mp39590, notmuch

On Thu, Dec 22 2016, mp39590@gmail.com wrote:

> From: Mikhail <mp39590@gmail.com>
>
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
>
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.


I've been slow to respond, mostly busy, partly checking some freebsd
facts...

The patch is mostly good, but there are a few issues i am going to
mention a bit later -- now I just have few minutes...

IIRC you're a freebsd port maintainer and would like to have more
certainty that every new test patch doesn't break your test.
In general we desire more portability overall e.g. things work on
linux, *bsd, macos, solaris...

But, for now I'd like you to test the following script which
I named `make-test-freesd.sh` (well, that could also work on many other
OSses) and tell why it would not work ;) ... in that there is one hint
how the patch should be improved :D

run it instead of `gmake test` in notmuch source directory.


Tomi



--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--
#!/usr/bin/env bash

set -euf -o posix

SHELL=`command -v bash`
export SHELL

mkwrp () {
	command -v "$2" >/dev/null || return 0
	eval "$1 () { $2 \"\$@\"; }"
	export -f $1
}

mkwrp date gdate
mkwrp base64 gbase64
mkwrp gdb "$NOTMUCH_GDB"
mkwrp wc gwc
mkwrp sha256sum gsha256sum

set -x
exec gmake test
--8<----8<----8<----8<----8<----8<----8<----8<----8<--


> ---
>  configure                | 3 +++
>  test/README              | 6 ++++++
>  test/test-lib-FREEBSD.sh | 8 ++++++++
>  test/test-lib-common.sh  | 5 +++++
>  4 files changed, 22 insertions(+)
>  create mode 100644 test/test-lib-FREEBSD.sh
>
> diff --git a/configure b/configure
> index 72db26df..7ba9b9eb 100755
> --- a/configure
> +++ b/configure
> @@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
>  # Are the ruby development files (and ruby) available? If not skip
>  # building/testing ruby bindings.
>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
> +
> +# Platform we are run on
> +PLATFORM=${platform}
>  EOF
>  
>  # Finally, after everything configured, inform the user how to continue.
> diff --git a/test/README b/test/README
> index 104a120e..094e1d33 100644
> --- a/test/README
> +++ b/test/README
> @@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
>  
>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>  
> +For FreeBSD you will need to install coreutils, which provides GNU
> +versions of basic utils like 'date' or 'wc'. Also you will need to
> +install latest gdb from ports or packages and provide path to it in
> +BSD_GDB variable before executing the tests, native FreeBSD gdb will not
> +work.
> +
>  Running Tests
>  -------------
>  The easiest way to run tests is to say "make test", (or simply run the
> diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
> new file mode 100644
> index 00000000..24079689
> --- /dev/null
> +++ b/test/test-lib-FREEBSD.sh
> @@ -0,0 +1,8 @@
> +# Use GNU Coreutils instead of a native BSD utils
> +
> +date () { gdate "$@"; }
> +base64 () { gbase64 "$@"; }
> +gdb () { $BSD_GDB "$@"; }
> +wc () { gwc "$@"; }
> +sed () { gsed "$@"; }
> +sha256sum () { gsha256sum "$@"; }
> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
> index 03ef1d2d..1c8d7f6e 100644
> --- a/test/test-lib-common.sh
> +++ b/test/test-lib-common.sh
> @@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
>  # configure output
>  . $notmuch_path/sh.config || exit 1
>  
> +# load OS specifics
> +if [ -e ./test-lib-$PLATFORM.sh ]; then
> +	. ./test-lib-$PLATFORM.sh
> +fi
> +
>  if test -n "$valgrind"
>  then
>  	make_symlink () {
> -- 
> 2.11.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

* [PATCH] tests: add compatibility layer
  2016-12-31 11:03     ` David Bremner
@ 2017-01-02 13:57       ` mp39590
  2017-01-03 15:48         ` Tomi Ollila
  0 siblings, 1 reply; 16+ messages in thread
From: mp39590 @ 2017-01-02 13:57 UTC (permalink / raw)
  To: notmuch

From: Mikhail <mp39590@gmail.com>

Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
additional shim for platform specifics.

Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
---
 configure                | 3 +++
 test/README              | 6 ++++++
 test/test-lib-FREEBSD.sh | 8 ++++++++
 test/test-lib-common.sh  | 5 +++++
 4 files changed, 22 insertions(+)
 create mode 100644 test/test-lib-FREEBSD.sh

diff --git a/configure b/configure
index 72db26df..7ba9b9eb 100755
--- a/configure
+++ b/configure
@@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
 # Are the ruby development files (and ruby) available? If not skip
 # building/testing ruby bindings.
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
+
+# Platform we are run on
+PLATFORM=${platform}
 EOF
 
 # Finally, after everything configured, inform the user how to continue.
diff --git a/test/README b/test/README
index 104a120e..8376616f 100644
--- a/test/README
+++ b/test/README
@@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
 
 e.g. env PATH=/opt/gnu/bin:$PATH make test
 
+For FreeBSD you will need to install coreutils, which provides GNU
+versions of basic utils like 'date' or 'wc'. Also you will need to
+install latest gdb from ports or packages and provide path to it in
+NOTMUCH_GDB variable before executing the tests, native FreeBSD gdb will
+not work.
+
 Running Tests
 -------------
 The easiest way to run tests is to say "make test", (or simply run the
diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
new file mode 100644
index 00000000..b8039705
--- /dev/null
+++ b/test/test-lib-FREEBSD.sh
@@ -0,0 +1,8 @@
+# Use GNU Coreutils instead of a native BSD utils
+
+date () { gdate "$@"; }
+base64 () { gbase64 "$@"; }
+gdb () { $NOTMUCH_GDB "$@"; }
+wc () { gwc "$@"; }
+sed () { gsed "$@"; }
+sha256sum () { gsha256sum "$@"; }
diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
index 03ef1d2d..1c8d7f6e 100644
--- a/test/test-lib-common.sh
+++ b/test/test-lib-common.sh
@@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
 # configure output
 . $notmuch_path/sh.config || exit 1
 
+# load OS specifics
+if [ -e ./test-lib-$PLATFORM.sh ]; then
+	. ./test-lib-$PLATFORM.sh
+fi
+
 if test -n "$valgrind"
 then
 	make_symlink () {
-- 
2.11.0

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

* Re: [PATCH] tests: add compatibility layer
  2017-01-02 13:57       ` mp39590
@ 2017-01-03 15:48         ` Tomi Ollila
  2017-01-03 15:52           ` Tomi Ollila
                             ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Tomi Ollila @ 2017-01-03 15:48 UTC (permalink / raw)
  To: mp39590, notmuch

On Mon, Jan 02 2017, mp39590@gmail.com wrote:

> From: Mikhail <mp39590@gmail.com>
>
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
>
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.

Ok, now I've git a bit of time to read and respond; If I'd read earlier
and not had time to respone, I'd have thought this issue too much in
between... so let's quickly process this :D

> ---
>  configure                | 3 +++
>  test/README              | 6 ++++++
>  test/test-lib-FREEBSD.sh | 8 ++++++++
>  test/test-lib-common.sh  | 5 +++++
>  4 files changed, 22 insertions(+)
>  create mode 100644 test/test-lib-FREEBSD.sh
>
> diff --git a/configure b/configure
> index 72db26df..7ba9b9eb 100755
> --- a/configure
> +++ b/configure
> @@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
>  # Are the ruby development files (and ruby) available? If not skip
>  # building/testing ruby bindings.
>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
> +
> +# Platform we are run on
> +PLATFORM=${platform}

This change looks good.

>  EOF
>  
>  # Finally, after everything configured, inform the user how to continue.
> diff --git a/test/README b/test/README
> index 104a120e..8376616f 100644
> --- a/test/README
> +++ b/test/README
> @@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
>  
>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>  
> +For FreeBSD you will need to install coreutils, which provides GNU
> +versions of basic utils like 'date' or 'wc'. Also you will need to
> +install latest gdb from ports or packages and provide path to it in
> +NOTMUCH_GDB variable before executing the tests, native FreeBSD gdb will
> +not work.

I think this is a bit too strong statement; one should not *need* to
install such a bloaty beast as a GNU coreutils to use instead of nice
native utils system is shipped with; IMO this should mention that
installing coreutils (in a way that these g-prefixed commands appear
in PATH!) may improve the success of test run... or something. 

Also, IMHO, native FreeBSD gdb does not work is much nicer than stating
it will not work -- but that may just be my feeling of how it sounds...

...

> +
>  Running Tests
>  -------------
>  The easiest way to run tests is to say "make test", (or simply run the
> diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
> new file mode 100644
> index 00000000..b8039705
> --- /dev/null
> +++ b/test/test-lib-FREEBSD.sh
> @@ -0,0 +1,8 @@
> +# Use GNU Coreutils instead of a native BSD utils
> +
> +date () { gdate "$@"; }
> +base64 () { gbase64 "$@"; }
> +gdb () { $NOTMUCH_GDB "$@"; }
> +wc () { gwc "$@"; }
> +sed () { gsed "$@"; }
> +sha256sum () { gsha256sum "$@"; }

The above is currently problematic when one does not install GNU coreutils;
tests will fail more when e.g. `gwc` does not exist but `wc` does. When I
tested with this patch a few days ago, test failure count increased from
~70 to ~140. That was the quickly written mkwrp() for...

I use my freebsd KVM virtual machine for e.g. portability testing; things
that work on freebsd (in addition to linux) have more chance to work on
netbsd, openbsd, macos, solaris, openindiana. With the above applied
verbatim I would always need to remember to dump the "compatibility file"
before running tests...

To fix the above, one could use (possibly better written) mkwrap()
implementation, or do all of those by hand: e.g.

if command -v gdate >/dev/null; then date () { gdate "$@"; }; fi
if command -v gbase64 >/dev/null; then base64 () { gbase64 "$@"; }; fi
...

the gdb wrapper could be dumped completely and have the following in
test-lib-common.sh:

: ${NOTMUCH_GDB:-gdb}

and then replace all (the few) gdb in code with $NOTMUCH_GDB (did not use
quotes like "$NOTMUCH_GDB" so that one can hack args there.

but if the gdb function were there, then the following might work:

gdb () { command ${NOTMUCH_GDB:-gdb} "$@"; }


> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
> index 03ef1d2d..1c8d7f6e 100644
> --- a/test/test-lib-common.sh
> +++ b/test/test-lib-common.sh
> @@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
>  # configure output
>  . $notmuch_path/sh.config || exit 1
>  
> +# load OS specifics
> +if [ -e ./test-lib-$PLATFORM.sh ]; then
> +	. ./test-lib-$PLATFORM.sh
> +fi
> +

This change looks good.


Tomi

>  if test -n "$valgrind"
>  then
>  	make_symlink () {
> -- 

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

* Re: [PATCH] tests: add compatibility layer
  2017-01-03 15:48         ` Tomi Ollila
@ 2017-01-03 15:52           ` Tomi Ollila
  2017-01-03 17:21           ` Tomi Ollila
  2017-01-04  1:57           ` Mikhail
  2 siblings, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2017-01-03 15:52 UTC (permalink / raw)
  To: mp39590, notmuch

On Tue, Jan 03 2017, Tomi Ollila <tomi.ollila@iki.fi> wrote:

>
> To fix the above, one could use (possibly better written) mkwrap()
> implementation, or do all of those by hand: e.g.
>
> if command -v gdate >/dev/null; then date () { gdate "$@"; }; fi
> if command -v gbase64 >/dev/null; then base64 () { gbase64 "$@"; }; fi
> ...
>
> the gdb wrapper could be dumped completely and have the following in
> test-lib-common.sh:
>
> : ${NOTMUCH_GDB:-gdb}

Argh, the above would not work, but this: 

: ${NOTMUCH_GDB:=gdb}

emphasis++ ftw \o/ ! ;)

Tomi

>
> and then replace all (the few) gdb in code with $NOTMUCH_GDB (did not use
> quotes like "$NOTMUCH_GDB" so that one can hack args there.
>
> but if the gdb function were there, then the following might work:
>
> gdb () { command ${NOTMUCH_GDB:-gdb} "$@"; }
>

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

* Re: [PATCH] tests: add compatibility layer
  2017-01-03 15:48         ` Tomi Ollila
  2017-01-03 15:52           ` Tomi Ollila
@ 2017-01-03 17:21           ` Tomi Ollila
  2017-01-04  1:57           ` Mikhail
  2 siblings, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2017-01-03 17:21 UTC (permalink / raw)
  To: mp39590, notmuch

On Tue, Jan 03 2017, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Mon, Jan 02 2017, mp39590@gmail.com wrote:
>
>> From: Mikhail <mp39590@gmail.com>
>>
>> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
>> additional shim for platform specifics.
>>
>> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
>
> Ok, now I've git a bit of time to read and respond; If I'd read earlier
> and not had time to respone, I'd have thought this issue too much in
> between... so let's quickly process this :D
>
>> ---
>>  configure                | 3 +++
>>  test/README              | 6 ++++++
>>  test/test-lib-FREEBSD.sh | 8 ++++++++
>>  test/test-lib-common.sh  | 5 +++++
>>  4 files changed, 22 insertions(+)
>>  create mode 100644 test/test-lib-FREEBSD.sh
>>
>> diff --git a/configure b/configure
>> index 72db26df..7ba9b9eb 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
>>  # Are the ruby development files (and ruby) available? If not skip
>>  # building/testing ruby bindings.
>>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
>> +
>> +# Platform we are run on
>> +PLATFORM=${platform}
>
> This change looks good.
>
>>  EOF
>>  
>>  # Finally, after everything configured, inform the user how to continue.
>> diff --git a/test/README b/test/README
>> index 104a120e..8376616f 100644
>> --- a/test/README
>> +++ b/test/README
>> @@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
>>  
>>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>>  
>> +For FreeBSD you will need to install coreutils, which provides GNU
>> +versions of basic utils like 'date' or 'wc'. Also you will need to
>> +install latest gdb from ports or packages and provide path to it in
>> +NOTMUCH_GDB variable before executing the tests, native FreeBSD gdb will
>> +not work.
>
> I think this is a bit too strong statement; one should not *need* to
> install such a bloaty beast as a GNU coreutils to use instead of nice
> native utils system is shipped with; IMO this should mention that
> installing coreutils (in a way that these g-prefixed commands appear
> in PATH!) may improve the success of test run... or something. 
>
> Also, IMHO, native FreeBSD gdb does not work is much nicer than stating
> it will not work -- but that may just be my feeling of how it sounds...


What I mean that the IMO the tone should more like suggestive than
impeartive -- it looks more like users have a choice...

>
> ...
>
>> +
>>  Running Tests
>>  -------------
>>  The easiest way to run tests is to say "make test", (or simply run the
>> diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
>> new file mode 100644
>> index 00000000..b8039705
>> --- /dev/null
>> +++ b/test/test-lib-FREEBSD.sh
>> @@ -0,0 +1,8 @@
>> +# Use GNU Coreutils instead of a native BSD utils
>> +
>> +date () { gdate "$@"; }
>> +base64 () { gbase64 "$@"; }
>> +gdb () { $NOTMUCH_GDB "$@"; }
>> +wc () { gwc "$@"; }
>> +sed () { gsed "$@"; }
>> +sha256sum () { gsha256sum "$@"; }
>
> The above is currently problematic when one does not install GNU coreutils;
> tests will fail more when e.g. `gwc` does not exist but `wc` does. When I
> tested with this patch a few days ago, test failure count increased from
> ~70 to ~140. That was the quickly written mkwrp() for...
>
> I use my freebsd KVM virtual machine for e.g. portability testing; things
> that work on freebsd (in addition to linux) have more chance to work on
> netbsd, openbsd, macos, solaris, openindiana. With the above applied
> verbatim I would always need to remember to dump the "compatibility file"
> before running tests...
>
> To fix the above, one could use (possibly better written) mkwrap()
> implementation, or do all of those by hand: e.g.
>
> if command -v gdate >/dev/null; then date () { gdate "$@"; }; fi
> if command -v gbase64 >/dev/null; then base64 () { gbase64 "$@"; }; fi
> ...
>
> the gdb wrapper could be dumped completely and have the following in
> test-lib-common.sh:
>
> : ${NOTMUCH_GDB:=gdb}
>

Sneaky fix ;)

> and then replace all (the few) gdb in code with $NOTMUCH_GDB (did not use
> quotes like "$NOTMUCH_GDB" so that one can hack args there.
>
> but if the gdb function were there, then the following might work:
>
> gdb () { command ${NOTMUCH_GDB:-gdb} "$@"; }
>
>
>> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
>> index 03ef1d2d..1c8d7f6e 100644
>> --- a/test/test-lib-common.sh
>> +++ b/test/test-lib-common.sh
>> @@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
>>  # configure output
>>  . $notmuch_path/sh.config || exit 1
>>  
>> +# load OS specifics
>> +if [ -e ./test-lib-$PLATFORM.sh ]; then
>> +	. ./test-lib-$PLATFORM.sh
>> +fi
>> +
>
> This change looks good.

actually, the second line, to be consistent:

+	. ./test-lib-$PLATFORM.sh || exit 1

>
> Tomi
>



>>  if test -n "$valgrind"
>>  then
>>  	make_symlink () {
>> -- 

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

* Re: [PATCH] tests: add compatibility layer
  2017-01-03 15:48         ` Tomi Ollila
  2017-01-03 15:52           ` Tomi Ollila
  2017-01-03 17:21           ` Tomi Ollila
@ 2017-01-04  1:57           ` Mikhail
  2 siblings, 0 replies; 16+ messages in thread
From: Mikhail @ 2017-01-04  1:57 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On 18:48 03-Jan 2017 Tomi Ollila wrote:
>>
>>  # Finally, after everything configured, inform the user how to continue.
>> diff --git a/test/README b/test/README
>> index 104a120e..8376616f 100644
>> --- a/test/README
>> +++ b/test/README
>> @@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
>>
>>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>>
>> +For FreeBSD you will need to install coreutils, which provides GNU
>> +versions of basic utils like 'date' or 'wc'. Also you will need to
>> +install latest gdb from ports or packages and provide path to it in
>> +NOTMUCH_GDB variable before executing the tests, native FreeBSD gdb will
>> +not work.
>
>I think this is a bit too strong statement; one should not *need* to
>install such a bloaty beast as a GNU coreutils to use instead of nice
>native utils system is shipped with; IMO this should mention that
>installing coreutils (in a way that these g-prefixed commands appear
>in PATH!) may improve the success of test run... or something.

GNU utils is what developers use and what majority of users are using.
Tests, in my view, have been created to test functionality of notmuch,
and if user want to be sure that notmuch is working as expected - he
should use the tools developers were using. There is not much difference
anyway - mostly spacing in 'wc' or column number in 'base64', or '-i'
handling in 'sed', or 'date' flags.

Improving success rate is not a goal, goal - 100% test passing, as on
Linux. To get closer to 100% - install coreutils, if it isn't 100% -
there is a problem - it has to be investigated.

If you don't want 100% passage - there is a difference in our goals and
we are talking about different things.

>> +date () { gdate "$@"; }
>> +base64 () { gbase64 "$@"; }
>> +gdb () { $NOTMUCH_GDB "$@"; }
>> +wc () { gwc "$@"; }
>> +sed () { gsed "$@"; }
>> +sha256sum () { gsha256sum "$@"; }
>
>The above is currently problematic when one does not install GNU coreutils;
>tests will fail more when e.g. `gwc` does not exist but `wc` does. When I
>tested with this patch a few days ago, test failure count increased from
>~70 to ~140. That was the quickly written mkwrp() for...

>I use my freebsd KVM virtual machine for e.g. portability testing; things
>that work on freebsd (in addition to linux) have more chance to work on
>netbsd, openbsd, macos, solaris, openindiana. With the above applied
>verbatim I would always need to remember to dump the "compatibility file"
>before running tests...
>
>To fix the above, one could use (possibly better written) mkwrap()
>implementation, or do all of those by hand: e.g.
>
>if command -v gdate >/dev/null; then date () { gdate "$@"; }; fi
>if command -v gbase64 >/dev/null; then base64 () { gbase64 "$@"; }; fi
>...
>
>the gdb wrapper could be dumped completely and have the following in
>test-lib-common.sh:
>
>: ${NOTMUCH_GDB:-gdb}
>
>and then replace all (the few) gdb in code with $NOTMUCH_GDB (did not use
>quotes like "$NOTMUCH_GDB" so that one can hack args there.
>
>but if the gdb function were there, then the following might work:
>
>gdb () { command ${NOTMUCH_GDB:-gdb} "$@"; }

My only concern is a simplicity, if installing coreutils and "alias'ing"
native 'wc' to 'gwc' solves the problem - why to create more entities?
In real world, if you have gwc - you have gbase64, I can't imagine any
exceptions.

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

* [PATCH] tests: add compatibility layer
@ 2017-03-09 13:32 David Bremner
  2017-03-09 15:47 ` Tomi Ollila
  2017-03-24 20:33 ` Mikhail
  0 siblings, 2 replies; 16+ messages in thread
From: David Bremner @ 2017-03-09 13:32 UTC (permalink / raw)
  To: notmuch; +Cc: Mikhail

From: Mikhail <mp39590@gmail.com>

Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
additional shim for platform specifics.

Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.

- amended by db following Tomi's suggestions
---

I haven't tested this, except to verify it doesn't crash under GNU/Linux

 configure                |  3 +++
 test/README              | 11 +++++++++++
 test/test-lib-FREEBSD.sh |  9 +++++++++
 test/test-lib-common.sh  |  5 +++++
 4 files changed, 28 insertions(+)
 create mode 100644 test/test-lib-FREEBSD.sh

diff --git a/configure b/configure
index fa77eb8f..eb452a12 100755
--- a/configure
+++ b/configure
@@ -1186,6 +1186,9 @@ NOTMUCH_PYTHON=${python}
 # Are the ruby development files (and ruby) available? If not skip
 # building/testing ruby bindings.
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
+
+# Platform we are run on
+PLATFORM=${platform}
 EOF
 
 # Finally, after everything configured, inform the user how to continue.
diff --git a/test/README b/test/README
index 104a120e..ae22d6e0 100644
--- a/test/README
+++ b/test/README
@@ -33,6 +33,17 @@ chosen directory to your PATH before running the tests.
 
 e.g. env PATH=/opt/gnu/bin:$PATH make test
 
+For FreeBSD you need to install latest gdb from ports or packages and
+provide path to it in TEST_GDB environment variable before executing
+the tests, native FreeBSD gdb does not not work.  If you install
+coreutils, which provides GNU versions of basic utils like 'date' and
+'base64' on FreeBSD, the test suite will use these instead of the
+native ones. This provides robustness against portability issues with
+these system tools. Most often the tests are written, reviewed and
+tested on Linux system so such portability issues arise from time to
+time.
+
+
 Running Tests
 -------------
 The easiest way to run tests is to say "make test", (or simply run the
diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
new file mode 100644
index 00000000..d1840b56
--- /dev/null
+++ b/test/test-lib-FREEBSD.sh
@@ -0,0 +1,9 @@
+# If present, use GNU Coreutils instead of a native BSD utils
+if command -v gdate >/dev/null
+   then
+       date () { gdate "$@"; }
+       base64 () { gbase64 "$@"; }
+       wc () { gwc "$@"; }
+       sed () { gsed "$@"; }
+       sha256sum () { gsha256sum "$@"; }
+   fi
diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
index a96cfbeb..ef409171 100644
--- a/test/test-lib-common.sh
+++ b/test/test-lib-common.sh
@@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
 # configure output
 . $notmuch_path/sh.config || exit 1
 
+# load OS specifics
+if [ -e ./test-lib-$PLATFORM.sh ]; then
+	. ./test-lib-$PLATFORM.sh || exit 1
+fi
+
 if test -n "$valgrind"
 then
 	make_symlink () {
-- 
2.11.0

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

* Re: [PATCH] tests: add compatibility layer
  2017-03-09 13:32 David Bremner
@ 2017-03-09 15:47 ` Tomi Ollila
  2017-03-24 20:33 ` Mikhail
  1 sibling, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2017-03-09 15:47 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: Mikhail

On Thu, Mar 09 2017, David Bremner <david@tethera.net> wrote:

> From: Mikhail <mp39590@gmail.com>
>
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
>
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
>
> - amended by db following Tomi's suggestions
> ---
>
> I haven't tested this, except to verify it doesn't crash under GNU/Linux

I cannot test as I will not pollute my purish Frisbee KVM image with
coreutils package -- I could play with second image but I', too lazy
to do so.

Anyway, it looks ok to me.

Tomi


>
>  configure                |  3 +++
>  test/README              | 11 +++++++++++
>  test/test-lib-FREEBSD.sh |  9 +++++++++
>  test/test-lib-common.sh  |  5 +++++
>  4 files changed, 28 insertions(+)
>  create mode 100644 test/test-lib-FREEBSD.sh
>
> diff --git a/configure b/configure
> index fa77eb8f..eb452a12 100755
> --- a/configure
> +++ b/configure
> @@ -1186,6 +1186,9 @@ NOTMUCH_PYTHON=${python}
>  # Are the ruby development files (and ruby) available? If not skip
>  # building/testing ruby bindings.
>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
> +
> +# Platform we are run on
> +PLATFORM=${platform}
>  EOF
>  
>  # Finally, after everything configured, inform the user how to continue.
> diff --git a/test/README b/test/README
> index 104a120e..ae22d6e0 100644
> --- a/test/README
> +++ b/test/README
> @@ -33,6 +33,17 @@ chosen directory to your PATH before running the tests.
>  
>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>  
> +For FreeBSD you need to install latest gdb from ports or packages and
> +provide path to it in TEST_GDB environment variable before executing
> +the tests, native FreeBSD gdb does not not work.  If you install
> +coreutils, which provides GNU versions of basic utils like 'date' and
> +'base64' on FreeBSD, the test suite will use these instead of the
> +native ones. This provides robustness against portability issues with
> +these system tools. Most often the tests are written, reviewed and
> +tested on Linux system so such portability issues arise from time to
> +time.
> +
> +
>  Running Tests
>  -------------
>  The easiest way to run tests is to say "make test", (or simply run the
> diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
> new file mode 100644
> index 00000000..d1840b56
> --- /dev/null
> +++ b/test/test-lib-FREEBSD.sh
> @@ -0,0 +1,9 @@
> +# If present, use GNU Coreutils instead of a native BSD utils
> +if command -v gdate >/dev/null
> +   then
> +       date () { gdate "$@"; }
> +       base64 () { gbase64 "$@"; }
> +       wc () { gwc "$@"; }
> +       sed () { gsed "$@"; }
> +       sha256sum () { gsha256sum "$@"; }
> +   fi
> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
> index a96cfbeb..ef409171 100644
> --- a/test/test-lib-common.sh
> +++ b/test/test-lib-common.sh
> @@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
>  # configure output
>  . $notmuch_path/sh.config || exit 1
>  
> +# load OS specifics
> +if [ -e ./test-lib-$PLATFORM.sh ]; then
> +	. ./test-lib-$PLATFORM.sh || exit 1
> +fi
> +
>  if test -n "$valgrind"
>  then
>  	make_symlink () {
> -- 
> 2.11.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH] tests: add compatibility layer
  2017-03-09 13:32 David Bremner
  2017-03-09 15:47 ` Tomi Ollila
@ 2017-03-24 20:33 ` Mikhail
  2017-03-25 11:10   ` David Bremner
  1 sibling, 1 reply; 16+ messages in thread
From: Mikhail @ 2017-03-24 20:33 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On 16:32 09-Mar 2017 David Bremner wrote:
> From: Mikhail <mp39590@gmail.com>
> 
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
> 
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
> 
> - amended by db following Tomi's suggestions

I've tested the patch on FreeBSD and it works fine.

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

* Re: [PATCH] tests: add compatibility layer
  2017-03-24 20:33 ` Mikhail
@ 2017-03-25 11:10   ` David Bremner
  0 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2017-03-25 11:10 UTC (permalink / raw)
  To: Mikhail; +Cc: notmuch

Mikhail <mp39590@gmail.com> writes:

> On 16:32 09-Mar 2017 David Bremner wrote:
>> From: Mikhail <mp39590@gmail.com>
>> 
>> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
>> additional shim for platform specifics.
>> 
>> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.
>> 
>> - amended by db following Tomi's suggestions
>
> I've tested the patch on FreeBSD and it works fine.

Thanks for all your work on this, I've pushed the patch to master.  By
the way, I'm not sure if it's also an issue for FreeBSD, but we've
renamed libutil.a to libnotmuch_util.a to avoid a name conflict on some
BSD-ish systems.

d

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

end of thread, other threads:[~2017-03-25 11:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-20 19:47 [PATCH] tests: add compatibility layer mp39590
2016-12-20 22:24 ` David Bremner
2016-12-21  5:12   ` Tomi Ollila
2016-12-21 14:56   ` Mikhail
2016-12-22  9:31   ` mp39590
2016-12-31 11:03     ` David Bremner
2017-01-02 13:57       ` mp39590
2017-01-03 15:48         ` Tomi Ollila
2017-01-03 15:52           ` Tomi Ollila
2017-01-03 17:21           ` Tomi Ollila
2017-01-04  1:57           ` Mikhail
2016-12-31 15:17     ` Tomi Ollila
  -- strict thread matches above, loose matches on Subject: below --
2017-03-09 13:32 David Bremner
2017-03-09 15:47 ` Tomi Ollila
2017-03-24 20:33 ` Mikhail
2017-03-25 11:10   ` David Bremner

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

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

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