Re: [PATCH 1/4] test-lib: add XSAN_OPTIONS, inherit [AL]SAN_OPTIONS

[Taylor Blau <me@ttaylorr.com>]

On Sat, Feb 19, 2022 at 03:48:42AM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> > +}
> >> > +
> >> > +# If [AL]SAN is in effect we want to abort so that we notice problems.
> >> > +prepend_var XSAN_OPTIONS : abort_on_error=1
> >>
> >> XSAN_OPTIONS stands for "options that are common to all ?SAN", I
> >> guess.
> >
> > I was also unclear on this. Looking around in the google/sanitizers
> > repository, I don't see something called "XSAN_OPTIONS" mentioned
> > anywhere (neither in documentation nor in the actual source code).
> >
> > Is this a convenience variable that we use to store options that are
> > common to both ASAN_OPTIONS and LSAN_OPTIONS? If so, I am not sure the
> > extra confusion is worth it, since it only contains abort_on_error=1.
> >
> > I guess it makes it (along with the prepend_var function introduced by
> > this patch) possible for a caller to write XSAN_OPTIONS=... into their
> > environment and then run a test script and have their settings feed into
> > ASAN_OPTIONS and LSAN_OPTIONS. But I don't know that callers would find
> > this super useful (or, at least not dramatically more convenient than
> > setting both variables).
> >
> > I could be wrong, and I'm obviously biased towards my own usage of the
> > ASAN/LSAN builds, but to me this patch might be clearer without the
> > extra variable.
>
> Sorry, yes it's just a git.git invention to refer to "common LSAN and
> ASAN' things. Perhaps GIT_ASAN_AND_ASAN_COMMON or something would be
> much less confusing & better? Bikeshedding most welcome :)

Yeah, I would be fine with something like GIT_ASAN_AND_LSAN_COMMON if it
makes things easier. I think it definitely is clearer, and does make it
easier to add new options to both.

I probably wouldn't ever tweak its value myself, so I don't think I have
a strong opinion here. If it were me, I'd probably just as soon
duplicate setting `abort_on_error=1` in the ASan- and LSan-specific
variables. But I don't have strong feelings here, so whatever makes the
most sense to you is fine with me.

Thanks,
Taylor