zsh-workers
 help / color / mirror / code / Atom feed
* Lots of test failures when --disable-multibyte
@ 2022-04-04  2:03 Bart Schaefer
  2022-04-04 14:43 ` Peter Stephenson
  2022-04-04 18:52 ` Lots of test failures when --disable-multibyte Bart Schaefer
  0 siblings, 2 replies; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04  2:03 UTC (permalink / raw)
  To: Zsh hackers list

Many seem to be because typeset fails to apply $'...' quoting to
strings with embedded metacharacters or NUL bytes, but there are
several other odd ones too.

./A03quoting.ztst: starting.
--- /tmp/zsh.ztst.150326/ztst.out    2022-04-03 18:48:09.354312275 -0700
+++ /tmp/zsh.ztst.150326/ztst.tout    2022-04-03 18:48:09.354312275 -0700
@@ -1 +1,3 @@
-$'one\\two\n\'buckle\'\tmy\\shoe\n'
+'one\two
+'\''buckle'\''    my\shoe
+'
Test ./A03quoting.ztst failed: output differs from expected as shown above for:
 foo=$'one\\two\n\'buckle\'\tmy\\shoe\n'
 print -r ${(q+)foo}
Was testing: Extended minimal quoting of quotes and backslashes
./A03quoting.ztst: test failed.

./B02typeset.ztst: starting.
--- /tmp/zsh.ztst.157742/ztst.out    2022-04-03 18:48:24.710512660 -0700
+++ /tmp/zsh.ztst.157742/ztst.tout    2022-04-03 18:48:24.714512712 -0700
@@ -1,7 +1,7 @@
 l o c a
 typeset -UT SCALAR array=( l o c a ) ''
 typeset -aT SCALAR array=( l o c a ) ''
-SCALAR=$'l\C-@o\C-@c\C-@a'
+SCALAR=loca
 array=( l o c a )
 local unique tied array SCALAR
 array local tied SCALAR array
Test ./B02typeset.ztst failed: output differs from expected as shown above for:
 typeset -T SCALAR=$'l\000o\000c\000a\000l' array $'\000'
 typeset -U SCALAR
 print $array
 typeset -p SCALAR array
 typeset -m SCALAR array
 typeset +m SCALAR array
 [[ $SCALAR == $'l\000o\000c\000a' ]]
Was testing: Tied parameters and uniquified arrays with NUL-character
as separator
./B02typeset.ztst: test failed.

./B03print.ztst: starting.
--- /tmp/zsh.ztst.157826/ztst.out    2022-04-03 18:48:26.106530878 -0700
+++ /tmp/zsh.ztst.157826/ztst.tout    2022-04-03 18:48:26.110530931 -0700
@@ -1,2 +1,2 @@
 typeset -g foo='once more'
-typeset -g foo=$'into\C-@the-breach\C-@-'
+typeset -g foo=intothe-breach-
Test ./B03print.ztst failed: output differs from expected as shown above for:
 unset foo
 print -v foo once more
 typeset -p foo
 printf -v foo "%s\0%s-" into the breach
 typeset -p foo
Was testing: print and printf into a variable
./B03print.ztst: test failed.

./D01prompt.ztst: starting.
--- /tmp/zsh.ztst.159862/ztst.out    2022-04-03 18:48:45.714786786 -0700
+++ /tmp/zsh.ztst.159862/ztst.tout    2022-04-03 18:48:45.718786838 -0700
@@ -1 +1 @@
-
+V
Test ./D01prompt.ztst failed: output differs from expected as shown above for:
  print ${(%U)Y-%(v}
Was testing: Regression test for test on empty psvar
./D01prompt.ztst: test failed.

./D04parameter.ztst: starting.
--- /tmp/zsh.ztst.160387/ztst.out       2022-04-03 18:48:48.790826935 -0700
+++ /tmp/zsh.ztst.160387/ztst.tout    2022-04-03 18:48:48.790826935 -0700
@@ -1 +1 @@
-^?^@
+\C-?\C-@
Test ./D04parameter.ztst failed: output differs from expected as shown
above for:
  foo=$'\x7f\x00'
  print -r -- ${(V)foo}
Was testing: ${(V)...}
./D04parameter.ztst: test failed.

./E02xtrace.ztst: starting.
--- /tmp/zsh.ztst.161648/ztst.out    2022-04-03 18:48:57.014934283 -0700
+++ /tmp/zsh.ztst.161648/ztst.tout    2022-04-03 18:48:57.018934335 -0700
@@ -6,11 +6,11 @@
     # traced
     echo inner
 }
