unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
@ 2021-05-18  5:54 Tomi Ollila
  2021-05-19  7:29 ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2021-05-18  5:54 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

notmuch_passwd_sanitize() in test-lib.sh is too generic, it cannot
work in many cases...

The more specific version _libconfig_sanitize() replaces it in
T590-libconfig.sh and the code that uses it is modified to output
the keys (ascending numbers printed in hex) so the sanitizer knows
what to sanitize in which lines...

"@" + fqdn -> "@FQDN" replacement is used as fqdn could --
in theory -- be substring of 'USERNAME'.

'user -> 'USER_FULL_NAME replacement to work in cases where user
is empty -- as only first ' is replaced that works as expected.

In addition to ".(none)" now also ".localdomain" is filtered from
USERNAME@FQDN.
---

Changes to [v2]:

* work in cases of empty user (e.g. in passwd gecos field)
* replace only 1st match; e.g. fqdn could contain substring of user

v2: id:20210517193315.11343-1-tomi.ollila@iki.fi
v1: id:20210502181535.31292-1-tomi.ollila@iki.fi

When tried w/ one replacement and w/o sq usage and emptied gecos, got

.  @@ -9,5 +9,5 @@
.  7: 'true'
.  8: 'USERNAME@FQDN'
.  9: 'NULL'
. -a: 'USER_FULL_NAME'
. +USER_FULL_NAMEa: ''

 test/T590-libconfig.sh | 97 +++++++++++++++++++++++++-----------------
 test/test-lib.sh       | 20 ---------
 2 files changed, 59 insertions(+), 58 deletions(-)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 745e1bb4..42cbe6e0 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -5,6 +5,26 @@ test_description="library config API"
 
 add_email_corpus
 
+_libconfig_sanitize() {
+    ${NOTMUCH_PYTHON} -c '
+import os, sys, pwd, socket
+
+pw = pwd.getpwuid(os.getuid())
+user = pw.pw_name
+name = pw.pw_gecos.partition(",")[0]
+fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0,
+                          socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
+for l in sys.stdin:
+    if l[:3] == "8: ":
+        l = l.replace(user, "USERNAME", 1).replace("@" + fqdn, "@FQDN", 1)
+        l = l.replace(".(none)", "", 1).replace(".localdomain", "", 1)
+    elif l[:3] == "a: ":
+        sq = chr(39) # single quote
+        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
+    sys.stdout.write(l)
+'
+}
+
 cat <<EOF > c_head
 #include <string.h>
 #include <stdlib.h>
@@ -380,26 +400,26 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} '' %NULL%
 	 key < NOTMUCH_CONFIG_LAST;
 	 key = (notmuch_config_key_t)(key + 1)) {
 	const char *val = notmuch_config_get (db, key);
-        printf("%s\n", val ? val : "NULL" );
+	printf("%x: '%s'\n", key, val ? val : "NULL" );
     }
 }
 EOF
 
-notmuch_passwd_sanitize < OUTPUT > OUTPUT.clean
+_libconfig_sanitize < OUTPUT > OUTPUT.clean
 
 cat <<'EOF' >EXPECTED
 == stdout ==
-MAIL_DIR
-MAIL_DIR
-MAIL_DIR/.notmuch/hooks
-MAIL_DIR/.notmuch/backups
-
-unread;inbox
-
-true
-USERNAME@FQDN
-NULL
-USER_FULL_NAME
+0: 'MAIL_DIR'
+1: 'MAIL_DIR'
+2: 'MAIL_DIR/.notmuch/hooks'
+3: 'MAIL_DIR/.notmuch/backups'
+4: ''
+5: 'unread;inbox'
+6: ''
+7: 'true'
+8: 'USERNAME@FQDN'
+9: 'NULL'
+a: 'USER_FULL_NAME'
 == stderr ==
 EOF
 unset MAILDIR
@@ -694,23 +714,23 @@ cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR} %NULL% %NULL%
 	 key < NOTMUCH_CONFIG_LAST;
 	 key = (notmuch_config_key_t)(key + 1)) {
 	const char *val = notmuch_config_get (db, key);
-        printf("%s\n", val ? val : "NULL" );
+	printf("%x: '%s'\n", key, val ? val : "NULL" );
     }
 }
 EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
