Re: [PATCH 2/7] help -a: do not list commands that are excluded from the build
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 18, 2019, 12:06 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1904181403330.46@tvgsbejvaqbjf.bet>
- In-Reply-To
- <xmqqo957sq2g.fsf@gitster-ct.c.googlers.com>
Hi Junio,
On Mon, 15 Apr 2019, Junio C Hamano wrote:
Show 32 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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.
Show 9 quoted lines
> > @@ -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