Hi Junio,
On Mon, 15 Apr 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
> > $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> > diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> > index 709d67405b..7867b99d19 100755
> > --- a/generate-cmdlist.sh
> > +++ b/generate-cmdlist.sh
> > @@ -6,7 +6,7 @@ die () {
> > }
> >
> > command_list () {
> > - grep -v '^#' "$1"
> > + eval grep -ve '^#' $exclude_programs "$1"
>
> The original protects against $IFS in the filename given as $1, but
> with the eval that is no longer true. We have been feeding the
> constant "command-list.txt" to the program since its inception, and
> I do not expect it to change, so this regression in defensiveness
> does not matter in practice. Also because # is prefixed with ^, the
> unintended loss of quotes around it when the eval makes the shell
> re-parse the generated command line would not make the remainder
> into a comment.
>
> But it does look brittle, and smells like a trap for less
> experienced shell programmers to blindly copy & paste & tweak
> without understanding what is going on, leading to bugs.
>
> eval "grep -v -e '^#' $exclude_programs" <"$1"
>
> or something like that, perhaps?
Yes! Thank you.
> > @@ -93,6 +93,14 @@ EOF
> > EOF
> > }
> >
> > +exclude_programs=
> > +while test "a$1" = "a--exclude-program"
>
> s/a/z/g; if you want to pretend to be old-fashioned, but s/a//g;
> should be sufficient in the modern world.
Don't you know, I always used "a" without realizing. You can see my
misdeed even in git-rebase--preserve-merges.sh. I won't fix it there,
though, as I hope that it'll be gone soon enough.
Will be fixed in the next iteration I send.
Thanks,
Dscho