-MAIL_DIR
-MAIL_DIR
-MAIL_DIR/.notmuch/hooks
-MAIL_DIR/.notmuch/backups
-foo;bar;fub
-unread;inbox
-sekrit_junk
-true
-test_suite@notmuchmail.org
-test_suite_other@notmuchmail.org;test_suite@otherdomain.org
-Notmuch Test Suite
+0: 'MAIL_DIR'
+1: 'MAIL_DIR'
+2: 'MAIL_DIR/.notmuch/hooks'
+3: 'MAIL_DIR/.notmuch/backups'
+4: 'foo;bar;fub'
+5: 'unread;inbox'
+6: 'sekrit_junk'
+7: 'true'
+8: 'test_suite@notmuchmail.org'
+9: 'test_suite_other@notmuchmail.org;test_suite@otherdomain.org'
+a: 'Notmuch Test Suite'
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
@@ -723,25 +743,26 @@ cat c_head2 - c_tail <<'EOF' | test_C ${MAIL_DIR} /nonexistent %NULL%
 	 key < NOTMUCH_CONFIG_LAST;
 	 key = (notmuch_config_key_t)(key + 1)) {
 	const char *val = notmuch_config_get (db, key);
-	printf("%s\n", val ? val : "NULL" );
+	printf("%x: '%s'\n", key, val ? val : "NULL" );
     }
 }
 EOF
 
-notmuch_passwd_sanitize < OUTPUT > OUTPUT.clean
+_libconfig_sanitize < OUTPUT > OUTPUT.clean
+
 cat <<'EOF' >EXPECTED
 == stdout ==
-MAIL_DIR
-MAIL_DIR
-MAIL_DIR/.notmuch/hooks
-MAIL_DIR/.notmuch/backups
-
-unread;inbox
-
-true
-USERNAME@FQDN
-NULL
-USER_FULL_NAME
+0: 'MAIL_DIR'
+1: 'MAIL_DIR'
+2: 'MAIL_DIR/.notmuch/hooks'
+3: 'MAIL_DIR/.notmuch/backups'
+4: ''
+5: 'unread;inbox'
+6: ''
+7: 'true'
+8: 'USERNAME@FQDN'
+9: 'NULL'
+a: 'USER_FULL_NAME'
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT.clean
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 6be5d11f..35bea097 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -561,26 +561,6 @@ notmuch_built_with_sanitize () {
     sed 's/^built_with[.]\(.*\)=.*$/built_with.\1=something/'
 }
 
-notmuch_passwd_sanitize () {
-    ${NOTMUCH_PYTHON} -c'
-import os, sys, pwd, socket
-
-pw = pwd.getpwuid(os.getuid())
-user = pw.pw_name
-name = pw.pw_gecos.partition(",")[0]
-fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0, socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
-
-for l in sys.stdin:
-    if user:
-        l = l.replace(user, "USERNAME")
-    if fqdn:
-        l = l.replace(fqdn, "FQDN").replace(".(none)","")
-    if name:
-        l = l.replace(name, "USER_FULL_NAME")
-    sys.stdout.write(l)
-'
-}
-
 notmuch_config_sanitize () {
     notmuch_dir_sanitize | notmuch_built_with_sanitize
 }
