unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] test-databases: use wget or curl to download test databases
@ 2017-03-12 12:59 Tomi Ollila
  2017-03-12 18:31 ` [PATCH] test: verify test database v1 checksum in more portable way Tomi Ollila
  2017-03-14 15:39 ` [PATCH] test-databases: use wget or curl to download test databases David Bremner
  0 siblings, 2 replies; 5+ messages in thread
From: Tomi Ollila @ 2017-03-12 12:59 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Often Linux systems are shipped with wget(1) by default (and no curl).

Many BSDs, macOS, and e.g. some Linux minimal/container images
comes with curl(1) (and no wget).

Attempting to download with curl if wget is not available increases
the likelihood for this to succeed.
---

This is an update to id:1395846591-3490-1-git-send-email-tomi.ollila@iki.fi
(~1080 days). Changed 'hash' to 'command -v' and added '-L' to curl options
(just to be sure) -- and added more detail to the commit message -- Latest
observation was that centos 7 (docker) container image did not have wget(1)
but curl(1) there were.

David asked whether this a job for configure. For the time being I think this
is simplest to use -- any alternative adds unnecessary cohesion between the
tool/configuration and this make. If, in the future, we will add more online
capabilities to this (test) system, then, perhaps we'd better to make some
script (as command) to handle that; now 3 years has passed since my previous
version -- and that is good as we'd like to be as offline as possible there.

 test/test-databases/Makefile.local | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/test-databases/Makefile.local b/test/test-databases/Makefile.local
index dcc8863c70d8..7aedff70f6e3 100644
--- a/test/test-databases/Makefile.local
+++ b/test/test-databases/Makefile.local
@@ -7,7 +7,13 @@ dir := test/test-databases
 test_databases := $(dir)/database-v1.tar.xz
 
 %.tar.xz:
-	wget -nv -O $@ ${TEST_DATABASE_MIRROR}/$(notdir $@);
+	@exec 1>&2 ;\
+	if command -v wget >/dev/null ;\
+	then set -x; wget -nv -O $@ ${TEST_DATABASE_MIRROR}/$(notdir $@) ;\
+	elif command -v curl >/dev/null ;\
+	then set -x; curl -L -s -o $@ ${TEST_DATABASE_MIRROR}/$(notdir $@) ;\
+	else echo Cannot fetch databases, no wget nor curl available; exit 1 ;\
+	fi
 
 download-test-databases: ${test_databases}
 
-- 
2.11.0

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

* [PATCH] test: verify test database v1 checksum in more portable way
  2017-03-12 12:59 [PATCH] test-databases: use wget or curl to download test databases Tomi Ollila
@ 2017-03-12 18:31 ` Tomi Ollila
  2017-03-12 22:33   ` David Bremner
  2017-03-14 15:39 ` [PATCH] test-databases: use wget or curl to download test databases David Bremner
  1 sibling, 1 reply; 5+ messages in thread
From: Tomi Ollila @ 2017-03-12 18:31 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Replaced use of sha256 (gnu coreutils binary) with more portable
openssl sha256 execution.
---

Works on Linux and also on my FreeBSD KVM environment.

 test/T530-upgrade.sh                          | 6 ++++--
 test/test-databases/database-v1.tar.xz.sha256 | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)
 delete mode 100644 test/test-databases/database-v1.tar.xz.sha256

diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
index f0fd151..bda3f98 100755
--- a/test/T530-upgrade.sh
+++ b/test/T530-upgrade.sh
@@ -12,8 +12,10 @@ fi
 
 test_begin_subtest "database checksum"
 test_expect_success \
-    '( cd $TEST_DIRECTORY/test-databases &&
-       sha256sum --quiet --check --status ${dbtarball}.sha256 )'
+    'case `openssl sha256 $TEST_DIRECTORY/test-databases/${dbtarball}` in
+	*4299e051b10e1fa7b33ea2862790a09ebfe96859681804e5251e130f800e69d2*) ;;
+	*) false ;;
+     esac'
 
 tar xf $TEST_DIRECTORY/test-databases/${dbtarball} -C ${MAIL_DIR} --strip-components=1
 
diff --git a/test/test-databases/database-v1.tar.xz.sha256 b/test/test-databases/database-v1.tar.xz.sha256
deleted file mode 100644
index 2cc4f96..0000000
--- a/test/test-databases/database-v1.tar.xz.sha256
+++ /dev/null
@@ -1 +0,0 @@
-4299e051b10e1fa7b33ea2862790a09ebfe96859681804e5251e130f800e69d2  database-v1.tar.xz
-- 
2.9.3

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

* Re: [PATCH] test: verify test database v1 checksum in more portable way
  2017-03-12 18:31 ` [PATCH] test: verify test database v1 checksum in more portable way Tomi Ollila
