Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-12 Thread Daniel Kahn Gillmor
On Sat 2020-05-09 08:47:10 -0300, David Bremner wrote:
> I'm confused about where to apply 2/2. If I apply it on top of (updated)
> master, it causes test failures. If I apply after the rest of the
> patches in this thread then presumably there is some interval where the
> build is broken (if only for certain GMime versions).

fwiw, i believe that the current master
( 627460d76b95a07084c2b6fc7f647a5547e1 ) *is* broken on systems with
older gmime.

In particular, on debian buster, i see:

T356-protected-headers: Testing Message decryption with protected headers
 FAIL   verify signed PKCS#7 subject (multipart-signed)
sig_uid: object not found: 
data[0][0][0]["crypto"]["signed"]["status"][0]["userid"]

My [PATCH v2/2 v2] in this thread (sent shortly) should resolve this
situation.  Then, i'll send a rebased version of the full S/MIME series
in a new thread.

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-10 Thread David Bremner
Daniel Kahn Gillmor  writes:

>
> yes, that's right.  For certain GMime versions, there's an intermediate
> breakage.  I think you're saying you want me to rerun the series, with
> these things split out so that even on systems with older GMime, tests
> pass at every commit.  I'll try to get that done today.

Sounds perfect.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-10 Thread Daniel Kahn Gillmor
On Sat 2020-05-09 08:47:10 -0300, David Bremner wrote:
> I'm confused about where to apply 2/2. If I apply it on top of (updated)
> master, it causes test failures. If I apply after the rest of the
> patches in this thread then presumably there is some interval where the
> build is broken (if only for certain GMime versions).

yes, that's right.  For certain GMime versions, there's an intermediate
breakage.  I think you're saying you want me to rerun the series, with
these things split out so that even on systems with older GMime, tests
pass at every commit.  I'll try to get that done today.

 --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-09 Thread David Bremner
Daniel Kahn Gillmor  writes:

> Several functions in test/test-lib.sh used variable names that are
> also used outside of those functions (e.g. $output and $expected are
> used in many of the test scripts), but they are not expected to
> communicate via those variables.
>
> We mark those variables "local" within test-lib.sh so that they do not
> get clobbered when used outside test-lib.
>
> We also move the local variable declarations to beginning of each
> function, to avoid weird gotchas with local variable declarations as
> described in https://tldp.org/LDP/abs/html/localvar.html.

Pushed this one to master.

I'm confused about where to apply 2/2. If I apply it on top of (updated)
master, it causes test failures. If I apply after the rest of the
patches in this thread then presumably there is some interval where the
build is broken (if only for certain GMime versions).

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-09 Thread Tomi Ollila
On Fri, May 08 2020, Daniel Kahn Gillmor wrote:

> Several functions in test/test-lib.sh used variable names that are
> also used outside of those functions (e.g. $output and $expected are
> used in many of the test scripts), but they are not expected to
> communicate via those variables.
>
> We mark those variables "local" within test-lib.sh so that they do not
> get clobbered when used outside test-lib.
>
> We also move the local variable declarations to beginning of each
> function, to avoid weird gotchas with local variable declarations as
> described in https://tldp.org/LDP/abs/html/localvar.html.

LGTM. I have to tolerate 'we' (passive) as I seem to use those myself, too
(although communicating a bit different thing...) =D

Tomi

> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  test/test-lib.sh | 31 ++-
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 5c8eab7c..58972339 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -109,7 +109,6 @@ unset ALTERNATE_EDITOR
>  
>  add_gnupg_home ()
>  {
> -local output
>  [ -e "${GNUPGHOME}/gpg.conf" ] && return
>  _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
>  at_exit_function _gnupg_exit
> @@ -345,13 +344,14 @@ trap 'trap_signal' HUP INT TERM
>  # to the message and encrypting/signing.
>  emacs_deliver_message ()
>  {
> -local subject="$1"
> -local body="$2"
> +local subject body smtp_dummy_pid smtp_dummy_port
> +subject="$1"
> +body="$2"
>  shift 2
>  # before we can send a message, we have to prepare the FCC maildir
>  mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
>  # eval'ing smtp-dummy --background will set smtp_dummy_pid and -_port
> -local smtp_dummy_pid= smtp_dummy_port=
> +smtp_dummy_pid= smtp_dummy_port=
>  eval `$TEST_DIRECTORY/smtp-dummy --background sent_message`
>  test -n "$smtp_dummy_pid" || return 1
>  test -n "$smtp_dummy_port" || return 1
> @@ -391,13 +391,14 @@ emacs_deliver_message ()
>  # new" after message delivery
>  emacs_fcc_message ()
>  {
> -local nmn_args=''
> +local nmn_args subject body
> +nmn_args=''
>  while [[ "$1" =~ ^-- ]]; do
>  nmn_args="$nmn_args $1"
>  shift
>  done
> -local subject="$1"
> -local body="$2"
> +subject="$1"
> +body="$2"
>  shift 2
>  # before we can send a message, we have to prepare the FCC maildir
>  mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
> @@ -427,6 +428,7 @@ emacs_fcc_message ()
>  # number of messages.
>  add_email_corpus ()
>  {
> +local corpus
>  corpus=${1:-default}
>  
>  rm -rf ${MAIL_DIR}
> @@ -457,6 +459,7 @@ test_begin_subtest ()
>  # name.
>  test_expect_equal ()
>  {
> + local output expected testname
>   exec 1>&6 2>&7  # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_expect_equal without 
> test_begin_subtest"
> @@ -483,6 +486,7 @@ test_expect_equal ()
>  # Like test_expect_equal, but takes two filenames.
>  test_expect_equal_file ()
>  {
> + local file1 file2 testname basename1 basename2
>   exec 1>&6 2>&7  # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_expect_equal_file without 
> test_begin_subtest"
> @@ -512,10 +516,11 @@ test_expect_equal_file ()
>  # canonicalized before diff'ing.  If an argument cannot be parsed, it
>  # is used unchanged so that there's something to diff against.
>  test_expect_equal_json () {
> +local script output expected
>  # The test suite forces LC_ALL=C, but this causes Python 3 to
>  # decode stdin as ASCII.  We need to read JSON in UTF-8, so
>  # override Python's stdio encoding defaults.
> -local script='import json, sys; json.dump(json.load(sys.stdin), 
> sys.stdout, sort_keys=True, indent=4)'
> +script='import json, sys; json.dump(json.load(sys.stdin), sys.stdout, 
> sort_keys=True, indent=4)'
>  output=$(echo "$1" | PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c "$script" 
> \
>  || echo "$1")
>  expected=$(echo "$2" | PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c 
> "$script" \
> @@ -540,6 +545,7 @@ test_sort_json () {
>  # read the source of test/json_check_nodes.py (or the output when
>  # invoking it without arguments) for an explanation of the syntax.
>  test_json_nodes () {
> +local output
>  exec 1>&6 2>&7   # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_json_eval without 
> test_begin_subtest"
> @@ -561,6 +567,7 @@ test_json_nodes () {
>  }
>  
>  test_emacs_expect_t () {
> + local result
>   test "$#" = 1 ||
>   error "bug in the test script: not 1 parameter to test_emacs_expect_t"
>   if [ -z "$inside_subtest" ]; then
> @@ -653,7 +660,8 @@ notmuch_json_show_sanitize