-- 
2.25.1

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-18  5:54 [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize() Tomi Ollila
@ 2021-05-19  7:29 ` Felipe Contreras
  2021-05-19  8:44   ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2021-05-19  7:29 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On Tue, May 18, 2021 at 12:55 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
> notmuch_passwd_sanitize() in test-lib.sh is too generic, it cannot
> work in many cases...
>
> The more specific version _libconfig_sanitize() replaces it in
> T590-libconfig.sh and the code that uses it is modified to output
> the keys (ascending numbers printed in hex) so the sanitizer knows
> what to sanitize in which lines...
>
> "@" + fqdn -> "@FQDN" replacement is used as fqdn could --
> in theory -- be substring of 'USERNAME'.
>
> 'user -> 'USER_FULL_NAME replacement to work in cases where user
> is empty -- as only first ' is replaced that works as expected.
>
> In addition to ".(none)" now also ".localdomain" is filtered from
> USERNAME@FQDN.
> ---
>
> Changes to [v2]:
>
> * work in cases of empty user (e.g. in passwd gecos field)
> * replace only 1st match; e.g. fqdn could contain substring of user
>
> v2: id:20210517193315.11343-1-tomi.ollila@iki.fi
> v1: id:20210502181535.31292-1-tomi.ollila@iki.fi
>
> When tried w/ one replacement and w/o sq usage and emptied gecos, got
>
> .  @@ -9,5 +9,5 @@
> .  7: 'true'
> .  8: 'USERNAME@FQDN'
> .  9: 'NULL'
> . -a: 'USER_FULL_NAME'
> . +USER_FULL_NAMEa: ''
>
>  test/T590-libconfig.sh | 97 +++++++++++++++++++++++++-----------------
>  test/test-lib.sh       | 20 ---------
>  2 files changed, 59 insertions(+), 58 deletions(-)
>
> diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
> index 745e1bb4..42cbe6e0 100755
> --- a/test/T590-libconfig.sh
> +++ b/test/T590-libconfig.sh
> @@ -5,6 +5,26 @@ test_description="library config API"
>
>  add_email_corpus
>
> +_libconfig_sanitize() {
> +    ${NOTMUCH_PYTHON} -c '
> +import os, sys, pwd, socket

Why not use a heredoc?

  python <<-EOF
  ..
  EOF

> +pw = pwd.getpwuid(os.getuid())
> +user = pw.pw_name
> +name = pw.pw_gecos.partition(",")[0]
> +fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0,
> +                          socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
> +for l in sys.stdin:
> +    if l[:3] == "8: ":
> +        l = l.replace(user, "USERNAME", 1).replace("@" + fqdn, "@FQDN", 1)
> +        l = l.replace(".(none)", "", 1).replace(".localdomain", "", 1)
> +    elif l[:3] == "a: ":
> +        sq = chr(39) # single quote
> +        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)

Then we can simply do:

l.replace("'" + name, "'USER_FULL_NAME", 1)

The rest looks fine to me.

-- 
Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-19  7:29 ` Felipe Contreras
@ 2021-05-19  8:44   ` Tomi Ollila
  2021-05-19 17:34     ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2021-05-19  8:44 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: notmuch

On Wed, May 19 2021, Felipe Contreras wrote:

> On Tue, May 18, 2021 at 12:55 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>
>> notmuch_passwd_sanitize() in test-lib.sh is too generic, it cannot
>> work in many cases...
>>
>> The more specific version _libconfig_sanitize() replaces it in
>> T590-libconfig.sh and the code that uses it is modified to output
>> the keys (ascending numbers printed in hex) so the sanitizer knows
>> what to sanitize in which lines...
>>
>> "@" + fqdn -> "@FQDN" replacement is used as fqdn could --
>> in theory -- be substring of 'USERNAME'.
>>
>> 'user -> 'USER_FULL_NAME replacement to work in cases where user
>> is empty -- as only first ' is replaced that works as expected.
>>
>> In addition to ".(none)" now also ".localdomain" is filtered from
>> USERNAME@FQDN.
>> ---
>>
>> Changes to [v2]:
>>
>> * work in cases of empty user (e.g. in passwd gecos field)
>> * replace only 1st match; e.g. fqdn could contain substring of user
>>
>> v2: id:20210517193315.11343-1-tomi.ollila@iki.fi
>> v1: id:20210502181535.31292-1-tomi.ollila@iki.fi
>>
>> When tried w/ one replacement and w/o sq usage and emptied gecos, got
>>
>> .  @@ -9,5 +9,5 @@
>> .  7: 'true'
>> .  8: 'USERNAME@FQDN'
>> .  9: 'NULL'
>> . -a: 'USER_FULL_NAME'
>> . +USER_FULL_NAMEa: ''
>>
>>  test/T590-libconfig.sh | 97 +++++++++++++++++++++++++-----------------
>>  test/test-lib.sh       | 20 ---------
>>  2 files changed, 59 insertions(+), 58 deletions(-)
>>
>> diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
>> index 745e1bb4..42cbe6e0 100755
>> --- a/test/T590-libconfig.sh
>> +++ b/test/T590-libconfig.sh
>> @@ -5,6 +5,26 @@ test_description="library config API"
>>
>>  add_email_corpus
>>
>> +_libconfig_sanitize() {
>> +    ${NOTMUCH_PYTHON} -c '
>> +import os, sys, pwd, socket
>
> Why not use a heredoc?
>
>   python <<-EOF
>   ..
>   EOF

tldr: I'll post change to use heredoc.

Probably my bias against heredoc's when there are alternatives
-- although this is much more tolerable than cat <<EOF ... EOF
to write stuff to stdout ;)

Also, I did not recall it is this simple to read python code
from stdin to be executed (right, that loses stdin for user
input -- I was once bitten by that so that contributes to 
my bias).

Here the ability to use "'" (for clarity) is compelling reason
to use heredoc.

(alternatives would have been:

 * l.replace("'\''" + name ...

 * l.replace("'"'"'" + name ... ;D

 * use other delimiter than ' (but not unicode quotes >;)
)

While testing this option I looked (once again) how dash and
bash do heredocs (in linux) (just to update my knowledge):

dash creates pipe and dup2's fd[0] to 0 
     (that makes stdin not seekable)

bash clones subprocess; in subprocess it creates temporary file,
     writes data there, closes it, opens it for reading, 
     unlinks it from fs (could be problematic on windows), 
     dup2()'s it to stdin, closes the dupped fd and finally
     execve's python

zsh works like bash (i.e. these 2 provide seekable stdin)

$ (strace -f -ofile zsh heredoc-test.sh)

Tomi

>
>> +pw = pwd.getpwuid(os.getuid())
>> +user = pw.pw_name
>> +name = pw.pw_gecos.partition(",")[0]
>> +fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0,
>> +                          socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
>> +for l in sys.stdin:
>> +    if l[:3] == "8: ":
>> +        l = l.replace(user, "USERNAME", 1).replace("@" + fqdn, "@FQDN", 1)
>> +        l = l.replace(".(none)", "", 1).replace(".localdomain", "", 1)
>> +    elif l[:3] == "a: ":
>> +        sq = chr(39) # single quote
>> +        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
>
> Then we can simply do:
>
> l.replace("'" + name, "'USER_FULL_NAME", 1)
>
> The rest looks fine to me.
>
> -- 
> Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-19  8:44   ` Tomi Ollila
@ 2021-05-19 17:34     ` Tomi Ollila
  2021-05-19 19:51       ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2021-05-19 17:34 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: notmuch

On Wed, May 19 2021, Tomi Ollila wrote:

> On Wed, May 19 2021, Felipe Contreras wrote:
>
>>
>> Why not use a heredoc?
>>
>>   python <<-EOF
>>   ..
>>   EOF
>
> tldr: I'll post change to use heredoc.

... which did not work ...

> Probably my bias against heredoc's when there are alternatives
> -- although this is much more tolerable than cat <<EOF ... EOF
> to write stuff to stdout ;)
>
> Also, I did not recall it is this simple to read python code
> from stdin to be executed (right, that loses stdin for user
> input -- I was once bitten by that so that contributes to 
> my bias).

Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
reading python script from stdin don't work (perl has __DATA__ ;).
(bitten again, I did and tested the change... :D).

I'd rather keep the same "filter" style here as in everywhere
else in our test scripts, and chr(39) instead of exiting out
from the single quoted string temporarily in order to add
that/those single quote(s) to the argument for python3 -c ...

So [PATCH v3] in id:20210518055443.21964-1-tomi.ollila@iki.fi
is my fix proposal.

Tomi

>
> Here the ability to use "'" (for clarity) is compelling reason
> to use heredoc.
>
> (alternatives would have been:
>
>  * l.replace("'\''" + name ...
>
>  * l.replace("'"'"'" + name ... ;D
>
>  * use other delimiter than ' (but not unicode quotes >;)
> )
>
> While testing this option I looked (once again) how dash and
> bash do heredocs (in linux) (just to update my knowledge):
>
> dash creates pipe and dup2's fd[0] to 0 
>      (that makes stdin not seekable)
>
> bash clones subprocess; in subprocess it creates temporary file,
>      writes data there, closes it, opens it for reading, 
>      unlinks it from fs (could be problematic on windows), 
>      dup2()'s it to stdin, closes the dupped fd and finally
>      execve's python
>
> zsh works like bash (i.e. these 2 provide seekable stdin)
>
> $ (strace -f -ofile zsh heredoc-test.sh)
>
> Tomi
>
>>
>>> +pw = pwd.getpwuid(os.getuid())
>>> +user = pw.pw_name
>>> +name = pw.pw_gecos.partition(",")[0]
>>> +fqdn = socket.getaddrinfo(socket.gethostname(), 0, 0,
>>> +                          socket.SOCK_STREAM, 0, socket.AI_CANONNAME)[0][3]
>>> +for l in sys.stdin:
>>> +    if l[:3] == "8: ":
>>> +        l = l.replace(user, "USERNAME", 1).replace("@" + fqdn, "@FQDN", 1)
>>> +        l = l.replace(".(none)", "", 1).replace(".localdomain", "", 1)
>>> +    elif l[:3] == "a: ":
>>> +        sq = chr(39) # single quote
>>> +        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
>>
>> Then we can simply do:
>>
>> l.replace("'" + name, "'USER_FULL_NAME", 1)
>>
>> The rest looks fine to me.
>>
>> -- 
>> Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-19 17:34     ` Tomi Ollila
@ 2021-05-19 19:51       ` Felipe Contreras
  2021-05-20  7:43         ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2021-05-19 19:51 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On Wed, May 19, 2021 at 12:34 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:

> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
> reading python script from stdin don't work (perl has __DATA__ ;).
> (bitten again, I did and tested the change... :D).

That can be fixed with:

  python /dev/fd/3 3<<EOF
  EOF

-- 
Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-19 19:51       ` Felipe Contreras
@ 2021-05-20  7:43         ` Tomi Ollila
  2021-05-21  9:46           ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2021-05-20  7:43 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: notmuch

On Wed, May 19 2021, Felipe Contreras wrote:

> On Wed, May 19, 2021 at 12:34 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
>> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
>> reading python script from stdin don't work (perl has __DATA__ ;).
>> (bitten again, I did and tested the change... :D).
>
> That can be fixed with:
>
>   python /dev/fd/3 3<<EOF
>   EOF

According to 

https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links

that solution could be portable enough.

Another way still using -c ... I've played to look how it actually looks is
(diff since patch v3)

-        sq = chr(39) # single quote
-        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
+        l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1)

Tested the above. That python /dev/fd/3 3<<EOF is so neat it at least
have to be tested to see how it looks like and behaves... :D

>
> -- 
> Felipe Contreras

Tomi

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-20  7:43         ` Tomi Ollila
@ 2021-05-21  9:46           ` Felipe Contreras
  2021-05-21 18:22             ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2021-05-21  9:46 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On Thu, May 20, 2021 at 2:43 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
> On Wed, May 19 2021, Felipe Contreras wrote:
>
> > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:
> >
> >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
> >> reading python script from stdin don't work (perl has __DATA__ ;).
> >> (bitten again, I did and tested the change... :D).
> >
> > That can be fixed with:
> >
> >   python /dev/fd/3 3<<EOF
> >   EOF
>
> According to
>
> https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links
>
> that solution could be portable enough.

What the operating system does doesn't really matter, bash emulates /dev/fd/x:

"If the operating system on which Bash is running provides these
special files, bash will use them; otherwise it will emulate them
internally with the behavior described below."

https://www.gnu.org/software/bash/manual/html_node/Redirections.html

And as far as I know the testing framework only works correctly on bash... So...

> Another way still using -c ... I've played to look how it actually looks is
> (diff since patch v3)
>
> -        sq = chr(39) # single quote
> -        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
> +        l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1)

Yes, that works too. But that's what I said in another mail that is
weird stuff. I had to read it again three times and then copy to a
proper text editor with monospace font to see if it was correct.

> Tested the above. That python /dev/fd/3 3<<EOF is so neat it at least
> have to be tested to see how it looks like and behaves... :D

Yeap. Took me a while to find the right documentation to figure that
out, but in my opinion it's better to write a helper for the tests
once, and then forget about it and just re-use it for all.

-- 
Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-21  9:46           ` Felipe Contreras
@ 2021-05-21 18:22             ` Tomi Ollila
  2021-05-24  2:34               ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2021-05-21 18:22 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: notmuch

On Fri, May 21 2021, Felipe Contreras wrote:

> On Thu, May 20, 2021 at 2:43 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>
>> On Wed, May 19 2021, Felipe Contreras wrote:
>>
>> > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> >
>> >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
>> >> reading python script from stdin don't work (perl has __DATA__ ;).
>> >> (bitten again, I did and tested the change... :D).
>> >
>> > That can be fixed with:
>> >
>> >   python /dev/fd/3 3<<EOF
>> >   EOF
>>
>> According to
>>
>> https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links
>>
>> that solution could be portable enough.
>
> What the operating system does doesn't really matter, bash emulates /dev/fd/x:

In this case, /dev/fd/3 is given as a parameter to a command, not part of 
redirections -- bash cannot know how the program is going to use the
arguments it gets...

Anyway, the [PATCH v4] in id:20210520134628.11653-1-tomi.ollila@iki.fi
implemented your suggestion ${NOTMUCH_PYTHON} /dev/fd/3 3<<'EOF' ...

> "If the operating system on which Bash is running provides these
> special files, bash will use them; otherwise it will emulate them
> internally with the behavior described below."
>
> https://www.gnu.org/software/bash/manual/html_node/Redirections.html

... "Bash handles several filenames specially when they are used in
redirections, as described in the following table." is just before
your snippet >;D

>
> And as far as I know the testing framework only works correctly on bash... So...
>
>> Another way still using -c ... I've played to look how it actually looks is
>> (diff since patch v3)
>>
>> -        sq = chr(39) # single quote
>> -        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
>> +        l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1)
>
> Yes, that works too. But that's what I said in another mail that is
> weird stuff. I had to read it again three times and then copy to a
> proper text editor with monospace font to see if it was correct.

To me '\'' is /idiomatic/ way to embed ' in the middle of a long argument
string...


>> Tested the above. That python /dev/fd/3 3<<EOF is so neat it at least
>> have to be tested to see how it looks like and behaves... :D
>
> Yeap. Took me a while to find the right documentation to figure that
> out, but in my opinion it's better to write a helper for the tests
> once, and then forget about it and just re-use it for all.
>
> -- 
> Felipe Contreras

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

* Re: [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize()
  2021-05-21 18:22             ` Tomi Ollila
@ 2021-05-24  2:34               ` Felipe Contreras
  0 siblings, 0 replies; 9+ messages in thread
From: Felipe Contreras @ 2021-05-24  2:34 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

On Fri, May 21, 2021 at 1:22 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
> On Fri, May 21 2021, Felipe Contreras wrote:
>
> > On Thu, May 20, 2021 at 2:43 AM Tomi Ollila <tomi.ollila@iki.fi> wrote:
> >>
> >> On Wed, May 19 2021, Felipe Contreras wrote:
> >>
> >> > On Wed, May 19, 2021 at 12:34 PM Tomi Ollila <tomi.ollila@iki.fi> wrote:
> >> >
> >> >> Haha, as we do _libconfig_sanitize < OUTPUT > OUTPUT.clean
> >> >> reading python script from stdin don't work (perl has __DATA__ ;).
> >> >> (bitten again, I did and tested the change... :D).
> >> >
> >> > That can be fixed with:
> >> >
> >> >   python /dev/fd/3 3<<EOF
> >> >   EOF
> >>
> >> According to
> >>
> >> https://unix.stackexchange.com/questions/123602/portability-of-file-descriptor-links
> >>
> >> that solution could be portable enough.
> >
> > What the operating system does doesn't really matter, bash emulates /dev/fd/x:
>
> In this case, /dev/fd/3 is given as a parameter to a command, not part of
> redirections -- bash cannot know how the program is going to use the
> arguments it gets...

Right. I did try doing the redirection inside the python code. That
also works fine, but I don't think there's a need of doing that unless
there's an actual issue with /dev/fd/3.

> > And as far as I know the testing framework only works correctly on bash... So...
> >
> >> Another way still using -c ... I've played to look how it actually looks is
> >> (diff since patch v3)
> >>
> >> -        sq = chr(39) # single quote
> >> -        l = l.replace(sq + name, sq + "USER_FULL_NAME", 1)
> >> +        l = l.replace("'\''" + name, "'\''USER_FULL_NAME", 1)
> >
> > Yes, that works too. But that's what I said in another mail that is
> > weird stuff. I had to read it again three times and then copy to a
> > proper text editor with monospace font to see if it was correct.
>
> To me '\'' is /idiomatic/ way to embed ' in the middle of a long argument
> string...

Maybe. I've written a lot of shell code and whenever that happens I
switch to double quotes. "foo 'quote' bar" is much more readable to me
than 'foo '\''quote'\'' bar'.

Cheers.

-- 
Felipe Contreras

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

end of thread, other threads:[~2021-05-24  2:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18  5:54 [PATCH v3] test: replace notmuch_passwd_sanitize() with _libconfig_sanitize() Tomi Ollila
2021-05-19  7:29 ` Felipe Contreras
2021-05-19  8:44   ` Tomi Ollila
2021-05-19 17:34     ` Tomi Ollila
2021-05-19 19:51       ` Felipe Contreras
2021-05-20  7:43         ` Tomi Ollila
2021-05-21  9:46           ` Felipe Contreras
2021-05-21 18:22             ` Tomi Ollila
2021-05-24  2:34               ` Felipe Contreras

unofficial mirror of notmuch@notmuchmail.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


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

	notmuch.git.git (no URL configured)

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git