Re: [PATCH 1/2] test: provide machinery to make and use test_shims

Subject: Re: [PATCH 1/2] test: provide machinery to make and use test_shims

Date: Tue, 25 Jun 2019 19:33:47 -0400

To: Tomi Ollila, David Bremner, notmuch@notmuchmail.org

Cc:

From: Daniel Kahn Gillmor


On Wed 2019-06-26 00:05:09 +0300, Tomi Ollila wrote:
> LD_PRELOAD=./${shim_file}${LD_PRELOAD:+:$LD_PRELOAD} notmuch-shared "$@"
>
> So that if LD_PRELOAD is already set and is non-empty then ':' and its
> old value is appended to new value of LD_PRELOAD before this
> notmuch-shared invocation.

agreed, good catch, Tomi.

> Also I'm "insisting" that your patch 2/2 is updated based on
> id:8736kipe9f.fsf@fifthhorseman.net -- dkg had good opening how to 
> improve robustness in `eval` executions, but since there are quite
> a few similar problems in code (i.e. and there are quite a few ways
> to improve it) IMO maintaining status quo we can this feature improved
> incrementally and choose how to fix *all* related problems...

sounds fine to me, as long as we've got gen_insert_msg in the right
place.

> One option, which is relatively easy to start using (when needed) while
> writing code is to use ${parameter@Q} transformation; 
>
> from bash (4.4+) documentation:
>
>     Q      The  expansion is a string that is the value of parameter
>            quoted in a format that can be reused as input.
>
> ( that transforms e.g.  foo'bar to 'foo'\''bar' )

nice find!  "reused as input" sounds pretty weird to me.  does it mean
"reused as a single shell token" or something like that?  because:

    x='foo bar'
    head -v $x

is a legitimate form of "reusing" $x as input, though it will read the
two files "foo" and "bar" instead of the single file "foo bar".

I suppose "help printf" itself says:

        %q	quote the argument in a way that can be reused as shell input

so it probably means the same thing :)

> I personally would like to use this feature; it is easy to add into scripts
> and pretty lean learning curve. I tried to do the same with bash syntax
> ${param//\'/\'\\\'\'}, but that looks more complicated -- and even more 
> complicated to get it working right on bash 4.1 where the feature is a bit
> buggy when included inside double quotes ;(

ugh, this is not only more visually complicated, it's also much harder
to tell if it's correct at a glance.

> […]
> likewise, Ubuntu 16.04 (xenial) has neither (not even in backports)

Thanks for doing the research about where bash 4.4 is supported.  I
don't personally mind requiring bash 4.4 for running the test suite.

I can add older bash to the xenial PPA for travis if we need to (or if
someone else wants to to it, i'm happy to add you as an authorized
uploader for that repo).

      --dkg
signature.asc (application/pgp-signature)
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: