On Wed, Mar 30, 2022 at 11:13 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "Neeraj Singh via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > diff --git a/t/lib-unique-files.sh b/t/lib-unique-files.sh
> > new file mode 100644
> > index 00000000000..34c01a65256
> > --- /dev/null
> > +++ b/t/lib-unique-files.sh
> > @@ -0,0 +1,34 @@
> > +# Helper to create files with unique contents
> > +
> > +# Create multiple files with unique contents within this test run. Takes the
> > +# number of directories, the number of files in each directory, and the base
> > +# directory.
> > +#
> > +# test_create_unique_files 2 3 my_dir -- Creates 2 directories with 3 files
> > +# each in my_dir, all with contents
> > +# different from previous invocations
> > +# of this command in this run.
> > +
> > +test_create_unique_files () {
> > + test "$#" -ne 3 && BUG "3 param"
> > +
> > + local dirs="$1" &&
> > + local files="$2" &&
> > + local basedir="$3" &&
> > + local counter=0 &&
>
> The cover letter mentioned that in this round, "local var=val" is
> avoided?
>
Whoops.. I guess I wasn't looking widely enough. I noticed failures
on my Ubuntu system that uses dash.
> I've seen instances of local being "curious", e.g.
> https://lore.kernel.org/git/20220125092419.cgtfw32nk2niazfk@carbon/
> and the discussion indicates that it may still be relevant.
>
> > + local i &&
> > + local j &&
> > + test_tick &&
> > + local basedata=$basedir$test_tick &&
> > + rm -rf "$basedir" &&
> > + for i in $(test_seq $dirs)
> > + do
> > + local dir=$basedir/dir$i &&
>
> This, too.
>
> To summarize the findings from the thread is:
>
> - very old releases of /bin/dash that predates Git, like 0.3.8,
> would not correctly handle assignment on "local" at all. It may
> not matter to us.
>
> - semi-old /bin/dash 0.5.10, which is still in use, mishandled
> 'local var=$val', but 'local var="$val"' is an acceptable
> workaround for the bug. "git grep" tells us that we use this
> form in our shell scripts quite a lot, so we may be OK.
>
> - /bin/dash 0.5.11, which was tagged mid 2020, and newer would glok
> 'local var=$val' just fine even $val has $IFS whitespace in it.
>
> So, I'd say the safe practice we should adopt is to use "local" one
> per variable, assignment to the variable on the same line of "local"
> is OK, but unlike the regular assignment, older dash may mishandle
> the right-hand-side unless it is quoted, i.e.
>
> local var='string literal'
> local var="$variable interpolation"
>
Thanks for updating me on the documentation. I think I'll go back to
one-line assignments with the proper quotations and single variables,
just to keep things short and consistent. I've been looking up
everything as I go with these shell scripts, but there's a lot of
subtlety. CMD batch files are even uglier, but I've had many years to
learn the common practices there.
I'll ask a question on the perf test patch on a related shell topic.
Thanks,
Neeraj