-$'\M-c\M-\C-C\M-\C-L' () {
+ヌ () {
     # traced
     echo inner
 }
-$'ba\C-@z' () {
+baz () {
     # traced
     echo inner
 }
Test ./E02xtrace.ztst failed: output differs from expected as shown above for:
  test_cases=(
      f            # baseline
      foo-bar      # Dash
      ヌ           # Meta (the UTF-8 representation of this character
has an 0x83 byte)
      \$\'ba\\0z\' # Nul, escaped as though by ${(qqqq)}
  )
  for 1 in "$test_cases[@]"; do
    eval "
      ${1}() {
        ${1}() { echo inner }
      }
      functions -T ${1}
      ${1}
      LC_ALL=C which ${1}
    "
  done
Was testing: a function that redefines itself preserves tracing
./E02xtrace.ztst: test failed.

./V05styles.ztst: starting.
--- /tmp/zsh.ztst.162371/ztst.out    2022-04-03 18:49:01.322990518 -0700
+++ /tmp/zsh.ztst.162371/ztst.tout    2022-04-03 18:49:01.326990570 -0700
@@ -1 +1 @@
-zstyle $'con\C-@text' $'ke\C-@y' $'val\C-@u' e
+zstyle context key valu e
Test ./V05styles.ztst failed: output differs from expected as shown above for:
 (
  zstyle $'con\x00text' $'ke\x00y' $'val\x00u' $'e'
  a=( ${(f)"$(zstyle -L)"} )
  a=( ${(M)a:#*con*text*ke*y*val*u*e} )
  print -r -- "$a"
 )
Was testing: zstyle -L escapes the key (regression: workers/48424)
./V05styles.ztst: test failed.

./V09datetime.ztst: starting.
Test case skipped: Japanese UTF-8 locale not supported
--- /tmp/zsh.ztst.162531/ztst.out    2022-04-03 18:49:01.842997306 -0700
+++ /tmp/zsh.ztst.162531/ztst.tout    2022-04-03 18:49:01.846997358 -0700
@@ -1 +1 @@
-1973^@03^@03
+1973\C-@03\C-@03
Test ./V09datetime.ztst failed: output differs from expected as shown above for:
  print -r -- ${(V)"$(strftime $'%Y\0%m\0%d' 100000000)"}
Was testing: Embedded nulls
./V09datetime.ztst: test failed.

... and a completion test I won't quote here that failed because the
{a..z} notation didn't work for the expression
  comptesteval "typeset -a bar=({$'\\0'..$'\\C-?'})"


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04  2:03 Lots of test failures when --disable-multibyte Bart Schaefer
@ 2022-04-04 14:43 ` Peter Stephenson
  2022-04-04 15:31   ` Bart Schaefer
  2022-04-04 18:52 ` Lots of test failures when --disable-multibyte Bart Schaefer
  1 sibling, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2022-04-04 14:43 UTC (permalink / raw)
  To: Bart Schaefer, Zsh hackers list

On 04 April 2022 at 03:03 Bart Schaefer <schaefer@brasslantern.com> wrote:
> Many seem to be because typeset fails to apply $'...' quoting to
> strings with embedded metacharacters or NUL bytes, but there are
> several other odd ones too.

Adding some extra single-byte "nice" quoting isn't so hard; this won't fix
everything but seems OK as far as it goes.

pws

commit 61802c2b2b06cab976d392e145d8db5d372a879d
Author: Peter Stephenson <p.stephenson@samsung.com>
Date:   Mon Apr 4 15:34:40 2022 +0100

    Single byte versions of nice quoting

diff --git a/Src/utils.c b/Src/utils.c
index f9127c70c..c5ee78db6 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -438,7 +438,6 @@ putshout(int c)
     return 0;
 }
 
-#ifdef MULTIBYTE_SUPPORT
 /*
  * Turn a character into a visible representation thereof.  The visible
  * string is put together in a static buffer, and this function returns
@@ -517,6 +516,8 @@ nicechar_sel(int c, int quotable)
     return buf;
 }
 
+#ifdef MULTIBYTE_SUPPORT
+
 /**/
 mod_export char *
 nicechar(int c)
@@ -5793,7 +5794,7 @@ mb_charlenconv(const char *s, int slen, wint_t *wcp)
 }
 
 /**/
-#else
+#else /* MULTIBYTE_SUPPORT */
 
 /* Simple replacement for mb_metacharlenconv */
 
@@ -5833,6 +5834,121 @@ charlenconv(const char *x, int len, int *c)
     return 1;
 }
 
+/*
+ * Non-multibyte version of mb_niceformat() above.  Same basic interface.
+ */
+
+/**/
+mod_export size_t
+sb_niceformat(const char *s, FILE *stream, char **outstrp, int flags)
+{
+    size_t l = 0, newl;
+    int umlen, outalloc, outleft;
+    char *ums, *ptr, *eptr, *fmt, *outstr, *outptr;
+
+    if (outstrp) {
+	outleft = outalloc = 2 * strlen(s);
+	outptr = outstr = zalloc(outalloc);
+    } else {
+	outleft = outalloc = 0;
+	outptr = outstr = NULL;
+    }
+
+    ums = ztrdup(s);
+    /*
+     * is this necessary at this point? niceztrlen does this
+     * but it's used in lots of places.  however, one day this may
+     * be, too.
+     */
+    untokenize(ums);
+    ptr = unmetafy(ums, &umlen);
+    eptr = ptr + umlen;
+
+    while (ptr < eptr) {
+	int c = STOUC(*ptr);
+	if (c == '\'' && (flags & NICEFLAG_QUOTE)) {
+	    fmt = "\\'";
+	    newl = 2;
+	}
+	else if (c == '\\' && (flags & NICEFLAG_QUOTE)) {
+	    fmt = "\\\\";
+	    newl = 2;
+	}
+	else {
+	    fmt = nicechar_sel(c, flags & NICEFLAG_QUOTE);
+	    newl = 1;
+	}
+
+	++ptr;
+	l += newl;
+
+	if (stream)
+	    zputs(fmt, stream);
+	if (outstr) {
+	    /* Append to output string */
+	    int outlen = strlen(fmt);
+	    if (outlen >= outleft) {
+		/* Reallocate to twice the length */
+		int outoffset = outptr - outstr;
+
+		outleft += outalloc;
+		outalloc *= 2;
+		outstr = zrealloc(outstr, outalloc);
+		outptr = outstr + outoffset;
+	    }
+	    memcpy(outptr, fmt, outlen);
+	    /* Update start position */
+	    outptr += outlen;
+	    /* Update available bytes */
+	    outleft -= outlen;
+	}
+    }
+
+    free(ums);
+    if (outstrp) {
+	*outptr = '\0';
+	/* Use more efficient storage for returned string */
+	if (flags & NICEFLAG_NODUP)
+	    *outstrp = outstr;
+	else {
+	    *outstrp = (flags & NICEFLAG_HEAP) ? dupstring(outstr) :
+		ztrdup(outstr);
+	    free(outstr);
+	}
+    }
+
+    return l;
+}
+
+/*
+ * Return 1 if sb_niceformat() would reformat this string, else 0.
+ */
+
+/**/
+mod_export int
+is_sb_niceformat(const char *s)
+{
+    int umlen, ret = 0;
+    char *ums, *ptr, *eptr;
+
+    ums = ztrdup(s);
+    untokenize(ums);
+    ptr = unmetafy(ums, &umlen);
+    eptr = ptr + umlen;
+
+    while (ptr < eptr) {
+	if (is_nicechar(*ptr))  {
+	    ret = 1;
+	    break;
+	}
+	++ptr;
+    }
+
+    free(ums);
+
+    return ret;
+}
+
 /**/
 #endif /* MULTIBYTE_SUPPORT */
 
@@ -6366,6 +6482,22 @@ quotedzputs(char const *s, FILE *stream)
 	    return outstr;
 	}
     }
+#else
+    if (is_sb_niceformat(s)){
+	if (stream) {
+	    fputs("$'", stream);
+	    sb_niceformat(s, stream, NULL, NICEFLAG_QUOTE);
+	    fputc('\'', stream);
+	    return NULL;
+	} else {
+	    char *substr;
+	    sb_niceformat(s, NULL, &substr, NICEFLAG_QUOTE|NICEFLAG_NODUP);
+	    outstr = (char *)zhalloc(4 + strlen(substr));
+	    sprintf(outstr, "$'%s'", substr);
+	    free(substr);
+	    return outstr;
+	}
+    }
 #endif /* MULTIBYTE_SUPPORT */
 
     if (!hasspecial(s)) {
diff --git a/Src/zsh.h b/Src/zsh.h
index 641d9c95c..40f9ea537 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -3277,14 +3277,15 @@ enum zexit_t {
 #define AFTERTRAPHOOK  (zshhooks + 2)
 #define GETCOLORATTR   (zshhooks + 3)
 
-#ifdef MULTIBYTE_SUPPORT
-/* Final argument to mb_niceformat() */
+/* Final argument to [ms]b_niceformat() */
 enum {
     NICEFLAG_HEAP = 1,		/* Heap allocation where needed */
     NICEFLAG_QUOTE = 2,		/* Result will appear in $'...' */
     NICEFLAG_NODUP = 4,         /* Leave allocated */
 };
 
+#ifdef MULTIBYTE_SUPPORT
+
 /* Metafied input */
 #define nicezputs(str, outs)	(void)mb_niceformat((str), (outs), NULL, 0)
 #define MB_METACHARINIT()	mb_charinit()


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 14:43 ` Peter Stephenson
@ 2022-04-04 15:31   ` Bart Schaefer
  2022-04-04 16:23     ` Peter Stephenson
  2022-04-04 21:10     ` Bart Schaefer
  0 siblings, 2 replies; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04 15:31 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Apr 4, 2022 at 7:43 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> On 04 April 2022 at 03:03 Bart Schaefer <schaefer@brasslantern.com> wrote:
> > Many seem to be because typeset fails to apply $'...' quoting
>
> Adding some extra single-byte "nice" quoting isn't so hard; this won't fix
> everything but seems OK as far as it goes.

Yes, this cleans up the majority of the test failures, thanks!

Remaining are that ${(V)...} doesn't output the same format in
single-byte mode, and that {$'\\0'..$'\\C-?'} doesn't expand to a
range.


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 15:31   ` Bart Schaefer
@ 2022-04-04 16:23     ` Peter Stephenson
  2022-04-04 21:10     ` Bart Schaefer
  1 sibling, 0 replies; 16+ messages in thread
From: Peter Stephenson @ 2022-04-04 16:23 UTC (permalink / raw)
  To: Zsh hackers list

This tidies single-byte niceness up to bring the interface roughly into line
with the one for multibyte compilation.  (Note that, as Bart will already know,
this has no effect for "unsetopt multibyte" if multbyte was compiled in, it's
purely for systems that don't support it at all.)

I'll commit this tomorrow if nothing further turns up along this front (I'll
assume other test issues are unrelated).

pws

diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index 0fed297b5..57789c0f3 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -2248,15 +2248,13 @@ iprintm(Cmgroup g, Cmatch *mp, UNUSED(int mc), UNUSED(int ml), int lastc, int wi
 #ifdef MULTIBYTE_SUPPORT
 	len = mb_niceformat(m->disp, shout, NULL, 0);
 #else
-	nicezputs(m->disp, shout);
-	len = niceztrlen(m->disp);
+	len = sb_niceformat(m->disp, shout, NULL, 0);
 #endif
     } else {
 #ifdef MULTIBYTE_SUPPORT
 	len = mb_niceformat(m->str, shout, NULL, 0);
 #else
-	nicezputs(m->str, shout);
-	len = niceztrlen(m->str);
+	len = sb_niceformat(m->str, shout, NULL, 0);
 #endif
 
 	if ((g->flags & CGF_FILES) && m->modec) {
diff --git a/Src/utils.c b/Src/utils.c
index f9127c70c..d92e00024 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -438,7 +438,6 @@ putshout(int c)
     return 0;
 }
 
-#ifdef MULTIBYTE_SUPPORT
 /*
  * Turn a character into a visible representation thereof.  The visible
  * string is put together in a static buffer, and this function returns
@@ -517,6 +516,8 @@ nicechar_sel(int c, int quotable)
     return buf;
 }
 
+#ifdef MULTIBYTE_SUPPORT
+
 /**/
 mod_export char *
 nicechar(int c)
@@ -5253,26 +5254,11 @@ zputs(char const *s, FILE *stream)
 mod_export char *
 nicedup(char const *s, int heap)
 {
-    int c, len = strlen(s) * 5 + 1;
-    VARARR(char, buf, len);
-    char *p = buf, *n;
+    char *retstr;
 
-    while ((c = *s++)) {
-	if (itok(c)) {
-	    if (c <= Comma)
-		c = ztokens[c - Pound];
-	    else
-		continue;
-	}
-	if (c == Meta)
-	    c = *s++ ^ 32;
-	/* The result here is metafied */
-	n = nicechar(c);
-	while(*n)
-	    *p++ = *n++;
-    }
-    *p = '\0';
-    return heap ? dupstring(buf) : ztrdup(buf);
+    (void)sb_niceformat(s, NULL, &retstr, heap ? NICEFLAG_HEAP : 0);
+
+    return retstr;
 }
 #endif
 
@@ -5291,20 +5277,7 @@ nicedupstring(char const *s)
 mod_export int
 nicezputs(char const *s, FILE *stream)
 {
-    int c;
-
-    while ((c = *s++)) {
-	if (itok(c)) {
-	    if (c <= Comma)
-		c = ztokens[c - Pound];
-	    else
-		continue;
-	}
-	if (c == Meta)
-	    c = *s++ ^ 32;
-	if(zputs(nicechar(c), stream) < 0)
-	    return EOF;
-    }
+    sb_niceformat(s, stream, NULL, 0);
     return 0;
 }
 
@@ -5793,7 +5766,7 @@ mb_charlenconv(const char *s, int slen, wint_t *wcp)
 }
 
 /**/
-#else
+#else /* MULTIBYTE_SUPPORT */
 
 /* Simple replacement for mb_metacharlenconv */
 
@@ -5833,6 +5806,121 @@ charlenconv(const char *x, int len, int *c)
     return 1;
 }
 
+/*
+ * Non-multibyte version of mb_niceformat() above.  Same basic interface.
+ */
+
+/**/
+mod_export size_t
+sb_niceformat(const char *s, FILE *stream, char **outstrp, int flags)
+{
+    size_t l = 0, newl;
+    int umlen, outalloc, outleft;
+    char *ums, *ptr, *eptr, *fmt, *outstr, *outptr;
+
+    if (outstrp) {
+	outleft = outalloc = 2 * strlen(s);
+	outptr = outstr = zalloc(outalloc);
+    } else {
+	outleft = outalloc = 0;
+	outptr = outstr = NULL;
+    }
+
+    ums = ztrdup(s);
+    /*
+     * is this necessary at this point? niceztrlen does this
+     * but it's used in lots of places.  however, one day this may
+     * be, too.
+     */
+    untokenize(ums);
+    ptr = unmetafy(ums, &umlen);
+    eptr = ptr + umlen;
+
+    while (ptr < eptr) {
+	int c = STOUC(*ptr);
+	if (c == '\'' && (flags & NICEFLAG_QUOTE)) {
+	    fmt = "\\'";
+	    newl = 2;
+	}
+	else if (c == '\\' && (flags & NICEFLAG_QUOTE)) {
+	    fmt = "\\\\";
+	    newl = 2;
+	}
+	else {
+	    fmt = nicechar_sel(c, flags & NICEFLAG_QUOTE);
+	    newl = 1;
+	}
+
+	++ptr;
+	l += newl;
+
+	if (stream)
+	    zputs(fmt, stream);
+	if (outstr) {
+	    /* Append to output string */
+	    int outlen = strlen(fmt);
+	    if (outlen >= outleft) {
+		/* Reallocate to twice the length */
+		int outoffset = outptr - outstr;
+
+		outleft += outalloc;
+		outalloc *= 2;
+		outstr = zrealloc(outstr, outalloc);
+		outptr = outstr + outoffset;
+	    }
+	    memcpy(outptr, fmt, outlen);
+	    /* Update start position */
+	    outptr += outlen;
+	    /* Update available bytes */
+	    outleft -= outlen;
+	}
+    }
+
+    free(ums);
+    if (outstrp) {
+	*outptr = '\0';
+	/* Use more efficient storage for returned string */
+	if (flags & NICEFLAG_NODUP)
+	    *outstrp = outstr;
+	else {
+	    *outstrp = (flags & NICEFLAG_HEAP) ? dupstring(outstr) :
+		ztrdup(outstr);
+	    free(outstr);
+	}
+    }
+
+    return l;
+}
+
+/*
+ * Return 1 if sb_niceformat() would reformat this string, else 0.
+ */
+
+/**/
+mod_export int
+is_sb_niceformat(const char *s)
+{
+    int umlen, ret = 0;
+    char *ums, *ptr, *eptr;
+
+    ums = ztrdup(s);
+    untokenize(ums);
+    ptr = unmetafy(ums, &umlen);
+    eptr = ptr + umlen;
+
+    while (ptr < eptr) {
+	if (is_nicechar(*ptr))  {
+	    ret = 1;
+	    break;
+	}
+	++ptr;
+    }
+
+    free(ums);
+
+    return ret;
+}
+
 /**/
 #endif /* MULTIBYTE_SUPPORT */
 
@@ -6366,6 +6454,22 @@ quotedzputs(char const *s, FILE *stream)
 	    return outstr;
 	}
     }
