On Wed, Jun 16, 2021 at 11:49:20AM +0200, Ævar Arnfjörð Bjarmason wrote: > > [...] 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 don't have much of an opinion on other uses of the scratch dir, not having seen them. I agree if we do have one and are paying the cost for it already, then using it here isn't a big deal. > I am a bit wary of this being our first ever use of "set -f", but maybe > it's sufficiently portable. Yep, me too. The nice thing about it is that we can swap out the solution pretty easily if it turns out not to be. I don't know how good our coverage of obscure shells is in CI, though (e.g., on your gcc build-farm stuff, are we mostly using system shells?). > > 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. My main complaint is that it's a real gotcha for the callers. They have to remember to do this cd-to-scratch (or whatever technique we use). That's cumbersome, and if they forget, their call is wrong in a really subtle way that basic testing isn't likely to uncover. -Peff
