On Wed, Jun 16 2021, Jeff King wrote: > On Wed, Jun 16, 2021 at 10:24:13AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> Those options will try to match an argument like --verbose-only=1* >> against a test number like "10", but since we run the match in our own >> trash directory an earlier test creating a file like "1one.txt" will >> break that option. >> >> We cannot simply quote the $GIT_SKIP_TESTS" being passed to >> match_pattern_list(), since we are relying on the $IFS semantics. >> >> Let's instead setup a .test-lib-trash subdirectory under the trash >> directory, and an "empty-dir" directory under that. Then let's run the >> match_pattern_list() in a sub-shell in that directory. > > Looks like my email just crossed with this one. Your "cd to an empty > directory" is a fun version of my "maybe somebody can think of something > clever" statement. :) Yeah, I sent it before seeing yours. > As a general solution, it does fail if the globs may contain things that > look like absolute paths, but that is quite unlikely for our use case > here.[...] Not just unlikely, impossible. Yes it's possible in the general case, but in the match_pattern_list we are always normalizing e.g. ./t1234-some-test.sh t t1234, and the other match cases are test numbers etc. It doesn't need to deal with the general case. > [...] Still, I kind of like the "set -f" version because it doesn't need > the extra directory which could cause problems with "ls-files -o", etc, > as you mentioned. You could also create the empty directory on the fly, > though if "set -f" works portably, that seems less complicated to me. Yeah, in isolation I'd agree, but given the desire (with existing code and other in-flight code) to have a scratch are for the test library code itself simply creating such a directory seems like a good thing to have in general, and once we have it we can use the subshell trick, or just "set -f" I suppose and use the scratch dir for other stuff. I am a bit wary of this being our first ever use of "set -f", but maybe it's sufficiently portable. > Whatever the expansion mechanism, I do think it's worth having callers > quote "$GIT_SKIP_TESTS" and then performing the expansion within > match_pattern_list. Then the nasty mechanics are all in that one place. I think it's rather clean to not quote it, i.e. to have the loop get a list of item and then things to match, it would also make it easier to e.g. port it to a native C program.
