Re: [Valgrind-developers] Proposed valgrind environment-variable patch

[D. J. B. <aut...@bo...>]

Paul Floyd via Valgrind-developers writes:
> On 26-10-24 16:58, D. J. Bernstein wrote:
> > Paul Floyd via Valgrind-developers writes:
> > > 1. Using a / in environment variables doesn't work with most (all?) shells.
> > Also hyphens; but, as noted in my previous message, this is only a minor
> > disadvantage since one can set the variables using env rather than shell
> > builtins. This is also what the tests in the patch do, and it works for
> > the intended use case of simplifying scripts.
> Sorry to be so difficult, but I think that it's best to get things
> like this right before letting them out into the wild.

Of course! But please note that valgrind-env-patch-20241026.txt already
switched to being sh-builtin-compatible, using VALGRIND__x_y=z for
--x-y=z.

> How about just having VALGRIND_TRYOPTS or VALGRIND_TRY_OPTS. That would be
> the same as VALGRIND_OPTS but not result in failure if there's an
> unrecognized option.

This would create the following unnecessary usability issue: if a script
does an opportunistic

    env VALGRIND_TRY_OPTS='--leak-check=yes' valgrind ...

and the user has set an independent opportunistic

    export VALGRIND_TRY_OPTS='--time-stamp=yes'

then the script's VALGRIND_TRY_OPTS will wipe out the user's
VALGRIND_TRY_OPTS, unless the script author thinks of doing

    env VALGRIND_TRY_OPTS="$VALGRIND_TRY_OPTS --leak-check=yes" valgrind ...

instead. You can see from, e.g., the use of VALGRIND_OPTS in

    https://github.com/apache/subversion/blob/trunk/subversion/tests/README

that people often miss these composability issues. For comparison, with
valgrind-env-patch-20241026.txt, a script doing

    env VALGRIND__leak_check=yes valgrind ...

won't interfere with a user setting an independent opportunistic option:

    export VALGRIND__time_stamp=yes

It's also clear what the conciseness winner is in these examples; what
makes it easier for scripts to check whether an option is set, if that's
desired; etc.

In terms of purity, VALGRIND_OPTS has punctuation issues with user data
(what happens if an option value needs an embedded space?), while the
analysis of collisions for valgrind-env-patch-20241026.txt is determined
entirely by the valgrind-controlled list of option names. I also think
that analogies to VALGRIND_CLO_CHANGE, which already has the feature of
being opportunistic, are more useful than analogies to VALGRIND_OPTS.

---D. J. Bernstein