Re: [PATCH] test-lib: fix "$remove_trash" regression and match_pattern_list() bugs

[Jeff King <peff@peff.net>]

On Wed, Jun 16, 2021 at 01:38:55PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > +	# We need to use "$*" to get field-splitting, but we want to
> > +	# disable globbing, since we are matching against an arbitrary
> > +	# $arg, not what's in the filesystem. Using "set -f" accomplishes
> > +	# that, but we must do it in a subshell to avoid impacting the
> > +	# rest of the script. The exit value of the subshell becomes
> > +	# the function's return value.
> > +	(
> > +		set -f
> > +		for pattern_ in $*
> > +		do
> > +			case "$arg" in
> > +			$pattern_)
> > +				exit 0
> > +				;;
> > +			esac
> > +		done
> > +		exit 1
> > +	)
> >  }
> 
> Why not just start with a ret=1, set ret=0 if we have a match and break
> from the loop, and then do a "set +f" afterwards? I.e. is there an
> actual need for the subshell here.

My thought was that the subshell takes us back to the original state,
regardless of what it was. As opposed to "set +f" which takes us back to
a particular state. But it is unlikely that we'd have done a global "set
-f" before calling this, so maybe that is being overly conservative.

> I'm mildly paranoid about a new "set -<flag>" in the codebase for vague
> fears of portability (as noted in my linked message), but whatever shell
> supports "set -<flag>" surely supports the inverse with "set +<flag>",
> no?

Yes, I think we can assume that if it supports one, it supports the
other.

-Peff