@ 2017-03-12 22:33   ` David Bremner
  2017-03-13 19:32     ` Tomi Ollila
  0 siblings, 1 reply; 5+ messages in thread
From: David Bremner @ 2017-03-12 22:33 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

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

> Replaced use of sha256 (gnu coreutils binary) with more portable
> openssl sha256 execution.
> ---
>
> Works on Linux and also on my FreeBSD KVM environment.

There's a tradeoff here. In a minimal GNU/Linux environment coreutils is
there but not openssl. So I don't mind the substitution (for the test
suite), but I think you need to add a
"test_require_external prereq openssl" or equivalent to T530-upgrade.sh

This will also require adding openssl as a debian build dependency; I
can also live with that, although it might inconvenience some people
building the debian-snapshot target.

There's something aesthetically displeasing about hardcoding the
checksum into the test script but I think I'm just grumbling at this
point.

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

* Re: [PATCH] test: verify test database v1 checksum in more portable way
  2017-03-12 22:33   ` David Bremner
@ 2017-03-13 19:32     ` Tomi Ollila
  0 siblings, 0 replies; 5+ messages in thread
From: Tomi Ollila @ 2017-03-13 19:32 UTC (permalink / raw)
  To: David Bremner, notmuch

On Mon, Mar 13 2017, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> Replaced use of sha256 (gnu coreutils binary) with more portable
>> openssl sha256 execution.
>> ---
>>
>> Works on Linux and also on my FreeBSD KVM environment.
>
> There's a tradeoff here. In a minimal GNU/Linux environment coreutils is
> there but not openssl. So I don't mind the substitution (for the test

Köh... That did not come into my mind, but perhaps that is the fun of it.

I'd rather let this test fail on non-coreutils system (for the time being)
than introduce more dependencies ;) -- have to think whether proposing
alternative solution...

And had to test that one:

  localhost$ sudo docker run --rm -it debian:8.7 /bin/bash 
  root@1c96aac39246:/# openssl
  bash: openssl: command not found
  root@1c96aac39246:/# sha1sum < /dev/null
  da39a3ee5e6b4b0d3255bfef95601890afd80709  -
  root@1c96aac39246:/# exit
  exit
  localhost$

Marking this particular patch obsolete.

Tomi


> suite), but I think you need to add a
> "test_require_external prereq openssl" or equivalent to T530-upgrade.sh
>
> This will also require adding openssl as a debian build dependency; I
> can also live with that, although it might inconvenience some people
> building the debian-snapshot target.
>
> There's something aesthetically displeasing about hardcoding the
> checksum into the test script but I think I'm just grumbling at this
> point.

I personally disagree (in specific cases like this), but would not have
done that if there were simpler solution using the file...

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

* Re: [PATCH] test-databases: use wget or curl to download test databases
  2017-03-12 12:59 [PATCH] test-databases: use wget or curl to download test databases Tomi Ollila
  2017-03-12 18:31 ` [PATCH] test: verify test database v1 checksum in more portable way Tomi Ollila
@ 2017-03-14 15:39 ` David Bremner
  1 sibling, 0 replies; 5+ messages in thread
From: David Bremner @ 2017-03-14 15:39 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

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

> Often Linux systems are shipped with wget(1) by default (and no curl).
>
> Many BSDs, macOS, and e.g. some Linux minimal/container images
> comes with curl(1) (and no wget).
>
> Attempting to download with curl if wget is not available increases
> the likelihood for this to succeed.

the patch is OK for me. What I wondered was if we should switch to
committing the test artifact(s) to a git branch (maybe even to
master). Originally I was worried about accumulating binary blobs in git
history, but it seems we rarely change this blob.  Of course using git
is problem for people who want to run the upgrade tests without a git
checkout; I'm not sure how many of those people there are.

The performance test corpus is large enough that I think the seperate
download makes sense. I also hope that it will be updated to reflect
more real world bottlenecks.

d

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

end of thread, other threads:[~2017-03-14 15:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-12 12:59 [PATCH] test-databases: use wget or curl to download test databases Tomi Ollila
2017-03-12 18:31 ` [PATCH] test: verify test database v1 checksum in more portable way Tomi Ollila
2017-03-12 22:33   ` David Bremner
2017-03-13 19:32     ` Tomi Ollila
2017-03-14 15:39 ` [PATCH] test-databases: use wget or curl to download test databases 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).