+#else
+    if (is_sb_niceformat(s)){
+	if (stream) {
+	    fputs("$'", stream);
+	    sb_niceformat(s, stream, NULL, NICEFLAG_QUOTE);
+	    fputc('\'', stream);
+	    return NULL;
+	} else {
+	    char *substr;
+	    sb_niceformat(s, NULL, &substr, NICEFLAG_QUOTE|NICEFLAG_NODUP);
+	    outstr = (char *)zhalloc(4 + strlen(substr));
+	    sprintf(outstr, "$'%s'", substr);
+	    free(substr);
+	    return outstr;
+	}
+    }
 #endif /* MULTIBYTE_SUPPORT */
 
     if (!hasspecial(s)) {
diff --git a/Src/zsh.h b/Src/zsh.h
index 641d9c95c..40f9ea537 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -3277,14 +3277,15 @@ enum zexit_t {
 #define AFTERTRAPHOOK  (zshhooks + 2)
 #define GETCOLORATTR   (zshhooks + 3)
 
-#ifdef MULTIBYTE_SUPPORT
-/* Final argument to mb_niceformat() */
+/* Final argument to [ms]b_niceformat() */
 enum {
     NICEFLAG_HEAP = 1,		/* Heap allocation where needed */
     NICEFLAG_QUOTE = 2,		/* Result will appear in $'...' */
     NICEFLAG_NODUP = 4,         /* Leave allocated */
 };
 
+#ifdef MULTIBYTE_SUPPORT
+
 /* Metafied input */
 #define nicezputs(str, outs)	(void)mb_niceformat((str), (outs), NULL, 0)
 #define MB_METACHARINIT()	mb_charinit()


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04  2:03 Lots of test failures when --disable-multibyte Bart Schaefer
  2022-04-04 14:43 ` Peter Stephenson
@ 2022-04-04 18:52 ` Bart Schaefer
  1 sibling, 0 replies; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04 18:52 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, Apr 3, 2022 at 7:03 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> ./D01prompt.ztst: starting.
> --- /tmp/zsh.ztst.159862/ztst.out    2022-04-03 18:48:45.714786786 -0700
> +++ /tmp/zsh.ztst.159862/ztst.tout    2022-04-03 18:48:45.718786838 -0700
> @@ -1 +1 @@
> -
> +V
> Test ./D01prompt.ztst failed: output differs from expected as shown above for:
>   print ${(%U)Y-%(v}
> Was testing: Regression test for test on empty psvar
> ./D01prompt.ztst: test failed.

Here's a patch for that one, bringing it in line with the multibyte branch.

diff --git a/Src/hist.c b/Src/hist.c
index d4557d424..f9440dba7 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2252,6 +2252,7 @@ casemodify(char *str, int how)
 #endif
     while (*str) {
         int c;
+        int mod = 0;
         if (*str == Meta) {
         c = str[1] ^ 32;
         str += 2;
@@ -2259,13 +2260,17 @@ casemodify(char *str, int how)
         c = *str++;
         switch (how) {
         case CASMOD_LOWER:
-        if (isupper(c))
+        if (isupper(c)) {
             c = tolower(c);
+            mod = 1;
+        }
         break;

         case CASMOD_UPPER:
-        if (islower(c))
+        if (islower(c)) {
             c = toupper(c);
+            mod = 1;
+        }
         break;

         case CASMOD_CAPS:
@@ -2273,14 +2278,18 @@ casemodify(char *str, int how)
         if (!ialnum(c))
             nextupper = 1;
         else if (nextupper) {
-            if (islower(c))
+            if (islower(c)) {
             c = toupper(c);
+            mod = 1;
+            }
             nextupper = 0;
-        } else if (isupper(c))
+        } else if (isupper(c)) {
             c = tolower(c);
+            mod = 1;
+        }
         break;
         }
-        if (imeta(c)) {
+        if (mod && imeta(c)) {
         *ptr2++ = Meta;
         *ptr2++ = c ^ 32;
         } else


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 15:31   ` Bart Schaefer
  2022-04-04 16:23     ` Peter Stephenson
@ 2022-04-04 21:10     ` Bart Schaefer
  2022-04-04 21:45       ` Bart Schaefer
  1 sibling, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04 21:10 UTC (permalink / raw)
  To: Zsh hackers list

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

On Mon, Apr 4, 2022 at 8:31 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> Remaining are that ${(V)...} doesn't output the same format in
> single-byte mode, and that {$'\\0'..$'\\C-?'} doesn't expand to a
> range.

Here's a patch for the {a..z} discrepancy.  nicechar() now prefers the
^C format to \C-C, in line with multibyte.  I left a comment behind to
note a branch that seemingly never occurs in single-byte?

[-- Attachment #2: bracechardots.txt --]
[-- Type: text/plain, Size: 1658 bytes --]

diff --git a/Src/glob.c b/Src/glob.c
index d4ffc2274..ca28f20e8 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -2220,7 +2220,7 @@ bracechardots(char *str, convchar_t *c1p, convchar_t *c2p)
 #ifdef MULTIBYTE_SUPPORT
 	cstart == WEOF ||
 #else
-	!cstart ||
+	!*pconv ||
 #endif
 	pnext[0] != '.' || pnext[1] != '.')
 	return 0;
@@ -2241,7 +2241,7 @@ bracechardots(char *str, convchar_t *c1p, convchar_t *c2p)
 #ifdef MULTIBYTE_SUPPORT
 	cend == WEOF ||
 #else
-	!cend ||
+	!*pconv ||
 #endif
 	*pnext != Outbrace)
 	return 0;
@@ -2305,22 +2305,19 @@ xpandbraces(LinkList list, LinkNode *np)
 	    strp = str - str3;
 	    lenalloc = strp + strlen(str2+1) + 1;
 	    do {
-#ifdef MULTIBYTE_SUPPORT
 		char *ncptr;
 		int nclen;
+#ifdef MULTIBYTE_SUPPORT
 		mb_charinit();
 		ncptr = wcs_nicechar(cend, NULL, NULL);
+#else
+		ncptr = nicechar(cend);
+#endif
 		nclen = strlen(ncptr);
 		p = zhalloc(lenalloc + nclen);
 		memcpy(p, str3, strp);
 		memcpy(p + strp, ncptr, nclen);
 		strcpy(p + strp + nclen, str2 + 1);
-#else
-		p = zhalloc(lenalloc + 1);
-		memcpy(p, str3, strp);
-		sprintf(p + strp, "%c", cend);
-		strcat(p + strp, str2 + 1);
-#endif
 		insertlinknode(list, last, p);
 		if (rev)	/* decreasing:  add in reverse order. */
 		    last = nextnode(last);
diff --git a/Src/utils.c b/Src/utils.c
index f9127c70c..66cb2a63a 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -557,9 +557,14 @@ nicechar(int c)
 	*s++ = '\\';
 	c = 't';
     } else if (c < 0x20) {
-	*s++ = '\\';
-	*s++ = 'C';
-	*s++ = '-';
+      /*
+	if (quotable) {
+	    *s++ = '\\';
+	    *s++ = 'C';
+	    *s++ = '-';
+	} else
+      */
+	    *s++ = '^';
 	c += 0x40;
     }
     done:

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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 21:10     ` Bart Schaefer
@ 2022-04-04 21:45       ` Bart Schaefer
  2022-04-04 22:00         ` Bart Schaefer
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04 21:45 UTC (permalink / raw)
  To: Zsh hackers list

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

On Mon, Apr 4, 2022 at 2:10 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Mon, Apr 4, 2022 at 8:31 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > Remaining are that ${(V)...} doesn't output the same format in
> > single-byte mode, and that {$'\\0'..$'\\C-?'} doesn't expand to a
> > range.
>
> Here's a patch for the {a..z} discrepancy.  nicechar() now prefers the
> ^C format to \C-C, in line with multibyte.  I left a comment behind to
> note a branch that seemingly never occurs in single-byte?

I should have looked at ${(V)...} first.  Following patch discards the
single-byte-only implementation of nicechar() in favor of the
"fallback" implementation from multibyte, which fixes ${(V)...} and
supersedes the utils.c hunk of the previous patch.  (I've nevertheless
done them in sequence.)

[-- Attachment #2: nicechar.txt --]
[-- Type: text/plain, Size: 1621 bytes --]

diff --git a/Src/utils.c b/Src/utils.c
index 66cb2a63a..ffa59eff8 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -438,7 +438,6 @@ putshout(int c)
     return 0;
 }
 
-#ifdef MULTIBYTE_SUPPORT
 /*
  * Turn a character into a visible representation thereof.  The visible
  * string is put together in a static buffer, and this function returns
@@ -524,67 +523,6 @@ nicechar(int c)
     return nicechar_sel(c, 0);
 }
 
-#else /* MULTIBYTE_SUPPORT */
-
-/**/
-mod_export char *
-nicechar(int c)
-{
-    static char buf[10];
-    char *s = buf;
-    c &= 0xff;
-    if (ZISPRINT(c))
-	goto done;
-    if (c & 0x80) {
-	if (isset(PRINTEIGHTBIT))
-	    goto done;
-	*s++ = '\\';
-	*s++ = 'M';
-	*s++ = '-';
-	c &= 0x7f;
-	if(ZISPRINT(c))
-	    goto done;
-    }
-    if (c == 0x7f) {
-	*s++ = '\\';
-	*s++ = 'C';
-	*s++ = '-';
-	c = '?';
-    } else if (c == '\n') {
-	*s++ = '\\';
-	c = 'n';
-    } else if (c == '\t') {
-	*s++ = '\\';
-	c = 't';
-    } else if (c < 0x20) {
-      /*
-	if (quotable) {
-	    *s++ = '\\';
-	    *s++ = 'C';
-	    *s++ = '-';
-	} else
-      */
-	    *s++ = '^';
-	c += 0x40;
-    }
-    done:
-    /*
-     * The resulting string is still metafied, so check if
-     * we are returning a character in the range that needs metafication.
-     * This can't happen if the character is printed "nicely", so
-     * this results in a maximum of two bytes total (plus the null).
-     */
-    if (imeta(c)) {
-	*s++ = Meta;
-	*s++ = c ^ 32;
-    } else
-	*s++ = c;
-    *s = 0;
-    return buf;
-}
-
-#endif /* MULTIBYTE_SUPPORT */
-
 /*
  * Return 1 if nicechar() would reformat this character.
  */

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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 21:45       ` Bart Schaefer
@ 2022-04-04 22:00         ` Bart Schaefer
  2022-04-05 16:00           ` Bart Schaefer
  2022-04-05 20:29           ` Peter Stephenson
  0 siblings, 2 replies; 16+ messages in thread
From: Bart Schaefer @ 2022-04-04 22:00 UTC (permalink / raw)
  To: Zsh hackers list

After applying 49989 through 49992, the only remaining** test failure
in single-byte compilation is:

./D04parameter.ztst: starting.
--- /tmp/zsh.ztst.280446/ztst.out       2022-04-04 14:36:24.025587676 -0700
+++ /tmp/zsh.ztst.280446/ztst.tout    2022-04-04 14:36:24.025587676 -0700
@@ -1,4 +1,4 @@
 x
+
 x
-x
-y
+
Test ./D04parameter.ztst failed: output differs from expected as shown
above for:
  a=(1 "" 3)
  print -rl -- "${(@)a//*/x}"
  a=""
  print -rl -- "${(@)a//*/y}"
Was testing: Zero-length string match in parameter substitution

Plus the one immediately after it (Zero-length string match at end).

I'm going to have to leave those one to someone else.

** We might want to consider a ZTST_ mode where we attempt all tests
in a file even if one fails.  I confirmed these by marking them "0f"
one by one, but that could become tedious.


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 22:00         ` Bart Schaefer
@ 2022-04-05 16:00           ` Bart Schaefer
  2022-04-05 16:15             ` Mikael Magnusson
  2022-04-05 20:29           ` Peter Stephenson
  1 sibling, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2022-04-05 16:00 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, Apr 4, 2022 at 3:00 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> Was testing: Zero-length string match in parameter substitution
>
> Plus the one immediately after it (Zero-length string match at end).
>
> I'm going to have to leave those one to someone else.

Actually this turned out to be pretty trivial, the git revision on the
test cases made it possible to find the previous patches to the
multibyte code and then copy to the corresponding spot.

diff --git a/Src/glob.c b/Src/glob.c
index ca28f20e8..400be12d5 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -3331,7 +3331,7 @@ igetmatch(char **sp, Patprog p, int fl, int n,
char *replstr,
         /* Largest possible match at tail of string:       *
          * move forward along string until we get a match. *
          * Again there's no optimisation.                  */
-        for (ioff = 0, t = s, umlen = uml; t < send;
+        for (ioff = 0, t = s, umlen = uml; t <= send;
          ioff++, t++, umlen--) {
         set_pat_start(p, t-s);
         if (pattrylen(p, t, send - t, umlen, &patstralloc, ioff)) {
@@ -3362,7 +3362,7 @@ igetmatch(char **sp, Patprog p, int fl, int n,
char *replstr,
         do {
         /* loop over all matches for global substitution */
         matched = 0;
-        for (; t < send; t++, ioff++, umlen--) {
+        for (; t <= send; t++, ioff++, umlen--) {
             /* Find the longest match from this position. */
             set_pat_start(p, t-s);
             if (pattrylen(p, t, send - t, umlen, &patstralloc, ioff)) {


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-05 16:00           ` Bart Schaefer
@ 2022-04-05 16:15             ` Mikael Magnusson
  0 siblings, 0 replies; 16+ messages in thread
From: Mikael Magnusson @ 2022-04-05 16:15 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On 4/5/22, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mon, Apr 4, 2022 at 3:00 PM Bart Schaefer <schaefer@brasslantern.com>
> wrote:
>>
>> Was testing: Zero-length string match in parameter substitution
>>
>> Plus the one immediately after it (Zero-length string match at end).
>>
>> I'm going to have to leave those one to someone else.
>
> Actually this turned out to be pretty trivial, the git revision on the
> test cases made it possible to find the previous patches to the
> multibyte code and then copy to the corresponding spot.

Perhaps it's worth adding a note to the release procedure to (ask
someone to) run the test suite on the non-multibyte build?

-- 
Mikael Magnusson


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

* Re: Lots of test failures when --disable-multibyte
  2022-04-04 22:00         ` Bart Schaefer
  2022-04-05 16:00           ` Bart Schaefer
@ 2022-04-05 20:29           ` Peter Stephenson
  2022-04-06  3:48             ` Bart Schaefer
  2022-04-06  5:32             ` ZTST_continue (was Re: Lots of test failures when --disable-multibyte) Jun T
  1 sibling, 2 replies; 16+ messages in thread
From: Peter Stephenson @ 2022-04-05 20:29 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2022-04-04 at 15:00 -0700, Bart Schaefer wrote:
> ** We might want to consider a ZTST_ mode where we attempt all tests
> in a file even if one fails.  I confirmed these by marking them "0f"
> one by one, but that could become tedious.

This might help?

pws

diff --git a/Test/README b/Test/README
index 726d68e72..670434ac3 100644
--- a/Test/README
+++ b/Test/README
@@ -20,6 +20,13 @@ more information about the tests being performed with
   ZTST_verbose=1 make check
 (`test' is equivalent to `check') or change 1 to 2 for even more detail.
 
+A test file is usually aborted on the first error.  To continue to the
+end, run with
+  ZTST_continue=1 make check
+This can usefully be combined with ZTST_verbose.  The test is always
+aborted on a syntax error as in that case it is not obvoius how to
+continue.
+
 Individual or groups of tests can be performed with
   make TESTNUM=C02 check
 or
diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index 89fe69b5b..a5e118550 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -143,6 +143,8 @@ ZTST_testfailed() {
 $ZTST_failmsg"
   fi
   ZTST_testfailed=1
+  ret=1
+  (( ++failures ))
   return 1
 }
 ZTST_testxpassed() {
@@ -373,12 +375,12 @@ ZTST_diff() {
 
   return "$diff_ret"
 }
-    
+
 ZTST_test() {
   local last match mbegin mend found substlines
   local diff_out diff_err
   local ZTST_skip
-  integer expected_to_fail
+  integer expected_to_fail ret failures
 
   while true; do
     rm -f $ZTST_in $ZTST_out $ZTST_err
@@ -492,7 +494,7 @@ $ZTST_curline"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+	if [[ -z $ZTST_continue ]]; then return 1; else continue; fi
       fi
 
       ZTST_verbose 2 "ZTST_test: test produced standard output:
@@ -515,7 +517,7 @@ $(<$ZTST_terr)"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+	if [[ -z $ZTST_continue ]]; then return 1; else continue; fi
       fi
       if [[ $ZTST_flags = *q* && -s $ZTST_err ]]; then
 	substlines="$(<$ZTST_err)"
@@ -529,21 +531,27 @@ $(<$ZTST_terr)}"
         fi
 	ZTST_testfailed "error output differs from expected as shown above for:
 $ZTST_code"
-	return 1
+	if [[ -z $ZTST_continue ]]; then return 1; else continue; fi
       fi
       if (( expected_to_fail )); then
         ZTST_testxpassed
-        return 1
+	if [[ -z $ZTST_continue ]]; then return 1; else continue; fi
       fi
     fi
     ZTST_verbose 1 "Test successful."
     [[ -n $last ]] && break
   done
 
-  ZTST_verbose 2 "ZTST_test: all tests successful"
+  if (( failures )); then
+    ZTST_verbose 1 "ZTST_test: $failures tests failed"
+  else
+    ZTST_verbose 2 "ZTST_test: all tests successful"
+  fi
 
   # reset message to keep ZTST_testfailed output correct
   ZTST_message=''
+
+  return ret
 }
 
 



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

* Re: Lots of test failures when --disable-multibyte
  2022-04-05 20:29           ` Peter Stephenson
@ 2022-04-06  3:48             ` Bart Schaefer
  2022-04-06  5:32             ` ZTST_continue (was Re: Lots of test failures when --disable-multibyte) Jun T
  1 sibling, 0 replies; 16+ messages in thread
From: Bart Schaefer @ 2022-04-06  3:48 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Tue, Apr 5, 2022 at 1:30 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> This might help?

> +  ZTST_continue=1 make check

That is what I had in mind, yes.


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

* ZTST_continue (was Re: Lots of test failures when --disable-multibyte)
  2022-04-05 20:29           ` Peter Stephenson
  2022-04-06  3:48             ` Bart Schaefer
@ 2022-04-06  5:32             ` Jun T
  2022-04-06 13:37               ` Peter Stephenson
  1 sibling, 1 reply; 16+ messages in thread
From: Jun T @ 2022-04-06  5:32 UTC (permalink / raw)
  To: zsh-workers


> 2022/04/06 5:29, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> 
> This might help?

I've been preparing basically the same patch. The only differences are:
use (( $ZTST_continue )) so that "ZTST_continue=0 make check" will work,
rename "failures" --> "ZTST_failures" to avoid possible (future) conflicts.

While reading ztst.zsh I've noticed that the return value of ZTST_prepclean
is not checked. But the comment in B01cd.ztst says:

  %prep
  # ... each chunk of code is evaluated
  # in one go and must return status 0, or the preparation is deemed to have
  # failed and the test ends with an appropriate error message. 

Moreover, I couldn't understand the logic in ZTST_prepclean():
  while ZTST_getchunk; do                                         
    ZTST_execchunk >/dev/null || [[ -n $1 ]] || {   
      [[ -n "$ZTST_unimplemented" ]] ||
      ZTST_testfailed "non-zero status from preparation code:         
$ZTST_code" && return 0                                              
    }                 
  done

Since ZTST_testfailed() always returns 1, the "return 0" is never executed?
But the "return 0" is not required here; instead, it would be better to
"return 1" if a chunk in %prep fails.
# ZTST_unimplemented is taken care of in the main while loop (in which
# ZTST_prepclean is called) and we need not check it here, I think.

With the patch below:
(1) if a chunk in %prep fails then the test is aborted immediately.
(2) If the "bad placement of 'test' section" happens, we do not exit immediately
but run ZTST_cleanup.
Or should we run ZTST_cleanup even in (1)? (it will do no harm...)


diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index 89fe69b5b..e66db8f91 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -17,6 +17,9 @@
 # Defined in such a way that any value from the environment is used.
 : ${ZTST_verbose:=0}
 
+# If non-zero, continue the tests even after a test fails.
+: ${ZTST_continue:=0}
+
 # We require all options to be reset, not just emulation options.
 # Unfortunately, due to the crud which may be in /etc/zshenv this might
 # still not be good enough.  Maybe we should trick it somehow.
@@ -143,6 +146,10 @@ ZTST_testfailed() {
 $ZTST_failmsg"
   fi
   ZTST_testfailed=1
+  # if called from within ZTST_Test() this will increment ZTST_Test's local
+  # ZTST_failures. Otherwise global ZTST_failures will be incremented
+  # (but currently its value is not used).
+  (( ++ZTST_failures ))
   return 1
 }
 ZTST_testxpassed() {
@@ -156,6 +163,7 @@ ZTST_testxpassed() {
 $ZTST_failmsg"
   fi
   ZTST_testfailed=1
+  (( ++ZTST_failures ))
   return 1
 }
 
@@ -291,16 +299,17 @@ ZTST_execchunk() {
 }
 
 # Functions for preparation and cleaning.
-# When cleaning up (non-zero string argument), we ignore status.
-ZTST_prepclean() {
+# When cleaning up, we ignore status.
+ZTST_prep ZTST_clean () {
   # Execute indented code chunks.
   while ZTST_getchunk; do
-    ZTST_execchunk >/dev/null || [[ -n $1 ]] || {
-      [[ -n "$ZTST_unimplemented" ]] ||
+    ZTST_execchunk >/dev/null || [[ $0 = ZTST_clean ]] || {
       ZTST_testfailed "non-zero status from preparation code:
-$ZTST_code" && return 0
+$ZTST_code"
+      return 1
     }
   done
+  return 0
 }
 
 # diff wrapper
@@ -373,12 +382,12 @@ ZTST_diff() {
 
   return "$diff_ret"
 }
-    
+
 ZTST_test() {
   local last match mbegin mend found substlines
   local diff_out diff_err
   local ZTST_skip
-  integer expected_to_fail
+  integer expected_to_fail ZTST_failures
 
   while true; do
     rm -f $ZTST_in $ZTST_out $ZTST_err
@@ -492,7 +501,7 @@ $ZTST_curline"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
 
       ZTST_verbose 2 "ZTST_test: test produced standard output:
@@ -515,7 +524,7 @@ $(<$ZTST_terr)"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
       if [[ $ZTST_flags = *q* && -s $ZTST_err ]]; then
 	substlines="$(<$ZTST_err)"
@@ -529,21 +538,27 @@ $(<$ZTST_terr)}"
         fi
 	ZTST_testfailed "error output differs from expected as shown above for:
 $ZTST_code"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
       if (( expected_to_fail )); then
         ZTST_testxpassed
-        return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
     fi
     ZTST_verbose 1 "Test successful."
     [[ -n $last ]] && break
   done
 
-  ZTST_verbose 2 "ZTST_test: all tests successful"
+  if (( ZTST_failures )); then
+    ZTST_verbose 1 "ZTST_test: $ZTST_failures tests failed"
+  else
+    ZTST_verbose 2 "ZTST_test: all tests successful"
+  fi
 
   # reset message to keep ZTST_testfailed output correct
   ZTST_message=''
+
+  return ZTST_failures
 }
 
 
@@ -565,25 +580,27 @@ while [[ -z "$ZTST_unimplemented" ]] && ZTST_getsect $ZTST_skipok; do
 	    ZTST_testfailed "\`prep' section must come first"
             exit 1
 	  fi
-	  ZTST_prepclean
+	  ZTST_prep || ZTST_skipok=1
 	  ZTST_sects[prep]=1
 	  ;;
     (test)
 	  if (( ${ZTST_sects[test]} + ${ZTST_sects[clean]} )); then
 	    ZTST_testfailed "bad placement of \`test' section"
-	    exit 1
+	    break   # do not run %clean section, but run ZTST_cleanup
 	  fi
-	  # careful here: we can't execute ZTST_test before || or &&
-	  # because that affects the behaviour of traps in the tests.
-	  ZTST_test
-	  (( $? )) && ZTST_skipok=1
+          if [[ -z "$ZTST_skipok" ]]; then  # if no error in %prep
+            # careful here: we can't execute ZTST_test before || or &&
+            # because that affects the behaviour of traps in the tests.
+            ZTST_test
+            (( $? )) && ZTST_skipok=1
+          fi
 	  ZTST_sects[test]=1
 	  ;;
     (clean)
 	   if (( ${ZTST_sects[test]} == 0 || ${ZTST_sects[clean]} )); then
 	     ZTST_testfailed "bad use of \`clean' section"
 	   else
-	     ZTST_prepclean 1
+	     ZTST_clean
 	     ZTST_sects[clean]=1
 	   fi
 	   ZTST_skipok=





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

* Re: ZTST_continue (was Re: Lots of test failures when --disable-multibyte)
  2022-04-06  5:32             ` ZTST_continue (was Re: Lots of test failures when --disable-multibyte) Jun T
@ 2022-04-06 13:37               ` Peter Stephenson
  2022-04-07 12:33                 ` Jun T
  2022-04-07 12:34                 ` Jun T
  0 siblings, 2 replies; 16+ messages in thread
From: Peter Stephenson @ 2022-04-06 13:37 UTC (permalink / raw)
  To: zsh-workers

> On 06 April 2022 at 06:32 Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> > 2022/04/06 5:29, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > 
> > This might help?
> 
> I've been preparing basically the same patch. The only differences are:
> use (( $ZTST_continue )) so that "ZTST_continue=0 make check" will work,
> rename "failures" --> "ZTST_failures" to avoid possible (future) conflicts.

You might as well go ahead.
 
> With the patch below:
> (1) if a chunk in %prep fails then the test is aborted immediately.
> (2) If the "bad placement of 'test' section" happens, we do not exit immediately
> but run ZTST_cleanup.
> Or should we run ZTST_cleanup even in (1)? (it will do no harm...)

Well, since it shouldn't happen it's hard to know what to do if it does.
Cleaning up anything that did get added in the preparation would be kind
of logical, so I can't see why not.

pws


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

* Re: ZTST_continue (was Re: Lots of test failures when --disable-multibyte)
  2022-04-06 13:37               ` Peter Stephenson
@ 2022-04-07 12:33                 ` Jun T
  2022-04-07 12:34                 ` Jun T
  1 sibling, 0 replies; 16+ messages in thread
From: Jun T @ 2022-04-07 12:33 UTC (permalink / raw)
  To: zsh-workers

I will split the patch into two parts; one for ZTST_continue
and another for handling the return status of ZTST_prepclean.

Here is the patch for ZTST_continue (Almost identical with Peter's).
ZTST_failuers is incremented also in ZTST_testxpassed.
(patch for Test/README is identical with Peter's and not included here)


diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index 89fe69b5b..cdc84b160 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -17,6 +17,9 @@
 # Defined in such a way that any value from the environment is used.
 : ${ZTST_verbose:=0}
 
+# If non-zero, continue the tests even after a test fails.
+: ${ZTST_continue:=0}
+
 # We require all options to be reset, not just emulation options.
 # Unfortunately, due to the crud which may be in /etc/zshenv this might
 # still not be good enough.  Maybe we should trick it somehow.
@@ -143,6 +146,10 @@ ZTST_testfailed() {
 $ZTST_failmsg"
   fi
   ZTST_testfailed=1
+  # if called from within ZTST_Test() this will increment ZTST_Test's local
+  # ZTST_failures. Otherwise global ZTST_failures will be incremented
+  # (but currently its value is not used).
+  (( ++ZTST_failures ))
   return 1
 }
 ZTST_testxpassed() {
@@ -156,6 +163,7 @@ ZTST_testxpassed() {
 $ZTST_failmsg"
   fi
   ZTST_testfailed=1
+  (( ++ZTST_failures ))
   return 1
 }
 
@@ -373,12 +381,12 @@ ZTST_diff() {
 
   return "$diff_ret"
 }
-    
+
 ZTST_test() {
   local last match mbegin mend found substlines
   local diff_out diff_err
   local ZTST_skip
-  integer expected_to_fail
+  integer expected_to_fail ZTST_failures
 
   while true; do
     rm -f $ZTST_in $ZTST_out $ZTST_err
@@ -492,7 +500,7 @@ $ZTST_curline"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
 
       ZTST_verbose 2 "ZTST_test: test produced standard output:
@@ -515,7 +523,7 @@ $(<$ZTST_terr)"
 $ZTST_code${$(<$ZTST_terr):+
 Error output:
 $(<$ZTST_terr)}"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
       if [[ $ZTST_flags = *q* && -s $ZTST_err ]]; then
 	substlines="$(<$ZTST_err)"
@@ -529,21 +537,27 @@ $(<$ZTST_terr)}"
         fi
 	ZTST_testfailed "error output differs from expected as shown above for:
 $ZTST_code"
-	return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
       if (( expected_to_fail )); then
         ZTST_testxpassed
-        return 1
+        if (( ZTST_continue ));then continue; else return 1; fi
       fi
     fi
     ZTST_verbose 1 "Test successful."
     [[ -n $last ]] && break
   done
 
-  ZTST_verbose 2 "ZTST_test: all tests successful"
+  if (( ZTST_failures )); then
+    ZTST_verbose 1 "ZTST_test: $ZTST_failures test(s) failed"
+  else
+    ZTST_verbose 2 "ZTST_test: all tests successful"
+  fi
 
   # reset message to keep ZTST_testfailed output correct
   ZTST_message=''
+
+  return ZTST_failures
 }
 
 



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

* Re: ZTST_continue (was Re: Lots of test failures when --disable-multibyte)
  2022-04-06 13:37               ` Peter Stephenson
  2022-04-07 12:33                 ` Jun T
@ 2022-04-07 12:34                 ` Jun T
  1 sibling, 0 replies; 16+ messages in thread
From: Jun T @ 2022-04-07 12:34 UTC (permalink / raw)
  To: zsh-workers

This is for taking care of the return value of ZTST_prep.

ZTST_prep is modified so that if ZTST_unimplemented is set in any chunk
then the remaining chunks of %prep are skipped.

P01privileged.ztst is split into several chunks, and returns 0
when it sets ZTST_unimplemented.



diff --git a/Test/P01privileged.ztst b/Test/P01privileged.ztst
index 7c4a1be35..5d45c1a4c 100644
--- a/Test/P01privileged.ztst
+++ b/Test/P01privileged.ztst
@@ -26,23 +26,23 @@
 
 %prep
 
-  # Mind your empty lines here. The logic in this %prep section is somewhat
-  # complex compared to most others; to avoid lots of nested/duplicated
-  # conditions we need to make sure that this all gets executed as a single
-  # function from which we can return early
+  # If ZTST_unimplemented is set to non-null in a chunk then all the
+  # remaining chunks (and all of %test and %clean sections) will be skipped.
   [[ $EUID == 0 || -n $ZSH_TEST_UNPRIVILEGED_UID$ZSH_TEST_UNPRIVILEGED_GID ]] || {
     ZTST_unimplemented='PRIVILEGED tests require super-user privileges (or env var)'
-    return 1
+    return 0
   }
+
   (( $+commands[perl] )) || { # @todo Eliminate this dependency with a C wrapper?
     ZTST_unimplemented='PRIVILEGED tests require Perl'
-    return 1
+    return 0
   }
+
   grep -qE '#define HAVE_SETRES?UID' $ZTST_testdir/../config.h || {
     ZTST_unimplemented='PRIVILEGED tests require setreuid()/setresuid()'
-    return 1
+    return 0
   }
-  #
+
   ruid= euid= rgid= egid=
   #
   if [[ -n $ZSH_TEST_UNPRIVILEGED_UID ]]; then
@@ -76,13 +76,14 @@
   #
   [[ -n $ruid && -n $euid ]] || {
     ZTST_unimplemented='PRIVILEGED tests require unprivileged UID:EUID'
-    return 1
+    return 0
   }
+
   [[ -n $rgid || -n $egid ]] || {
     ZTST_unimplemented='PRIVILEGED tests require unprivileged GID:EGID'
-    return 1
+    return 0
   }
-  #
+
   print -ru$ZTST_fd \
     "Using unprivileged UID $ruid, EUID $euid, GID $rgid, EGID $egid"
   #
diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index cdc84b160..190deecfd 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -299,16 +299,18 @@ ZTST_execchunk() {
 }
 
 # Functions for preparation and cleaning.
-# When cleaning up (non-zero string argument), we ignore status.
-ZTST_prepclean() {
-  # Execute indented code chunks.
-  while ZTST_getchunk; do
-    ZTST_execchunk >/dev/null || [[ -n $1 ]] || {
-      [[ -n "$ZTST_unimplemented" ]] ||
+ZTST_prep ZTST_clean () {
+  # Execute indented code chunks. If ZTST_unimplemented is set
+  # in any chunk then we will skip the remaining chunks.
+  # We ignore return status of chunks when cleaning up.
+  while [[ -z "$ZTST_unimplemented" ]] && ZTST_getchunk; do
+    ZTST_execchunk >/dev/null || [[ $0 = ZTST_clean ]] || {
       ZTST_testfailed "non-zero status from preparation code:
-$ZTST_code" && return 0
+$ZTST_code"
+      return 1
     }
   done
+  return 0
 }
 
 # diff wrapper
@@ -577,27 +579,29 @@ while [[ -z "$ZTST_unimplemented" ]] && ZTST_getsect $ZTST_skipok; do
     (prep) if (( ${ZTST_sects[prep]} + ${ZTST_sects[test]} + \
 	        ${ZTST_sects[clean]} )); then
 	    ZTST_testfailed "\`prep' section must come first"
-            exit 1
+	    break   # skip %test and %clean sections, but run ZTST_cleanup
 	  fi
-	  ZTST_prepclean
+	  ZTST_prep || ZTST_skipok=1
 	  ZTST_sects[prep]=1
 	  ;;
     (test)
 	  if (( ${ZTST_sects[test]} + ${ZTST_sects[clean]} )); then
 	    ZTST_testfailed "bad placement of \`test' section"
-	    exit 1
+	    break   # skip %clean section, but run ZTST_cleanup
 	  fi
-	  # careful here: we can't execute ZTST_test before || or &&
-	  # because that affects the behaviour of traps in the tests.
-	  ZTST_test
-	  (( $? )) && ZTST_skipok=1
+          if [[ -z "$ZTST_skipok" ]]; then  # if no error in %prep
+            # careful here: we can't execute ZTST_test before || or &&
+            # because that affects the behaviour of traps in the tests.
+            ZTST_test
+            (( $? )) && ZTST_skipok=1
+          fi
 	  ZTST_sects[test]=1
 	  ;;
     (clean)
 	   if (( ${ZTST_sects[test]} == 0 || ${ZTST_sects[clean]} )); then
 	     ZTST_testfailed "bad use of \`clean' section"
 	   else
-	     ZTST_prepclean 1
+	     ZTST_clean
 	     ZTST_sects[clean]=1
 	   fi
 	   ZTST_skipok=





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

end of thread, other threads:[~2022-04-07 12:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04  2:03 Lots of test failures when --disable-multibyte Bart Schaefer
2022-04-04 14:43 ` Peter Stephenson
2022-04-04 15:31   ` Bart Schaefer
2022-04-04 16:23     ` Peter Stephenson
2022-04-04 21:10     ` Bart Schaefer
2022-04-04 21:45       ` Bart Schaefer
2022-04-04 22:00         ` Bart Schaefer
2022-04-05 16:00           ` Bart Schaefer
2022-04-05 16:15             ` Mikael Magnusson
2022-04-05 20:29           ` Peter Stephenson
2022-04-06  3:48             ` Bart Schaefer
2022-04-06  5:32             ` ZTST_continue (was Re: Lots of test failures when --disable-multibyte) Jun T
2022-04-06 13:37               ` Peter Stephenson
2022-04-07 12:33                 ` Jun T
2022-04-07 12:34                 ` Jun T
2022-04-04 18:52 ` Lots of test failures when --disable-multibyte Bart Schaefer

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

	https://git.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).